[CrOS Multidevice] Only show Instant Tethering if Unified Setup has been completed.

Integrates MultiDeviceSetupClient into TetherService.

If the chromeos::features::kEnableUnifiedMultiDeviceSetup flag is enabled,
Instant Tethering will only be displayed in Settings and Quick Settings
if a verified MultiDevice host exists.

Bug: 824568
Change-Id: Iad6b6778a2986c7c92176bc7d8f4ac47bd47f5e3
Reviewed-on: https://chromium-review.googlesource.com/1141145
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576750}

chrome/browser/chromeos/tether/fake_tether_service.cc

@@ -14,6 +14,8 @@ FakeTetherService::FakeTetherService(
     cryptauth::CryptAuthService* cryptauth_service,
     chromeos::device_sync::DeviceSyncClient* device_sync_client,
     chromeos::secure_channel::SecureChannelClient* secure_channel_client,
+    chromeos::multidevice_setup::MultiDeviceSetupClient*
+        multidevice_setup_client,
     chromeos::NetworkStateHandler* network_state_handler,
     session_manager::SessionManager* session_manager)
     : TetherService(profile,
@@ -21,6 +23,7 @@ FakeTetherService::FakeTetherService(
                     cryptauth_service,
                     device_sync_client,
                     secure_channel_client,
+                    multidevice_setup_client,
                     network_state_handler,
                     session_manager) {}
 

chrome/browser/chromeos/tether/fake_tether_service.h

@@ -18,6 +18,8 @@ class FakeTetherService : public TetherService {
       cryptauth::CryptAuthService* cryptauth_service,
       chromeos::device_sync::DeviceSyncClient* device_sync_client,
       chromeos::secure_channel::SecureChannelClient* secure_channel_client,
+      chromeos::multidevice_setup::MultiDeviceSetupClient*
+          multidevice_setup_client,
       chromeos::NetworkStateHandler* network_state_handler,
       session_manager::SessionManager* session_manager);
 

chrome/browser/chromeos/tether/tether_service.cc

@@ -112,6 +112,8 @@ std::string TetherService::TetherFeatureStateToString(
       return "[Wi-Fi is not present on the device]";
     case (TetherFeatureState::SUSPENDED):
       return "[Suspended]";
+    case (TetherFeatureState::MULTIDEVICE_HOST_UNVERIFIED):
+      return "[MultiDevice host unverified]";
     case (TetherFeatureState::TETHER_FEATURE_STATE_MAX):
       // |previous_feature_state_| is initialized to TETHER_FEATURE_STATE_MAX,
       // and this value is never actually used in practice.
@@ -128,6 +130,8 @@ TetherService::TetherService(
     cryptauth::CryptAuthService* cryptauth_service,
     chromeos::device_sync::DeviceSyncClient* device_sync_client,
     chromeos::secure_channel::SecureChannelClient* secure_channel_client,
+    chromeos::multidevice_setup::MultiDeviceSetupClient*
+        multidevice_setup_client,
     chromeos::NetworkStateHandler* network_state_handler,
     session_manager::SessionManager* session_manager)
     : profile_(profile),
@@ -135,6 +139,7 @@ TetherService::TetherService(
       cryptauth_service_(cryptauth_service),
       device_sync_client_(device_sync_client),
       secure_channel_client_(secure_channel_client),
+      multidevice_setup_client_(multidevice_setup_client),
       network_state_handler_(network_state_handler),
       session_manager_(session_manager),
       notification_presenter_(
@@ -161,33 +166,37 @@ TetherService::TetherService(
   tether_host_fetcher_->AddObserver(this);
   power_manager_client_->AddObserver(this);
   network_state_handler_->AddObserver(this, FROM_HERE);
-  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
     device_sync_client_->AddObserver(this);
 
+    if (base::FeatureList::IsEnabled(
+            chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
+      multidevice_setup_client_->AddObserver(this);
+    }
+  }
+
   registrar_.Init(profile_->GetPrefs());
   registrar_.Add(prefs::kInstantTetheringAllowed,
-                 base::Bind(&TetherService::OnPrefsChanged,
-                            weak_ptr_factory_.GetWeakPtr()));
+                 base::BindRepeating(&TetherService::OnPrefsChanged,
+                                     weak_ptr_factory_.GetWeakPtr()));
 
   UMA_HISTOGRAM_BOOLEAN("InstantTethering.UserPreference.OnStartup",
                         IsEnabledbyPreference());
   PA_LOG(INFO) << "TetherService has started. Initial user preference value: "
                << IsEnabledbyPreference();
 
-  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
-      !device_sync_client_->is_ready()) {
-    // Wait for OnReady() to be called. It will call GetAdapter().
+  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
+    if (device_sync_client_->is_ready())
+      OnReady();
+
+    // Wait for OnReady() to be called. If
+    // chromeos::features::kEnableUnifiedMultiDeviceSetup is disabled,
+    // OnReady() will call GetAdapter(). If enabled, OnReady() will indirectly
+    // call OnHostStatusChanged(), which will call GetAdapter().
     return;
   }
 
-  // GetAdapter may call OnBluetoothAdapterFetched immediately which can cause
-  // problems with the Fake implementation since the class is not fully
-  // constructed yet. Post the GetAdapter call to avoid this.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE,
-      base::Bind(device::BluetoothAdapterFactory::GetAdapter,
-                 base::Bind(&TetherService::OnBluetoothAdapterFetched,
-                            weak_ptr_factory_.GetWeakPtr())));
+  GetBluetoothAdapter();
 }
 
 TetherService::~TetherService() {
@@ -259,6 +268,10 @@ void TetherService::StopTetherIfNecessary() {
       shutdown_reason = chromeos::tether::TetherComponent::ShutdownReason::
           BLUETOOTH_CONTROLLER_DISAPPEARED;
       break;
+    case MULTIDEVICE_HOST_UNVERIFIED:
+      shutdown_reason = chromeos::tether::TetherComponent::ShutdownReason::
+          MULTIDEVICE_HOST_UNVERIFIED;
+      break;
     default:
       PA_LOG(ERROR) << "Unexpected shutdown reason. FeatureState is "
                     << GetTetherFeatureState() << ".";
@@ -282,8 +295,14 @@ void TetherService::Shutdown() {
   tether_host_fetcher_->RemoveObserver(this);
   power_manager_client_->RemoveObserver(this);
   network_state_handler_->RemoveObserver(this, FROM_HERE);
-  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
     device_sync_client_->RemoveObserver(this);
+
+    if (base::FeatureList::IsEnabled(
+            chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
+      multidevice_setup_client_->RemoveObserver(this);
+    }
+  }
   if (adapter_)
     adapter_->RemoveObserver(this);
   registrar_.RemoveAll();
@@ -401,9 +420,24 @@ void TetherService::OnReady() {
   if (shut_down_)
     return;
 
-  device::BluetoothAdapterFactory::GetAdapter(
-      base::Bind(&TetherService::OnBluetoothAdapterFetched,
-                 weak_ptr_factory_.GetWeakPtr()));
+  if (base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
+    multidevice_setup_client_->GetHostStatus(base::BindOnce(
+        &TetherService::OnHostStatusChanged, weak_ptr_factory_.GetWeakPtr()));
+  } else {
+    GetBluetoothAdapter();
+  }
+}
+
+void TetherService::OnHostStatusChanged(
+    chromeos::multidevice_setup::mojom::HostStatus host_status,
+    const base::Optional<cryptauth::RemoteDeviceRef>& host_device) {
+  host_status_ = host_status;
+
+  if (adapter_)
+    UpdateTetherTechnologyState();
+  else
+    GetBluetoothAdapter();
 }
 
 void TetherService::OnPrefsChanged() {
@@ -461,6 +495,7 @@ TetherService::GetTetherTechnologyState() {
     case WIFI_NOT_PRESENT:
     case NO_AVAILABLE_HOSTS:
     case CELLULAR_DISABLED:
+    case MULTIDEVICE_HOST_UNVERIFIED:
       return chromeos::NetworkStateHandler::TechnologyState::
           TECHNOLOGY_UNAVAILABLE;
 
@@ -485,6 +520,18 @@ TetherService::GetTetherTechnologyState() {
   }
 }
 
+void TetherService::GetBluetoothAdapter() {
+  // In the case that this is indirectly called from the constructor,
+  // GetAdapter() may call OnBluetoothAdapterFetched immediately which can cause
+  // problems with the Fake implementation since the class is not fully
+  // constructed yet. Post the GetAdapter call to avoid this.
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(device::BluetoothAdapterFactory::GetAdapter,
+                                base::BindRepeating(
+                                    &TetherService::OnBluetoothAdapterFetched,
+                                    weak_ptr_factory_.GetWeakPtr())));
+}
+
 void TetherService::OnBluetoothAdapterFetched(
     scoped_refptr<device::BluetoothAdapter> adapter) {
   if (shut_down_)
@@ -603,6 +650,14 @@ TetherService::TetherFeatureState TetherService::GetTetherFeatureState() {
   if (!IsEnabledbyPreference())
     return USER_PREFERENCE_DISABLED;
 
+  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
+      base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSetup) &&
+      host_status_ !=
+          chromeos::multidevice_setup::mojom::HostStatus::kHostVerified) {
+    return MULTIDEVICE_HOST_UNVERIFIED;
+  }
+
   return ENABLED;
 }
 
@@ -669,8 +724,8 @@ bool TetherService::HandleFeatureStateMetricIfUninitialized() {
   // metric value is actually correct.
   timer_->Start(FROM_HERE,
                 base::TimeDelta::FromSeconds(kMetricFalsePositiveSeconds),
-                base::Bind(&TetherService::RecordTetherFeatureState,
-                           weak_ptr_factory_.GetWeakPtr()));
+                base::BindRepeating(&TetherService::RecordTetherFeatureState,
+                                    weak_ptr_factory_.GetWeakPtr()));
 
   return true;
 }

chrome/browser/chromeos/tether/tether_service.h

@@ -17,6 +17,7 @@
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/network_state_handler_observer.h"
 #include "chromeos/services/device_sync/public/cpp/device_sync_client.h"
+#include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
 #include "components/cryptauth/cryptauth_device_manager.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/prefs/pref_change_registrar.h"
@@ -55,13 +56,15 @@ class PrefRegistrySyncable;
 //
 // This service starts up when the user logs in (or recovers from a crash) and
 // is shut down when the user logs out.
-class TetherService : public KeyedService,
-                      public chromeos::PowerManagerClient::Observer,
-                      public chromeos::tether::TetherHostFetcher::Observer,
-                      public device::BluetoothAdapter::Observer,
-                      public chromeos::NetworkStateHandlerObserver,
-                      public chromeos::tether::TetherComponent::Observer,
-                      public chromeos::device_sync::DeviceSyncClient::Observer {
+class TetherService
+    : public KeyedService,
+      public chromeos::PowerManagerClient::Observer,
+      public chromeos::tether::TetherHostFetcher::Observer,
+      public device::BluetoothAdapter::Observer,
+      public chromeos::NetworkStateHandlerObserver,
+      public chromeos::tether::TetherComponent::Observer,
+      public chromeos::device_sync::DeviceSyncClient::Observer,
+      public chromeos::multidevice_setup::MultiDeviceSetupClient::Observer {
  public:
   TetherService(
       Profile* profile,
@@ -69,6 +72,8 @@ class TetherService : public KeyedService,
       cryptauth::CryptAuthService* cryptauth_service,
       chromeos::device_sync::DeviceSyncClient* device_sync_client,
       chromeos::secure_channel::SecureChannelClient* secure_channel_client,
+      chromeos::multidevice_setup::MultiDeviceSetupClient*
+          multidevice_setup_client,
       chromeos::NetworkStateHandler* network_state_handler,
       session_manager::SessionManager* session_manager);
   ~TetherService() override;
@@ -118,6 +123,11 @@ class TetherService : public KeyedService,
   // chromeos::device_sync::DeviceSyncClient::Observer:
   void OnReady() override;
 
+  // chromeos::multidevice_setup::MultiDeviceSetupClient::Observer:
+  void OnHostStatusChanged(
+      chromeos::multidevice_setup::mojom::HostStatus host_status,
+      const base::Optional<cryptauth::RemoteDeviceRef>& host_device) override;
+
   // Callback when the controlling pref changes.
   void OnPrefsChanged();
 
@@ -139,6 +149,11 @@ class TetherService : public KeyedService,
   friend class TetherServiceTest;
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest, TestSuspend);
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest, TestDeviceSyncClientNotReady);
+  FRIEND_TEST_ALL_PREFIXES(
+      TetherServiceTest,
+      TestMultiDeviceSetupClientInitiallyHasNoVerifiedHost);
+  FRIEND_TEST_ALL_PREFIXES(TetherServiceTest,
+                           TestMultiDeviceSetupClientLosesVerifiedHost);
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest, TestBleAdvertisingNotSupported);
   FRIEND_TEST_ALL_PREFIXES(
       TetherServiceTest,
@@ -153,6 +168,9 @@ class TetherService : public KeyedService,
   FRIEND_TEST_ALL_PREFIXES(
       TetherServiceTest,
       TestGet_PrimaryUser_FeatureFlagEnabled_MultiDeviceApiFlagEnabled);
+  FRIEND_TEST_ALL_PREFIXES(
+      TetherServiceTest,
+      TestGet_PrimaryUser_FeatureFlagEnabled_MultiDeviceApiAndMultiDeviceSetupFlagsEnabled);
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest, TestNoTetherHosts);
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest, TestProhibitedByPolicy);
   FRIEND_TEST_ALL_PREFIXES(TetherServiceTest, TestIsBluetoothPowered);
