0

[Instant Hotspot] Show Tether Section if the user has any devices

This is one of two changes needed to implement the "Set Up Your Device"
button in Network Quick Settings (UI Deck: https://docs.google.com/presentation/d/1EDKDiBCu-KLgGGHL0Laz90S2BsEzU0HLuanbDHhpzdI/edit?resourcekey=0-HLWeff66N6-mD6ZIZFnWSg#slide=id.g21d9e5a24ff_0_0)

This button is shown in the "Your Devices" section if the user has
one eligible device for hotspot which is not currently enabled.

This change does the first part of this - if the user has
such a device, we will always shown the "Your Devices"
section if the rebrand is enabled. The next change will
update the content inside the section to include this button.

Example if the user has an eligible but not-set-up device: https://screenshot.googleplex.com/9VmX9KeTWbxJPpy

Example if the user has no devices to set up: https://screenshot.googleplex.com/UdfV5LRmUQqFZHP

Example if the user has a set up device: https://screenshot.googleplex.com/AbFyduoDicm4g3n

Test: Added Unit Tests

Change-Id: Ic5144eb5c72fec698670c633e65e4b8c2ef8d266
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5249740
Reviewed-by: Regan Hsu <hsuregan@chromium.org>
Commit-Queue: Joe Antonetti <joeantonetti@google.com>
Reviewed-by: Nikhil Nayunigari <nikhilcn@google.com>
Reviewed-by: Chad Duffin <chadduffin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1257516}
This commit is contained in:
Joe Antonetti
2024-02-07 19:53:40 +00:00
committed by Chromium LUCI CQ
parent 1298570379
commit 225ad350f4
3 changed files with 97 additions and 3 deletions

