Use MultideviceSetupClient to fetch tether hosts.

This will limit Magic Tether hosts to devices that have gone through
Better Together Setup.

Bug: 870072
Change-Id: I9fe120f4b8d07f831ebe84aeb28267ff379f2e82
Reviewed-on: https://chromium-review.googlesource.com/1180473Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Jeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584617}

chrome/browser/chromeos/tether/tether_service.cc

@@ -158,7 +158,8 @@ TetherService::TetherService(
       tether_host_fetcher_(
           chromeos::tether::TetherHostFetcherImpl::Factory::NewInstance(
               remote_device_provider_.get(),
-              device_sync_client_)),
+              device_sync_client_,
+              multidevice_setup_client_)),
       timer_(std::make_unique<base::OneShotTimer>()),
       weak_ptr_factory_(this) {
   tether_host_fetcher_->AddObserver(this);

chrome/browser/chromeos/tether/tether_service_unittest.cc

@@ -245,7 +245,9 @@ class FakeTetherHostFetcherFactory
   // chromeos::tether::TetherHostFetcherImpl::Factory :
   std::unique_ptr<chromeos::tether::TetherHostFetcher> BuildInstance(
       cryptauth::RemoteDeviceProvider* remote_device_provider,
-      chromeos::device_sync::DeviceSyncClient* device_sync_client) override {
+      chromeos::device_sync::DeviceSyncClient* device_sync_client,
+      chromeos::multidevice_setup::MultiDeviceSetupClient*
+          multidevice_setup_client) override {
     last_created_ =
         new chromeos::tether::FakeTetherHostFetcher(initial_devices_);
     return base::WrapUnique(last_created_);

chromeos/components/tether/BUILD.gn

@@ -136,6 +136,8 @@ static_library("tether") {
     "//chromeos/components/proximity_auth/logging",
     "//chromeos/components/tether/proto",
     "//chromeos/services/device_sync/public/cpp",
+    "//chromeos/services/multidevice_setup/public/cpp:cpp",
+    "//chromeos/services/multidevice_setup/public/mojom:mojom",
     "//chromeos/services/secure_channel/public/cpp/client",
     "//chromeos/services/secure_channel/public/mojom",
 
@@ -303,6 +305,8 @@ source_set("unit_tests") {
     # TODO(hansberry): Remove //chromeos/services/secure_channel dependency when
     # SecureChannelClient migration is complete.
     "//chromeos/services/device_sync/public/mojom",
+    "//chromeos/services/multidevice_setup/public/cpp:test_support",
+    "//chromeos/services/multidevice_setup/public/mojom:mojom",
     "//chromeos/services/secure_channel",
     "//chromeos/services/secure_channel:test_support",
     "//chromeos/services/secure_channel/public/cpp/client:test_support",

chromeos/components/tether/tether_host_fetcher_impl.cc

@@ -8,6 +8,7 @@
 
 #include "base/memory/ptr_util.h"
 #include "chromeos/chromeos_features.h"
+#include "chromeos/services/multidevice_setup/public/cpp/multidevice_setup_client.h"
 #include "components/cryptauth/remote_device.h"
 #include "components/cryptauth/remote_device_provider.h"
 
@@ -22,12 +23,14 @@ TetherHostFetcherImpl::Factory*
 // static
 std::unique_ptr<TetherHostFetcher> TetherHostFetcherImpl::Factory::NewInstance(
     cryptauth::RemoteDeviceProvider* remote_device_provider,
-    device_sync::DeviceSyncClient* device_sync_client) {
+    device_sync::DeviceSyncClient* device_sync_client,
+    chromeos::multidevice_setup::MultiDeviceSetupClient*
+        multidevice_setup_client) {
   if (!factory_instance_) {
     factory_instance_ = new Factory();
   }
-  return factory_instance_->BuildInstance(remote_device_provider,
-                                          device_sync_client);
+  return factory_instance_->BuildInstance(
+      remote_device_provider, device_sync_client, multidevice_setup_client);
 }
 
 // static
@@ -38,16 +41,22 @@ void TetherHostFetcherImpl::Factory::SetInstanceForTesting(Factory* factory) {
 std::unique_ptr<TetherHostFetcher>
 TetherHostFetcherImpl::Factory::BuildInstance(
     cryptauth::RemoteDeviceProvider* remote_device_provider,
-    device_sync::DeviceSyncClient* device_sync_client) {
-  return base::WrapUnique(
-      new TetherHostFetcherImpl(remote_device_provider, device_sync_client));
+    device_sync::DeviceSyncClient* device_sync_client,
+    chromeos::multidevice_setup::MultiDeviceSetupClient*
+        multidevice_setup_client) {
+  return base::WrapUnique(new TetherHostFetcherImpl(
+      remote_device_provider, device_sync_client, multidevice_setup_client));
 }
 
 TetherHostFetcherImpl::TetherHostFetcherImpl(
     cryptauth::RemoteDeviceProvider* remote_device_provider,
-    device_sync::DeviceSyncClient* device_sync_client)
+    device_sync::DeviceSyncClient* device_sync_client,
+    chromeos::multidevice_setup::MultiDeviceSetupClient*
+        multidevice_setup_client)
     : remote_device_provider_(remote_device_provider),
-      device_sync_client_(device_sync_client) {
+      device_sync_client_(device_sync_client),
+      multidevice_setup_client_(multidevice_setup_client),
+      weak_ptr_factory_(this) {
   if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
     device_sync_client_->AddObserver(this);
   else
@@ -91,23 +100,24 @@ void TetherHostFetcherImpl::CacheCurrentTetherHosts() {
   cryptauth::RemoteDeviceRefList updated_list;
 
   if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
-    for (const auto& remote_device : device_sync_client_->GetSyncedDevices()) {
-      if (remote_device.GetSoftwareFeatureState(
-              cryptauth::SoftwareFeature::MAGIC_TETHER_HOST) ==
-          cryptauth::SoftwareFeatureState::kSupported)
-        updated_list.push_back(remote_device);
-    }
-  } else {
-    for (const auto& remote_device :
-         remote_device_provider_->GetSyncedDevices()) {
-      if (base::ContainsKey(remote_device.software_features,
-                            cryptauth::SoftwareFeature::MAGIC_TETHER_HOST) &&
-          remote_device.software_features.at(
-              cryptauth::SoftwareFeature::MAGIC_TETHER_HOST) ==
-              cryptauth::SoftwareFeatureState::kSupported) {
-        updated_list.push_back(cryptauth::RemoteDeviceRef(
-            std::make_shared<cryptauth::RemoteDevice>(remote_device)));
-      }
+    multidevice_setup_client_->GetHostStatus(
+        base::BindOnce(&TetherHostFetcherImpl::OnHostStatusFetched,
+                       weak_ptr_factory_.GetWeakPtr()));
+    return;
+  }
+
+  for (const auto& remote_device :
+       remote_device_provider_->GetSyncedDevices()) {
+    if (base::ContainsKey(remote_device.software_features,
+                          cryptauth::SoftwareFeature::MAGIC_TETHER_HOST) &&
+        (remote_device.software_features.at(
+             cryptauth::SoftwareFeature::MAGIC_TETHER_HOST) ==
+             cryptauth::SoftwareFeatureState::kSupported ||
+         remote_device.software_features.at(
+             cryptauth::SoftwareFeature::MAGIC_TETHER_HOST) ==
+             cryptauth::SoftwareFeatureState::kEnabled)) {
+      updated_list.push_back(cryptauth::RemoteDeviceRef(
+          std::make_shared<cryptauth::RemoteDevice>(remote_device)));
     }
   }
 
@@ -118,6 +128,24 @@ void TetherHostFetcherImpl::CacheCurrentTetherHosts() {
   NotifyTetherHostsUpdated();
 }
 
+void TetherHostFetcherImpl::OnHostStatusFetched(
+    chromeos::multidevice_setup::mojom::HostStatus host_status,
+    const base::Optional<cryptauth::RemoteDeviceRef>& host_device) {
+  DCHECK(base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi));
+  cryptauth::RemoteDeviceRefList updated_list;
+
+  if (host_status ==
+      chromeos::multidevice_setup::mojom::HostStatus::kHostVerified) {
+    updated_list.push_back(*host_device);
+  }
+
+  if (updated_list == current_remote_device_list_)
+    return;
+
+  current_remote_device_list_.swap(updated_list);
+  NotifyTetherHostsUpdated();
+}
+
 }  // namespace tether
 
 }  // namespace chromeos

chromeos/components/tether/tether_host_fetcher_impl.h

@@ -8,8 +8,10 @@
 #include <memory>
 
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "chromeos/components/tether/tether_host_fetcher.h"
 #include "chromeos/services/device_sync/public/cpp/device_sync_client.h"
+#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 #include "components/cryptauth/remote_device_provider.h"
 #include "components/cryptauth/remote_device_ref.h"
 
@@ -19,6 +21,10 @@ class RemoteDeviceProvider;
 
 namespace chromeos {
 
+namespace multidevice_setup {
+class MultiDeviceSetupClient;
+}  // namespace multidevice_setup
+
 namespace tether {
 
 // Concrete TetherHostFetcher implementation. Despite the asynchronous function
@@ -39,14 +45,18 @@ class TetherHostFetcherImpl : public TetherHostFetcher,
    public:
     static std::unique_ptr<TetherHostFetcher> NewInstance(
         cryptauth::RemoteDeviceProvider* remote_device_provider,
-        device_sync::DeviceSyncClient* device_sync_client);
+        device_sync::DeviceSyncClient* device_sync_client,
+        chromeos::multidevice_setup::MultiDeviceSetupClient*
+            multidevice_setup_client);
 
     static void SetInstanceForTesting(Factory* factory);
 
    protected:
     virtual std::unique_ptr<TetherHostFetcher> BuildInstance(
         cryptauth::RemoteDeviceProvider* remote_device_provider,
-        device_sync::DeviceSyncClient* device_sync_client);
+        device_sync::DeviceSyncClient* device_sync_client,
+        chromeos::multidevice_setup::MultiDeviceSetupClient*
+            multidevice_setup_client);
 
    private:
     static Factory* factory_instance_;
@@ -70,15 +80,23 @@ class TetherHostFetcherImpl : public TetherHostFetcher,
   // TODO(crbug.com/848956): Remove RemoteDeviceProvider once all clients have
   // migrated to the DeviceSync Mojo API.
   TetherHostFetcherImpl(cryptauth::RemoteDeviceProvider* remote_device_provider,
-                        device_sync::DeviceSyncClient* device_sync_client);
+                        device_sync::DeviceSyncClient* device_sync_client,
+                        chromeos::multidevice_setup::MultiDeviceSetupClient*
+                            multidevice_setup_client_);
 
  private:
   void CacheCurrentTetherHosts();
+  void OnHostStatusFetched(
+      chromeos::multidevice_setup::mojom::HostStatus host_status,
+      const base::Optional<cryptauth::RemoteDeviceRef>& host_device);
 
   cryptauth::RemoteDeviceProvider* remote_device_provider_;
   device_sync::DeviceSyncClient* device_sync_client_;
+  chromeos::multidevice_setup::MultiDeviceSetupClient*
+      multidevice_setup_client_;
 
   cryptauth::RemoteDeviceRefList current_remote_device_list_;
+  base::WeakPtrFactory<TetherHostFetcherImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(TetherHostFetcherImpl);
 };

chromeos/components/tether/tether_host_fetcher_impl_unittest.cc

@@ -13,6 +13,8 @@
 #include "base/test/scoped_feature_list.h"
 #include "chromeos/chromeos_features.h"
 #include "chromeos/services/device_sync/public/cpp/fake_device_sync_client.h"
+#include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
+#include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
 #include "components/cryptauth/fake_remote_device_provider.h"
 #include "components/cryptauth/remote_device.h"
 #include "components/cryptauth/remote_device_ref.h"
@@ -55,6 +57,8 @@ class TetherHostFetcherImplTest : public testing::Test {
         std::make_unique<cryptauth::FakeRemoteDeviceProvider>();
     fake_device_sync_client_ =
         std::make_unique<device_sync::FakeDeviceSyncClient>();
+    fake_multidevice_setup_client_ = std::make_unique<
+        chromeos::multidevice_setup::FakeMultiDeviceSetupClient>();
   }
 
   void TearDown() override {
@@ -77,11 +81,32 @@ class TetherHostFetcherImplTest : public testing::Test {
             : fake_remote_device_provider_.get(),
         base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)
             ? fake_device_sync_client_.get()
+            : nullptr,
+        base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)
+            ? fake_multidevice_setup_client_.get()
             : nullptr);
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(test_remote_device_list_[0]);
     test_observer_ = std::make_unique<TestObserver>();
     tether_host_fetcher_->AddObserver(test_observer_.get());
   }
 