@@ -183,6 +201,7 @@ class TetherService : public KeyedService,
     BLE_NOT_PRESENT = 9,
     WIFI_NOT_PRESENT = 10,
     SUSPENDED = 11,
+    MULTIDEVICE_HOST_UNVERIFIED = 12,
     TETHER_FEATURE_STATE_MAX
   };
 
@@ -190,6 +209,11 @@ class TetherService : public KeyedService,
   static std::string TetherFeatureStateToString(
       const TetherFeatureState& state);
 
+  void OnHostStatusReceived(
+      chromeos::multidevice_setup::mojom::HostStatus host_status,
+      const base::Optional<cryptauth::RemoteDeviceRef>& host_device);
+
+  void GetBluetoothAdapter();
   void OnBluetoothAdapterFetched(
       scoped_refptr<device::BluetoothAdapter> adapter);
   void OnBluetoothAdapterAdvertisingIntervalSet();
@@ -247,6 +271,9 @@ class TetherService : public KeyedService,
   // was closed).
   bool suspended_ = false;
 
+  chromeos::multidevice_setup::mojom::HostStatus host_status_ =
+      chromeos::multidevice_setup::mojom::HostStatus::kNoEligibleHosts;
+
   // Whether the BLE advertising interval has attempted to be set during this
   // session.
   bool has_attempted_to_set_ble_advertising_interval_ = false;