@ -12,6 +12,7 @@
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
#include "ash/shell_delegate.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/style/ash_color_provider.h"
#include "ash/style/typography.h"
@ -27,6 +28,7 @@
#include "base/timer/timer.h"
#include "chromeos/ash/components/dbus/hermes/hermes_manager_client.h"
#include "chromeos/ash/services/bluetooth_config/public/cpp/cros_bluetooth_config_util.h"
#include "chromeos/ash/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
#include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
#include "chromeos/services/network_config/public/mojom/network_types.mojom-shared.h"
@ -151,6 +153,18 @@ NetworkListViewControllerImpl::NetworkListViewControllerImpl(
remote_cros_bluetooth_config_->ObserveSystemProperties(
cros_system_properties_observer_receiver_.BindNewPipeAndPassRemote());
if (features::IsInstantHotspotRebrandEnabled()) {
Shell::Get()->shell_delegate()->BindMultiDeviceSetup(
multidevice_setup_remote_.BindNewPipeAndPassReceiver());
multidevice_setup_remote_->AddHostStatusObserver(
host_status_observer_receiver_.BindNewPipeAndPassRemote());
multidevice_setup_remote_->GetHostStatus(
base::BindOnce(&NetworkListViewControllerImpl::OnHostStatusChanged,
weak_ptr_factory_.GetWeakPtr()));
}
GetNetworkStateList();
}
@ -194,6 +208,15 @@ void NetworkListViewControllerImpl::GetNetworkStateList() {
weak_ptr_factory_.GetWeakPtr()));
}
void NetworkListViewControllerImpl::OnHostStatusChanged(
multidevice_setup::mojom::HostStatus host_status,
const std::optional<multidevice::RemoteDevice>& host_device) {
has_phone_eligible_for_setup_ =
(host_status ==
multidevice_setup::mojom::HostStatus::kEligibleHostExistsButNoHostSet);
GetNetworkStateList();
}
void NetworkListViewControllerImpl::OnGetNetworkStateList(
std::vector<NetworkStatePropertiesPtr> networks) {
int old_position = network_detailed_network_view()->GetScrollPosition();
@ -439,7 +462,6 @@ size_t NetworkListViewControllerImpl::ShowConnectionWarningIfNetworkMonitored(
void NetworkListViewControllerImpl::MaybeShowConnectionWarningManagedIcon(
bool using_proxy) {
// If the proxy is set, check if it's a managed setting.
const NetworkStateProperties* default_network = model()->default_network();
if (using_proxy && default_network) {
@ -580,7 +602,16 @@ bool NetworkListViewControllerImpl::ShouldMobileDataSectionBeShown() {
bool NetworkListViewControllerImpl::ShouldTetherHostsSectionBeShown() {
// The section should never be shown if the feature flag is disabled.
DCHECK(features::IsInstantHotspotRebrandEnabled());
if (!features::IsInstantHotspotRebrandEnabled()) {
return false;
}
// The Tether Hosts section should always be shown if there is an eligible
// device which has not been set up yet.
if (has_phone_eligible_for_setup_) {
return true;
}
const DeviceStateType tether_state =
model()->GetDeviceState(NetworkType::kTether);

@ -25,6 +25,7 @@
#include "base/memory/raw_ptr_exclusion.h"
#include "base/timer/timer.h"
#include "chromeos/ash/services/bluetooth_config/public/mojom/cros_bluetooth_config.mojom.h"
#include "chromeos/ash/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "mojo/public/cpp/bindings/remote.h"
@ -43,6 +44,7 @@ class NetworkDetailedNetworkView;
class ASH_EXPORT NetworkListViewControllerImpl
: public TrayNetworkStateObserver,
public NetworkListViewController,
public multidevice_setup::mojom::HostStatusObserver,
public bluetooth_config::mojom::SystemPropertiesObserver {
public:
NetworkListViewControllerImpl(
@ -82,6 +84,11 @@ class ASH_EXPORT NetworkListViewControllerImpl
using NetworkIdToViewMap =
base::flat_map<std::string, NetworkListNetworkItemView*>;
// multidevice_setup::mojom::HostStatusObserver:
void OnHostStatusChanged(
multidevice_setup::mojom::HostStatus host_status,
const std::optional<multidevice::RemoteDevice>& host_device) override;
// TrayNetworkStateObserver:
void ActiveNetworkStateChanged() override;
void NetworkListChanged() override;
@ -217,6 +224,10 @@ class ASH_EXPORT NetworkListViewControllerImpl
remote_cros_bluetooth_config_;
mojo::Receiver<bluetooth_config::mojom::SystemPropertiesObserver>
cros_system_properties_observer_receiver_{this};
mojo::Remote<multidevice_setup::mojom::MultiDeviceSetup>
multidevice_setup_remote_;
mojo::Receiver<multidevice_setup::mojom::HostStatusObserver>
host_status_observer_receiver_{this};
bluetooth_config::mojom::BluetoothSystemState bluetooth_system_state_ =
bluetooth_config::mojom::BluetoothSystemState::kUnavailable;
@ -284,6 +295,10 @@ class ASH_EXPORT NetworkListViewControllerImpl
// managed.
bool is_vpn_managed_ = false;
// Indicates whether the user has a phone which could be set up via the
// cross-device suite of features.
bool has_phone_eligible_for_setup_ = false;
raw_ptr<NetworkDetailedNetworkView, DanglingUntriaged>
network_detailed_network_view_;
NetworkIdToViewMap network_id_to_view_map_;

@ -27,6 +27,8 @@
#include "ash/system/tray/tri_view.h"
#include "ash/test/ash_test_base.h"
#include "ash/test/ash_test_helper.h"
#include "ash/test_shell_delegate.h"
#include "base/functional/bind.h"
#include "base/memory/raw_ptr.h"
#include "base/run_loop.h"
#include "base/test/bind.h"
@ -36,6 +38,9 @@
#include "chromeos/ash/services/bluetooth_config/fake_adapter_state_controller.h"
#include "chromeos/ash/services/bluetooth_config/public/mojom/cros_bluetooth_config.mojom.h"
#include "chromeos/ash/services/bluetooth_config/scoped_bluetooth_config_test_helper.h"
#include "chromeos/ash/services/multidevice_setup/public/cpp/fake_multidevice_setup.h"
#include "chromeos/ash/services/multidevice_setup/public/mojom/multidevice_setup.mojom-shared.h"
#include "chromeos/ash/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
#include "chromeos/ash/services/network_config/public/cpp/cros_network_config_test_helper.h"
#include "chromeos/services/network_config/public/cpp/cros_network_config_util.h"
#include "chromeos/services/network_config/public/cpp/fake_cros_network_config.h"
@ -193,7 +198,14 @@ class NetworkListViewControllerTest : public AshTestBase,
}
feature_list_.InitWithFeatures(enabled_features, disabled_features);
AshTestBase::SetUp();
fake_multidevice_setup_ =
std::make_unique<multidevice_setup::FakeMultiDeviceSetup>();
auto delegate = std::make_unique<TestShellDelegate>();
delegate->SetMultiDeviceSetupBinder(base::BindRepeating(
&multidevice_setup::MultiDeviceSetupBase::BindReceiver,
base::Unretained(fake_multidevice_setup_.get())));
AshTestBase::SetUp(std::move(delegate));
cros_network_ = std::make_unique<FakeCrosNetworkConfig>();
Shell::Get()
@ -451,6 +463,9 @@ class NetworkListViewControllerTest : public AshTestBase,
FakeCrosNetworkConfig* cros_network() { return cros_network_.get(); }
std::unique_ptr<multidevice_setup::FakeMultiDeviceSetup>
fake_multidevice_setup_;
base::HistogramTester histogram_tester;
private:
@ -839,6 +854,39 @@ TEST_P(NetworkListViewControllerTest, HasCorrectEthernetNetworkList) {
/*guid=*/kCellularName);
}
TEST_P(NetworkListViewControllerTest,
WillShowTetherHostsNetworkListWhenHostIsAvailable) {
for (const auto& host_status :
{multidevice_setup::mojom::HostStatus::kNoEligibleHosts,
multidevice_setup::mojom::HostStatus::kHostVerified,
multidevice_setup::mojom::HostStatus::
kHostSetLocallyButWaitingForBackendConfirmation,
multidevice_setup::mojom::HostStatus::kHostSetButNotYetVerified}) {
fake_multidevice_setup_->NotifyHostStatusChanged(host_status, std::nullopt);
fake_multidevice_setup_->FlushForTesting();
base::RunLoop().RunUntilIdle();
// Since we didn't send a notification that the host is ready and not set,
// the Tether section shouldn't be shown regardless of the value of the
// feature flag.
EXPECT_THAT(GetTetherHostsSubHeader(), IsNull());
}
fake_multidevice_setup_->NotifyHostStatusChanged(
multidevice_setup::mojom::HostStatus::kEligibleHostExistsButNoHostSet,
std::nullopt);
fake_multidevice_setup_->FlushForTesting();
base::RunLoop().RunUntilIdle();
// If the rebrand is enabled, the tether section should be shown. If not, it
// shouldn't be
if (IsInstantHotspotRebrandEnabled()) {
EXPECT_THAT(GetTetherHostsSubHeader(), NotNull());
} else {
EXPECT_THAT(GetTetherHostsSubHeader(), IsNull());
}
}
TEST_P(NetworkListViewControllerTest, HasCorrectTetherHostsNetworkList) {
EXPECT_EQ(0u, network_list(NetworkType::kTether)->children().size());
EXPECT_THAT(GetTetherHostsSubHeader(), IsNull());