+  void InvokeGetHostStatusCallback(
+      const base::Optional<cryptauth::RemoteDevice>& host_device) {
+    EXPECT_TRUE(
+        base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi));
+    if (!host_device) {
+      fake_multidevice_setup_client_->InvokePendingGetHostStatusCallback(
+          chromeos::multidevice_setup::mojom::HostStatus::kNoEligibleHosts,
+          base::nullopt);
+    } else {
+      fake_multidevice_setup_client_->InvokePendingGetHostStatusCallback(
+          chromeos::multidevice_setup::mojom::HostStatus::kHostVerified,
+          cryptauth::RemoteDeviceRef(
+              std::make_shared<cryptauth::RemoteDevice>(*host_device)));
+    }
+  }
+
   void OnTetherHostListFetched(
       const cryptauth::RemoteDeviceRefList& device_list) {
     last_fetched_list_ = device_list;
@@ -121,6 +146,10 @@ class TetherHostFetcherImplTest : public testing::Test {
           cryptauth::SoftwareFeatureState::kSupported;
     }
 
+    // Mark the first device enabled instead of supported.
+    list[0].software_features[cryptauth::SoftwareFeature::MAGIC_TETHER_HOST] =
+        cryptauth::SoftwareFeatureState::kEnabled;
+
     return list;
   }
 