@@ -271,6 +298,8 @@ class TetherService : public KeyedService,
   cryptauth::CryptAuthService* cryptauth_service_;
   chromeos::device_sync::DeviceSyncClient* device_sync_client_;
   chromeos::secure_channel::SecureChannelClient* secure_channel_client_;
+  chromeos::multidevice_setup::MultiDeviceSetupClient*
+      multidevice_setup_client_;
   chromeos::NetworkStateHandler* network_state_handler_;
   session_manager::SessionManager* session_manager_;
   std::unique_ptr<chromeos::tether::NotificationPresenter>

chrome/browser/chromeos/tether/tether_service_factory.cc

@@ -9,6 +9,7 @@
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/chromeos/cryptauth/chrome_cryptauth_service_factory.h"
 #include "chrome/browser/chromeos/device_sync/device_sync_client_factory.h"
+#include "chrome/browser/chromeos/multidevice_setup/multidevice_setup_client_factory.h"
 #include "chrome/browser/chromeos/secure_channel/secure_channel_client_provider.h"
 #include "chrome/browser/chromeos/tether/fake_tether_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ -40,6 +41,8 @@ TetherServiceFactory::TetherServiceFactory()
           BrowserContextDependencyManager::GetInstance()) {
   DependsOn(chromeos::ChromeCryptAuthServiceFactory::GetInstance());
   DependsOn(chromeos::device_sync::DeviceSyncClientFactory::GetInstance());
+  DependsOn(chromeos::multidevice_setup::MultiDeviceSetupClientFactory::
+                GetInstance());
 }
 
 TetherServiceFactory::~TetherServiceFactory() {}
