Revert "Make Instant Tethering work with legacy hosts before hosts update."

This reverts commit 1c6c9d1b.

Reason for revert: Breaks tests when combined with https://chromium-review.googlesource.com/c/chromium/src/+/1260491. Doesn't properly wait for devicesync service to be ready before querying the list of devices.

Original change's description:
> Make Instant Tethering work with legacy hosts before hosts update.
> 
> The phone side flags and enrollment will be slower than the ChromeOS
> rollout. In order to avoid breaking Instant Tethering in the meantime,
> look for cases where:
> 
> 1) There are no host devices reporting BETTER_TOGETHER_HOST support.
> 2) There is at least one host device reporting INSTANT_TETHER_HOST
> support.
> 
> In this scenario, the only possiblity is that this host device has not
> reenrolled yet after receiving the new flags.
> 
> Bug: 893769
> Change-Id: Ib7fd0acdbe80903a499dd0dd42fd60365304371d
> Reviewed-on: https://chromium-review.googlesource.com/c/1277272
> Commit-Queue: Jeremy Klein <jlklein@chromium.org>
> Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599058}

TBR=khorimoto@chromium.org,jlklein@chromium.org

Change-Id: Iee7a71e551e875764f38cdf72391378cdacc80bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 893769
Reviewed-on: https://chromium-review.googlesource.com/c/1278541Reviewed-by: Jeremy Klein <jlklein@chromium.org>
Commit-Queue: Jeremy Klein <jlklein@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599208}

chrome/browser/chromeos/tether/tether_service.cc