@@ -159,24 +188,32 @@ class TetherHostFetcherImplTest : public testing::Test {
     // Update the list of devices to be empty.
     SetSyncedDevices(cryptauth::RemoteDeviceList());
     NotifyNewDevicesSynced();
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(base::nullopt);
     EXPECT_FALSE(tether_host_fetcher_->HasSyncedTetherHosts());
     EXPECT_EQ(1u, test_observer_->num_updates());
 
     // Notify that the list has changed, even though it hasn't. There should be
     // no update.
     NotifyNewDevicesSynced();
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(base::nullopt);
     EXPECT_FALSE(tether_host_fetcher_->HasSyncedTetherHosts());
     EXPECT_EQ(1u, test_observer_->num_updates());
 
     // Update the list to include device 0 only.
     SetSyncedDevices({test_remote_device_list_[0]});
     NotifyNewDevicesSynced();
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(test_remote_device_list_[0]);
     EXPECT_TRUE(tether_host_fetcher_->HasSyncedTetherHosts());
     EXPECT_EQ(2u, test_observer_->num_updates());
 
     // Notify that the list has changed, even though it hasn't. There should be
     // no update.
     NotifyNewDevicesSynced();
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(test_remote_device_list_[0]);
     EXPECT_TRUE(tether_host_fetcher_->HasSyncedTetherHosts());
     EXPECT_EQ(2u, test_observer_->num_updates());
   }