@@ -59,6 +62,8 @@ KeyedService* TetherServiceFactory::BuildServiceInstanceFor(
             Profile::FromBrowserContext(context)),
         chromeos::secure_channel::SecureChannelClientProvider::GetInstance()
             ->GetClient(),
+        chromeos::multidevice_setup::MultiDeviceSetupClientFactory::
+            GetForProfile(Profile::FromBrowserContext(context)),
         chromeos::NetworkHandler::Get()->network_state_handler(),
         session_manager::SessionManager::Get());
 
@@ -80,6 +85,8 @@ KeyedService* TetherServiceFactory::BuildServiceInstanceFor(
           Profile::FromBrowserContext(context)),
       chromeos::secure_channel::SecureChannelClientProvider::GetInstance()
           ->GetClient(),
+      chromeos::multidevice_setup::MultiDeviceSetupClientFactory::GetForProfile(
+          Profile::FromBrowserContext(context)),
       chromeos::NetworkHandler::Get()->network_state_handler(),
       session_manager::SessionManager::Get());
 }

chromeos/components/tether/tether_component.h

@@ -32,7 +32,8 @@ class TetherComponent {
     PREF_DISABLED = 5,
     BLUETOOTH_DISABLED = 6,
     CELLULAR_DISABLED = 7,
-    BLUETOOTH_CONTROLLER_DISAPPEARED = 8
+    BLUETOOTH_CONTROLLER_DISAPPEARED = 8,
+    MULTIDEVICE_HOST_UNVERIFIED = 9
   };
 
   TetherComponent();