@@ -684,15 +684,6 @@ TetherService::TetherFeatureState TetherService::GetTetherFeatureState() {
       case chromeos::multidevice_setup::mojom::FeatureState::
           kUnavailableSuiteDisabled:
         return BETTER_TOGETHER_SUITE_DISABLED;
-      case chromeos::multidevice_setup::mojom::FeatureState::
-          kUnavailableNoVerifiedHost:
-        // Note that because of the early return above after
-        // !HasSyncedTetherHosts, if this point is hit, there are synced tether
-        // hosts available, but the multidevice state is unverified. This switch
-        // case can only occur for legacy Magic Tether hosts, in which case the
-        // service should be enabled.
-        // TODO(crbug.com/894585): Remove this legacy special case after M71.
-        return ENABLED;
       case chromeos::multidevice_setup::mojom::FeatureState::
           kNotSupportedByChromebook:
         // CryptAuth may not yet know that this device supports
@@ -703,6 +694,10 @@ TetherService::TetherFeatureState TetherService::GetTetherFeatureState() {
         FALLTHROUGH;
       case chromeos::multidevice_setup::mojom::FeatureState::
           kNotSupportedByPhone:
+        FALLTHROUGH;
+      case chromeos::multidevice_setup::mojom::FeatureState::
+          kUnavailableNoVerifiedHost:
+        no_available_hosts_false_positive_encountered_ = true;
         return NO_AVAILABLE_HOSTS;
       default:
         // Other FeatureStates:

chrome/browser/chromeos/tether/tether_service_unittest.cc

@@ -77,9 +77,6 @@ cryptauth::RemoteDeviceRefList CreateTestDevices() {
   for (size_t i = 0; i < kNumTestDevices; ++i) {
     list.push_back(cryptauth::RemoteDeviceRefBuilder()
                        .SetSupportsMobileHotspot(true)
-                       .SetSoftwareFeatureState(
-                           cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST,
-                           cryptauth::SoftwareFeatureState::kSupported)
                        .Build());
   }
   return list;
@@ -718,7 +715,6 @@ TEST_F(TetherServiceTest, TestDeviceSyncClientNotReady) {
 
 TEST_F(TetherServiceTest,
        TestMultiDeviceSetupClientInitiallyHasNoVerifiedHost) {
-  fake_tether_host_fetcher_factory_->SetNoInitialDevices();
   base::test::ScopedFeatureList feature_list;
   feature_list.InitWithFeatures(
       {chromeos::features::kMultiDeviceApi,
@@ -737,8 +733,6 @@ TEST_F(TetherServiceTest,
           chromeos::NetworkTypePattern::Tether()));
   VerifyTetherActiveStatus(false /* expected_active */);
 
-  fake_tether_host_fetcher_factory_->last_created()->set_tether_hosts(
-      test_devices_);
   fake_multidevice_setup_client_->SetFeatureState(
       chromeos::multidevice_setup::mojom::Feature::kInstantTethering,
       chromeos::multidevice_setup::mojom::FeatureState::kEnabledByUser);
@@ -768,7 +762,6 @@ TEST_F(TetherServiceTest, TestMultiDeviceSetupClientLosesVerifiedHost) {
                 chromeos::NetworkTypePattern::Tether()));
   VerifyTetherActiveStatus(true /* expected_active */);
 
-  fake_tether_host_fetcher_factory_->last_created()->set_tether_hosts({});
   fake_multidevice_setup_client_->SetFeatureState(
       chromeos::multidevice_setup::mojom::Feature::kInstantTethering,
       chromeos::multidevice_setup::mojom::FeatureState::
@@ -780,7 +773,6 @@ TEST_F(TetherServiceTest, TestMultiDeviceSetupClientLosesVerifiedHost) {
           chromeos::NetworkTypePattern::Tether()));
   VerifyTetherActiveStatus(false /* expected_active */);
 
-  mock_timer_->Fire();
   ShutdownTetherService();
   VerifyTetherFeatureStateRecorded(
       TetherService::TetherFeatureState::NO_AVAILABLE_HOSTS,

chromeos/components/tether/tether_host_fetcher_impl.cc

@@ -15,6 +15,39 @@ namespace chromeos {
 
 namespace tether {
 
+namespace {
+
+enum class TetherHostSource {
+  UNKNOWN,
+  MULTIDEVICE_SETUP_CLIENT,
+  DEVICE_SYNC_CLIENT,
+  REMOTE_DEVICE_PROVIDER
+};
+
+TetherHostSource GetTetherHostSourceBasedOnFlags() {
+  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
+      base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
+    return TetherHostSource::MULTIDEVICE_SETUP_CLIENT;
+  }
+  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
+      !base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
+    return TetherHostSource::DEVICE_SYNC_CLIENT;
+  }
+  if (!base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
+      !base::FeatureList::IsEnabled(
+          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
+    return TetherHostSource::REMOTE_DEVICE_PROVIDER;
+  }
+  NOTREACHED() << "TetherHostFetcherImpl: Unexpected feature flag state of "
+               << "kMultiDeviceApi disabled and kEnableUnifiedMultiDeviceSetup "
+               << "enabled.";
+  return TetherHostSource::UNKNOWN;
+}
+
+}  // namespace
+
 // static
 TetherHostFetcherImpl::Factory*
     TetherHostFetcherImpl::Factory::factory_instance_ = nullptr;
@@ -62,10 +95,6 @@ TetherHostFetcherImpl::TetherHostFetcherImpl(
       break;
     case TetherHostSource::DEVICE_SYNC_CLIENT:
       device_sync_client_->AddObserver(this);
-      if (IsInLegacyHostMode()) {
-        multidevice_setup_client_->AddObserver(this);
-        started_in_legacy_mode_ = true;
-      }
       break;
     case TetherHostSource::REMOTE_DEVICE_PROVIDER:
       remote_device_provider_->AddObserver(this);
@@ -80,13 +109,9 @@ TetherHostFetcherImpl::~TetherHostFetcherImpl() {
   switch (GetTetherHostSourceBasedOnFlags()) {
     case TetherHostSource::MULTIDEVICE_SETUP_CLIENT:
       multidevice_setup_client_->RemoveObserver(this);
-      if (started_in_legacy_mode_)
-        device_sync_client_->RemoveObserver(this);
       break;
     case TetherHostSource::DEVICE_SYNC_CLIENT:
       device_sync_client_->RemoveObserver(this);
-      if (started_in_legacy_mode_)
-        multidevice_setup_client_->RemoveObserver(this);
       break;
     case TetherHostSource::REMOTE_DEVICE_PROVIDER:
       remote_device_provider_->RemoveObserver(this);
@@ -193,61 +218,6 @@ cryptauth::RemoteDeviceRefList TetherHostFetcherImpl::GenerateHostDeviceList() {
   return host_list;
 }
 
-bool TetherHostFetcherImpl::IsInLegacyHostMode() {
-  if (!base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi))
-    return false;
-
-  bool has_supported_tether_host = false;
-  for (const cryptauth::RemoteDeviceRef& remote_device_ref :
-       device_sync_client_->GetSyncedDevices()) {
-    cryptauth::SoftwareFeatureState better_together_host_state =
-        remote_device_ref.GetSoftwareFeatureState(
-            cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST);
-    // If there's any valid Better Together host, don't support legacy mode.
-    if (better_together_host_state ==
-            cryptauth::SoftwareFeatureState::kSupported ||
-        better_together_host_state ==
-            cryptauth::SoftwareFeatureState::kEnabled) {
-      return false;
-    }
-
-    cryptauth::SoftwareFeatureState magic_tether_host_state =
-        remote_device_ref.GetSoftwareFeatureState(
-            cryptauth::SoftwareFeature::MAGIC_TETHER_HOST);
-    if (magic_tether_host_state ==
-            cryptauth::SoftwareFeatureState::kSupported ||
-        magic_tether_host_state == cryptauth::SoftwareFeatureState::kEnabled) {
-      has_supported_tether_host = true;
-    }
-  }
-
-  return has_supported_tether_host;
-}
-
-TetherHostFetcherImpl::TetherHostSource
-TetherHostFetcherImpl::GetTetherHostSourceBasedOnFlags() {
-  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
-      !base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
-    return TetherHostSource::DEVICE_SYNC_CLIENT;
-  }
-  if (!base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
-      !base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
-    return TetherHostSource::REMOTE_DEVICE_PROVIDER;
-  }
-  if (base::FeatureList::IsEnabled(chromeos::features::kMultiDeviceApi) &&
-      base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
-    return IsInLegacyHostMode() ? TetherHostSource::DEVICE_SYNC_CLIENT
-                                : TetherHostSource::MULTIDEVICE_SETUP_CLIENT;
-  }
-  NOTREACHED() << "TetherHostFetcherImpl: Unexpected feature flag state of "
-               << "kMultiDeviceApi disabled and kEnableUnifiedMultiDeviceSetup "
-               << "enabled.";
-  return TetherHostSource::UNKNOWN;
-}
-
 }  // namespace tether
 
 }  // namespace chromeos

chromeos/components/tether/tether_host_fetcher_impl.h

@@ -30,11 +30,12 @@ namespace tether {
 // Note: TetherHostFetcherImpl, and the Tether feature as a whole, is currently
 // in the middle of a migration from using RemoteDeviceProvider to
 // DeviceSyncClient and eventually to MultiDeviceSetupClient. Its constructor
-// accepts all three objects, but some may be null. (This is controlled at a
-// higher level by features::kMultiDeviceApi and
-// features::kEnableUnifiedMultiDeviceSetup.). Once Tether has been fully
-// migrated, RemoteDeviceProvider and eventually DeviceSyncClient will be ripped
-// out of this class. See https://crbug.com/848956.
+// accepts all three objects, but expects only of one of them to be valid, and
+// the others null. (This is controlled at a higher level by
+// features::kMultiDeviceApi and features::kEnableUnifiedMultiDeviceSetup.).
+// Once Tether has been fully migrated, RemoteDeviceProvider and eventually
+// DeviceSyncClient will be ripped out of this class. See
+// https://crbug.com/848956.
 class TetherHostFetcherImpl
     : public TetherHostFetcher,
       public cryptauth::RemoteDeviceProvider::Observer,
@@ -93,24 +94,9 @@ class TetherHostFetcherImpl
                             multidevice_setup_client_);
 
  private:
-  enum class TetherHostSource {
-    UNKNOWN,
-    MULTIDEVICE_SETUP_CLIENT,
-    DEVICE_SYNC_CLIENT,
-    REMOTE_DEVICE_PROVIDER
-  };
-
   void CacheCurrentTetherHosts();
   cryptauth::RemoteDeviceRefList GenerateHostDeviceList();
-  TetherHostSource GetTetherHostSourceBasedOnFlags();
-  // This returns true if there is no BETTER_TOGETHER_HOST supported or enabled,
-  // but there *are* MAGIC_TETHER_HOSTs supported or enabled. This can only
-  // happen if the user's phone has not yet fully updated to the new multidevice
-  // world.
-  // TODO(crbug.com/894585): Remove this legacy special case after M71.
-  bool IsInLegacyHostMode();
-
-  bool started_in_legacy_mode_ = false;
+
   cryptauth::RemoteDeviceProvider* remote_device_provider_;
   device_sync::DeviceSyncClient* device_sync_client_;
   chromeos::multidevice_setup::MultiDeviceSetupClient*

chromeos/components/tether/tether_host_fetcher_impl_unittest.cc

@@ -122,8 +122,7 @@ class TetherHostFetcherImplTest : public testing::Test {
         tether_host_source == TetherHostSource::REMOTE_DEVICE_PROVIDER
             ? fake_remote_device_provider_.get()
             : nullptr,
-        tether_host_source == TetherHostSource::DEVICE_SYNC_CLIENT ||
-                tether_host_source == TetherHostSource::MULTIDEVICE_SETUP_CLIENT
+        tether_host_source == TetherHostSource::DEVICE_SYNC_CLIENT
             ? fake_device_sync_client_.get()
             : nullptr,
         tether_host_source == TetherHostSource::MULTIDEVICE_SETUP_CLIENT
@@ -176,9 +175,6 @@ class TetherHostFetcherImplTest : public testing::Test {
     // Mark the first device enabled instead of supported.
     list[0].software_features[cryptauth::SoftwareFeature::MAGIC_TETHER_HOST] =
         cryptauth::SoftwareFeatureState::kEnabled;
-    list[0]
-        .software_features[cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST] =
-        cryptauth::SoftwareFeatureState::kEnabled;
 
     return list;
   }
@@ -263,17 +259,8 @@ class TetherHostFetcherImplTest : public testing::Test {
     EXPECT_EQ(2u, test_observer_->num_updates());
   }
 
-  void TestSingleTetherHost(bool use_legacy_mode = false) {
+  void TestSingleTetherHost() {
     InitializeTest();
-    if (use_legacy_mode) {
-      test_remote_device_list_[0]
-          .software_features[cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST] =
-          cryptauth::SoftwareFeatureState::kNotSupported;
-      test_remote_device_ref_list_ =
-          CreateTestRemoteDeviceRefList(test_remote_device_list_);
-      SetSyncedDevices(test_remote_device_list_);
-      NotifyNewDevicesSynced();
-    }
 
     VerifySingleTetherHost(test_remote_device_ref_list_[0].GetDeviceId(),
                            test_remote_device_ref_list_[0]);
@@ -304,7 +291,7 @@ class TetherHostFetcherImplTest : public testing::Test {
                            base::nullopt);
   }
 
-  void TestFetchAllTetherHosts(bool use_legacy_mode = false) {
+  void TestFetchAllTetherHosts() {
     InitializeTest();
 
     // Create a list of test devices, only some of which are valid tether hosts.
@@ -315,11 +302,6 @@ class TetherHostFetcherImplTest : public testing::Test {
     test_remote_device_list_[4]
         .software_features[cryptauth::SoftwareFeature::MAGIC_TETHER_HOST] =
         cryptauth::SoftwareFeatureState::kNotSupported;
-    if (use_legacy_mode) {
-      test_remote_device_list_[0]
-          .software_features[cryptauth::SoftwareFeature::BETTER_TOGETHER_HOST] =
-          cryptauth::SoftwareFeatureState::kNotSupported;
-    }
 
     SetSyncedDevices(test_remote_device_list_);
     NotifyNewDevicesSynced();
@@ -327,12 +309,9 @@ class TetherHostFetcherImplTest : public testing::Test {
     cryptauth::RemoteDeviceRefList expected_host_device_list;
     switch (GetTetherHostSourceBasedOnFlags()) {
       case TetherHostSource::MULTIDEVICE_SETUP_CLIENT:
-        if (!use_legacy_mode) {
         expected_host_device_list =
             CreateTestRemoteDeviceRefList({test_remote_device_list_[0]});
         break;
-        }
-        FALLTHROUGH;
       case TetherHostSource::DEVICE_SYNC_CLIENT:
       case TetherHostSource::REMOTE_DEVICE_PROVIDER:
         expected_host_device_list = CreateTestRemoteDeviceRefList(
@@ -395,11 +374,6 @@ TEST_F(TetherHostFetcherImplTest,
   SetOnlyMultiDeviceApiFeatureEnabled();
   TestFetchAllTetherHosts();
 }
-TEST_F(TetherHostFetcherImplTest,
-       TestFetchAllTetherHosts_MultideviceApiAndSetupEnabledInLegacyMode) {
-  SetMultiDeviceApiAndSetupFeaturesEnabled();
-  TestFetchAllTetherHosts(true /* use_legacy_mode */);
-}
 
 // TestSingleTetherHost
 TEST_F(TetherHostFetcherImplTest, TestSingleTetherHost) {
@@ -415,11 +389,6 @@ TEST_F(TetherHostFetcherImplTest,
   SetOnlyMultiDeviceApiFeatureEnabled();
   TestSingleTetherHost();
 }
-TEST_F(TetherHostFetcherImplTest,
-       TestSingleTetherHost_MultideviceApiAndSetupEnabledInLegacyMode) {
-  SetMultiDeviceApiAndSetupFeaturesEnabled();
-  TestSingleTetherHost(true /* use_legacy_mode */);
-}
 
 // TestSingleTetherHost_IdDoesNotCorrespondToDevice
 TEST_F(TetherHostFetcherImplTest,