@@ -191,6 +228,8 @@ class TetherHostFetcherImplTest : public testing::Test {
     // requested.
     SetSyncedDevices(cryptauth::RemoteDeviceList{test_remote_device_list_[0]});
     NotifyNewDevicesSynced();
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(test_remote_device_list_[0]);
     VerifySingleTetherHost(test_remote_device_ref_list_[0].GetDeviceId(),
                            test_remote_device_ref_list_[0]);
 
@@ -203,12 +242,16 @@ class TetherHostFetcherImplTest : public testing::Test {
 
     SetSyncedDevices(cryptauth::RemoteDeviceList{remote_device});
     NotifyNewDevicesSynced();
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(remote_device);
     VerifySingleTetherHost(test_remote_device_ref_list_[0].GetDeviceId(),
                            base::nullopt);
 
     // Update the list; now, there are no more devices.
     SetSyncedDevices(cryptauth::RemoteDeviceList());
     NotifyNewDevicesSynced();
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
+      InvokeGetHostStatusCallback(base::nullopt);
     VerifySingleTetherHost(test_remote_device_ref_list_[0].GetDeviceId(),
                            base::nullopt);
   }
@@ -232,7 +275,13 @@ class TetherHostFetcherImplTest : public testing::Test {
 
     SetSyncedDevices(test_remote_device_list_);
     NotifyNewDevicesSynced();
-    VerifyAllTetherHosts(host_device_list);
+    if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi)) {
+      InvokeGetHostStatusCallback(test_remote_device_list_[0]);
+      VerifyAllTetherHosts(
+          CreateTestRemoteDeviceRefList({test_remote_device_list_[0]}));
+    } else {
+      VerifyAllTetherHosts(host_device_list);
+    }
   }
 
   cryptauth::RemoteDeviceList test_remote_device_list_;
@@ -245,6 +294,8 @@ class TetherHostFetcherImplTest : public testing::Test {
   std::unique_ptr<cryptauth::FakeRemoteDeviceProvider>
       fake_remote_device_provider_;
   std::unique_ptr<device_sync::FakeDeviceSyncClient> fake_device_sync_client_;
+  std::unique_ptr<chromeos::multidevice_setup::FakeMultiDeviceSetupClient>
+      fake_multidevice_setup_client_;
 
   std::unique_ptr<TetherHostFetcher> tether_host_fetcher_;