chromeos/components/tether/tether_component_impl.cc

@@ -55,6 +55,9 @@ GetSessionCompletionReasonFromShutdownReason(
     case TetherComponent::ShutdownReason::CELLULAR_DISABLED:
       return TetherSessionCompletionLogger::SessionCompletionReason::
           CELLULAR_DISABLED;
+    case TetherComponent::ShutdownReason::MULTIDEVICE_HOST_UNVERIFIED:
+      return TetherSessionCompletionLogger::SessionCompletionReason::
+          MULTIDEVICE_HOST_UNVERIFIED;
     default:
       break;
   }

chromeos/components/tether/tether_session_completion_logger.h

@@ -25,6 +25,7 @@ class TetherSessionCompletionLogger {
     CELLULAR_DISABLED = 7,
     WIFI_DISABLED = 8,
     BLUETOOTH_CONTROLLER_DISAPPEARED = 9,
+    MULTIDEVICE_HOST_UNVERIFIED = 10,
     SESSION_COMPLETION_REASON_MAX
   };
 

chromeos/components/tether/tether_session_completion_logger_unittest.cc

@@ -89,6 +89,12 @@ TEST_F(TetherSessionCompletionLoggerTest, TestBluetoothControllerDisappeared) {
           BLUETOOTH_CONTROLLER_DISAPPEARED);
 }
 
+TEST_F(TetherSessionCompletionLoggerTest, TestMultiDeviceHostUnverified) {
+  TestSessionCompletionReasonRecorded(
+      TetherSessionCompletionLogger::SessionCompletionReason::
+          MULTIDEVICE_HOST_UNVERIFIED);
+}
+
 }  // namespace tether
 
 }  // namespace chromeos

chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.cc

@@ -10,7 +10,13 @@ namespace multidevice_setup {
 
 FakeMultiDeviceSetupClient::FakeMultiDeviceSetupClient() = default;
 
-FakeMultiDeviceSetupClient::~FakeMultiDeviceSetupClient() = default;
+FakeMultiDeviceSetupClient::~FakeMultiDeviceSetupClient() {
+  DCHECK(get_eligible_host_devices_callback_queue_.empty());
+  DCHECK(set_host_device_public_key_and_callback_queue_.empty());
+  DCHECK(get_host_status_callback_queue_.empty());
+  DCHECK(retry_set_host_now_callback_queue_.empty());
+  DCHECK(trigger_event_for_debugging_type_and_callback_queue_.empty());
+}
 
 void FakeMultiDeviceSetupClient::InvokePendingGetEligibleHostDevicesCallback(
     const cryptauth::RemoteDeviceRefList& eligible_devices) {

tools/metrics/histograms/enums.xml

@@ -25313,6 +25313,7 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="9" label="BLE not present"/>
   <int value="10" label="WiFi not present"/>
   <int value="11" label="Suspended"/>
+  <int value="12" label="MultiDevice host unverified"/>
 </enum>
 
 <enum name="InstantTethering_HostScanResult">
@@ -25341,6 +25342,7 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="7" label="Cellular disabled"/>
   <int value="8" label="Wi-Fi disabled"/>
   <int value="9" label="Bluetooth controller disappeared"/>
+  <int value="10" label="MultiDevice host unverified"/>
 </enum>
 
 <enum name="IntelMaxMicroArchitecture">