Proximity Auth: Remove kEnableUnifiedMultiDeviceSetup flagging.

R=hansberry@chromium.org

Bug: 899324
Test: none
Change-Id: I45d7f04e97a914596cd1bffd5e2ea73543064810
Reviewed-on: https://chromium-review.googlesource.com/c/1329889
Commit-Queue: James Hawkins <jhawkins@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607051}

chromeos/components/proximity_auth/proximity_auth_profile_pref_manager.cc

@@ -27,21 +27,15 @@ ProximityAuthProfilePrefManager::ProximityAuthProfilePrefManager(
     : pref_service_(pref_service),
       multidevice_setup_client_(multidevice_setup_client),
       weak_ptr_factory_(this) {
-  if (base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
-    OnFeatureStatesChanged(multidevice_setup_client_->GetFeatureStates());
+  OnFeatureStatesChanged(multidevice_setup_client_->GetFeatureStates());
 
-    multidevice_setup_client_->AddObserver(this);
-  }
+  multidevice_setup_client_->AddObserver(this);
 }
 
 ProximityAuthProfilePrefManager::~ProximityAuthProfilePrefManager() {
   registrar_.RemoveAll();
 
-  if (base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSetup)) {
-    multidevice_setup_client_->RemoveObserver(this);
-  }
+  multidevice_setup_client_->RemoveObserver(this);
 }
 
 // static
@@ -128,9 +122,7 @@ void ProximityAuthProfilePrefManager::SetIsEasyUnlockEnabled(
 }
 
 bool ProximityAuthProfilePrefManager::IsEasyUnlockEnabled() const {
-  if (base::FeatureList::IsEnabled(
-          chromeos::features::kEnableUnifiedMultiDeviceSetup) &&
-      !is_in_legacy_host_mode_) {
+  if (!is_in_legacy_host_mode_) {
     return feature_state_ ==
            chromeos::multidevice_setup::mojom::FeatureState::kEnabledByUser;
   }

chromeos/components/proximity_auth/proximity_auth_profile_pref_manager_unittest.cc

@@ -14,6 +14,7 @@
 #include "chromeos/chromeos_features.h"
 #include "chromeos/components/proximity_auth/proximity_auth_local_state_pref_manager.h"
 #include "chromeos/components/proximity_auth/proximity_auth_pref_names.h"
+#include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
 #include "chromeos/services/multidevice_setup/public/cpp/prefs.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
@@ -37,96 +38,91 @@ const ProximityAuthPrefManager::ProximityThreshold kProximityThreshold2 =
 
 class ProximityAuthProfilePrefManagerTest : public testing::Test {
  protected:
-  ProximityAuthProfilePrefManagerTest() {}
+  ProximityAuthProfilePrefManagerTest() = default;
 
   void SetUp() override {
+    fake_multidevice_setup_client_ = std::make_unique<
+        chromeos::multidevice_setup::FakeMultiDeviceSetupClient>();
     ProximityAuthProfilePrefManager::RegisterPrefs(pref_service_.registry());
     chromeos::multidevice_setup::RegisterFeaturePrefs(pref_service_.registry());
-
-    scoped_feature_list_.InitWithFeatures(
-        std::vector<base::Feature>() /* enable_features */,
-        std::vector<base::Feature>{
-            chromeos::features::
-                kEnableUnifiedMultiDeviceSetup} /* disable_features */);
+    pref_manager_ = std::make_unique<ProximityAuthProfilePrefManager>(
+        &pref_service_, fake_multidevice_setup_client_.get());
   }
 
+  std::unique_ptr<chromeos::multidevice_setup::FakeMultiDeviceSetupClient>
+      fake_multidevice_setup_client_;
   sync_preferences::TestingPrefServiceSyncable pref_service_;
+  std::unique_ptr<ProximityAuthProfilePrefManager> pref_manager_;
 
  private:
-  base::test::ScopedFeatureList scoped_feature_list_;
   DISALLOW_COPY_AND_ASSIGN(ProximityAuthProfilePrefManagerTest);
 };
 
+// TODO(crbug/904005): Investigate using Unified Setup API instead of directly
+// manipulating the pref.
 TEST_F(ProximityAuthProfilePrefManagerTest, IsEasyUnlockAllowed) {
-  ProximityAuthProfilePrefManager pref_manager(
-      &pref_service_, nullptr /* multidevice_setup_service */);
-  EXPECT_TRUE(pref_manager.IsEasyUnlockAllowed());
+  EXPECT_TRUE(pref_manager_->IsEasyUnlockAllowed());
 
   // Simulating setting kEasyUnlockAllowed pref through enterprise policy.
   pref_service_.SetBoolean(
       chromeos::multidevice_setup::kSmartLockAllowedPrefName, false);
-  EXPECT_FALSE(pref_manager.IsEasyUnlockAllowed());
+  EXPECT_FALSE(pref_manager_->IsEasyUnlockAllowed());
 }
 
 TEST_F(ProximityAuthProfilePrefManagerTest, IsEasyUnlockEnabled) {
-  ProximityAuthProfilePrefManager pref_manager(
-      &pref_service_, nullptr /* multidevice_setup_service */);
-  EXPECT_TRUE(pref_manager.IsEasyUnlockEnabled());
+  pref_manager_->SetIsInLegacyHostMode(true);
+
+  EXPECT_TRUE(pref_manager_->IsEasyUnlockEnabled());
 
-  pref_manager.SetIsEasyUnlockEnabled(true);
-  EXPECT_TRUE(pref_manager.IsEasyUnlockEnabled());
+  pref_manager_->SetIsEasyUnlockEnabled(true);
+  EXPECT_TRUE(pref_manager_->IsEasyUnlockEnabled());
 
-  pref_manager.SetIsEasyUnlockEnabled(false);
-  EXPECT_FALSE(pref_manager.IsEasyUnlockEnabled());
+  pref_manager_->SetIsEasyUnlockEnabled(false);
+  EXPECT_FALSE(pref_manager_->IsEasyUnlockEnabled());
 }
 
 TEST_F(ProximityAuthProfilePrefManagerTest, LastPromotionCheckTimestamp) {
-  ProximityAuthProfilePrefManager pref_manager(
-      &pref_service_, nullptr /* multidevice_setup_service */);
-  EXPECT_EQ(0L, pref_manager.GetLastPromotionCheckTimestampMs());
-  pref_manager.SetLastPromotionCheckTimestampMs(kPromotionCheckTimestampMs1);
+  EXPECT_EQ(0L, pref_manager_->GetLastPromotionCheckTimestampMs());
+  pref_manager_->SetLastPromotionCheckTimestampMs(kPromotionCheckTimestampMs1);
   EXPECT_EQ(kPromotionCheckTimestampMs1,
-            pref_manager.GetLastPromotionCheckTimestampMs());
-  pref_manager.SetLastPromotionCheckTimestampMs(kPromotionCheckTimestampMs2);
+            pref_manager_->GetLastPromotionCheckTimestampMs());
+  pref_manager_->SetLastPromotionCheckTimestampMs(kPromotionCheckTimestampMs2);
   EXPECT_EQ(kPromotionCheckTimestampMs2,
-            pref_manager.GetLastPromotionCheckTimestampMs());
+            pref_manager_->GetLastPromotionCheckTimestampMs());
 }
 
 TEST_F(ProximityAuthProfilePrefManagerTest, PromotionShownCount) {
-  ProximityAuthProfilePrefManager pref_manager(
-      &pref_service_, nullptr /* multidevice_setup_service */);
-  EXPECT_EQ(0, pref_manager.GetPromotionShownCount());
-  pref_manager.SetPromotionShownCount(1);
-  EXPECT_EQ(1, pref_manager.GetPromotionShownCount());
-  pref_manager.SetPromotionShownCount(2);
-  EXPECT_EQ(2, pref_manager.GetPromotionShownCount());
+  EXPECT_EQ(0, pref_manager_->GetPromotionShownCount());
+  pref_manager_->SetPromotionShownCount(1);
+  EXPECT_EQ(1, pref_manager_->GetPromotionShownCount());
+  pref_manager_->SetPromotionShownCount(2);
+  EXPECT_EQ(2, pref_manager_->GetPromotionShownCount());
 }
 
 TEST_F(ProximityAuthProfilePrefManagerTest, ProximityThreshold) {
-  ProximityAuthProfilePrefManager pref_manager(
-      &pref_service_, nullptr /* multidevice_setup_service */);
-  EXPECT_EQ(1, pref_manager.GetProximityThreshold());
-  pref_manager.SetProximityThreshold(kProximityThreshold1);
-  EXPECT_EQ(kProximityThreshold1, pref_manager.GetProximityThreshold());
-  pref_manager.SetProximityThreshold(kProximityThreshold2);
-  EXPECT_EQ(kProximityThreshold2, pref_manager.GetProximityThreshold());
+  EXPECT_EQ(1, pref_manager_->GetProximityThreshold());
+  pref_manager_->SetProximityThreshold(kProximityThreshold1);
+  EXPECT_EQ(kProximityThreshold1, pref_manager_->GetProximityThreshold());
+  pref_manager_->SetProximityThreshold(kProximityThreshold2);
+  EXPECT_EQ(kProximityThreshold2, pref_manager_->GetProximityThreshold());
 }
 
 TEST_F(ProximityAuthProfilePrefManagerTest, IsChromeOSLoginEnabled) {
-  ProximityAuthProfilePrefManager pref_manager(
-      &pref_service_, nullptr /* multidevice_setup_service */);
-  EXPECT_FALSE(pref_manager.IsChromeOSLoginEnabled());
+  EXPECT_FALSE(pref_manager_->IsChromeOSLoginEnabled());
 
-  pref_manager.SetIsChromeOSLoginEnabled(true);
-  EXPECT_TRUE(pref_manager.IsChromeOSLoginEnabled());
+  pref_manager_->SetIsChromeOSLoginEnabled(true);
+  EXPECT_TRUE(pref_manager_->IsChromeOSLoginEnabled());
 
-  pref_manager.SetIsChromeOSLoginEnabled(false);
-  EXPECT_FALSE(pref_manager.IsChromeOSLoginEnabled());
+  pref_manager_->SetIsChromeOSLoginEnabled(false);
+  EXPECT_FALSE(pref_manager_->IsChromeOSLoginEnabled());
 }
 
 TEST_F(ProximityAuthProfilePrefManagerTest, SyncsToLocalPrefOnChange) {
+  // Use a local variable to ensure that the PrefRegistrar adds and removes
+  // observers on the same thread.
   ProximityAuthProfilePrefManager profile_pref_manager(
-      &pref_service_, nullptr /* multidevice_setup_service */);
+      &pref_service_, fake_multidevice_setup_client_.get());
+  profile_pref_manager.SetIsInLegacyHostMode(true);
 
   TestingPrefServiceSimple local_state;
   AccountId account_id = AccountId::FromUserEmail(kUserEmail);

chromeos/components/proximity_auth/proximity_auth_system_unittest.cc

@@ -139,9 +139,7 @@ class ProximityAuthSystemTest : public testing::Test {
     pref_manager_.reset();
   }
 
-  void InitializeTest(bool multidevice_flags_enabled) {
-    SetMultiDeviceApi(multidevice_flags_enabled /* enabled */);
-
+  void SetUp() override {
     fake_multidevice_setup_client_ = std::make_unique<
         chromeos::multidevice_setup::FakeMultiDeviceSetupClient>();
     pref_manager_ = std::make_unique<NiceMock<MockProximityAuthPrefManager>>(
@@ -167,17 +165,6 @@ class ProximityAuthSystemTest : public testing::Test {
     LockScreen();
   }
 
-  void SetMultiDeviceApi(bool enabled) {
-    static const std::vector<base::Feature> kFeatures{
-        chromeos::features::kEnableUnifiedMultiDeviceSetup};
-
-    scoped_feature_list_.InitWithFeatures(
-        (enabled ? kFeatures
-                 : std::vector<base::Feature>() /* enable_features */),
-        (enabled ? std::vector<base::Feature>()
-                 : kFeatures /* disable_features */));
-  }
-
   void InitProximityAuthSystem(ProximityAuthSystem::ScreenlockType type) {
     std::unique_ptr<MockUnlockManager> unlock_manager(
         new NiceMock<MockUnlockManager>());
@@ -239,7 +226,6 @@ class ProximityAuthSystemTest : public testing::Test {
 };
 
 TEST_F(ProximityAuthSystemTest, SetRemoteDevicesForUser_NotStarted) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   InitProximityAuthSystem(ProximityAuthSystem::SESSION_LOCK);
 
   AccountId account1 = AccountId::FromUserEmail(kUser1);
@@ -264,7 +250,6 @@ TEST_F(ProximityAuthSystemTest, SetRemoteDevicesForUser_NotStarted) {
 }
 
 TEST_F(ProximityAuthSystemTest, SetRemoteDevicesForUser_Started) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   InitProximityAuthSystem(ProximityAuthSystem::SESSION_LOCK);
   AccountId account1 = AccountId::FromUserEmail(kUser1);
   AccountId account2 = AccountId::FromUserEmail(kUser2);
@@ -284,7 +269,6 @@ TEST_F(ProximityAuthSystemTest, SetRemoteDevicesForUser_Started) {
 }
 
 TEST_F(ProximityAuthSystemTest, FocusRegisteredUser) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   EXPECT_FALSE(life_cycle());
   EXPECT_EQ(std::string(),
             ScreenlockBridge::Get()->focused_account_id().GetUserEmail());
@@ -304,7 +288,6 @@ TEST_F(ProximityAuthSystemTest, FocusRegisteredUser) {
 }
 
 TEST_F(ProximityAuthSystemTest, FocusUnregisteredUser) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   EXPECT_FALSE(life_cycle());
   EXPECT_EQ(std::string(),
             ScreenlockBridge::Get()->focused_account_id().GetUserEmail());
@@ -315,35 +298,6 @@ TEST_F(ProximityAuthSystemTest, FocusUnregisteredUser) {
 }
 
 TEST_F(ProximityAuthSystemTest, ToggleFocus_RegisteredUsers) {
-  InitializeTest(false /* multidevice_flags_enabled */);
-  proximity_auth_system_->SetRemoteDevicesForUser(
-      AccountId::FromUserEmail(kUser2), user2_remote_devices_,
-      user2_local_device_);
-
-  RemoteDeviceLifeCycle* life_cycle1 = nullptr;
-  EXPECT_CALL(*unlock_manager_, SetRemoteDeviceLifeCycle(_))
-      .WillOnce(SaveArg<0>(&life_cycle1));
-  FocusUser(kUser1);
-  EXPECT_EQ(kUser1, life_cycle1->GetRemoteDevice().user_id());
-
-  RemoteDeviceLifeCycle* life_cycle2 = nullptr;
-  {
-    InSequence sequence;
-    EXPECT_CALL(*unlock_manager_, SetRemoteDeviceLifeCycle(nullptr))
-        .Times(AtLeast(1));
-    EXPECT_CALL(*unlock_manager_, SetRemoteDeviceLifeCycle(_))
-        .WillOnce(SaveArg<0>(&life_cycle2));
-  }
-  FocusUser(kUser2);
-  EXPECT_EQ(kUser2, life_cycle2->GetRemoteDevice().user_id());
-
-  EXPECT_CALL(*unlock_manager_, SetRemoteDeviceLifeCycle(nullptr))
-      .Times(AtLeast(1));
-}
-
-TEST_F(ProximityAuthSystemTest,
-       ToggleFocus_RegisteredUsers_MultiDeviceApiEnabled) {
-  InitializeTest(true /* multidevice_flags_enabled */);
   proximity_auth_system_->SetRemoteDevicesForUser(
       AccountId::FromUserEmail(kUser1), user1_remote_devices_,
       user1_local_device_);
@@ -375,7 +329,6 @@ TEST_F(ProximityAuthSystemTest,
 }
 
 TEST_F(ProximityAuthSystemTest, ToggleFocus_UnregisteredUsers) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   FocusUser(kUser2);
   EXPECT_FALSE(life_cycle());
 
@@ -387,7 +340,6 @@ TEST_F(ProximityAuthSystemTest, ToggleFocus_UnregisteredUsers) {
 }
 
 TEST_F(ProximityAuthSystemTest, ToggleFocus_RegisteredAndUnregisteredUsers) {
-  InitializeTest(false /* multidevice_flags_enabled */);
 
   // Focus User 1, who is registered. This should create a new life cycle.
   RemoteDeviceLifeCycle* life_cycle = nullptr;
@@ -415,7 +367,6 @@ TEST_F(ProximityAuthSystemTest, ToggleFocus_RegisteredAndUnregisteredUsers) {
 }
 
 TEST_F(ProximityAuthSystemTest, ToggleFocus_SameUserRefocused) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   RemoteDeviceLifeCycle* life_cycle = nullptr;
   EXPECT_CALL(*unlock_manager_, SetRemoteDeviceLifeCycle(_))
       .WillOnce(SaveArg<0>(&life_cycle));
@@ -433,7 +384,6 @@ TEST_F(ProximityAuthSystemTest, ToggleFocus_SameUserRefocused) {
 }
 
 TEST_F(ProximityAuthSystemTest, RestartSystem_UnregisteredUserFocused) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   FocusUser(kUser2);
 
   EXPECT_CALL(*unlock_manager_, SetRemoteDeviceLifeCycle(nullptr))
@@ -444,7 +394,6 @@ TEST_F(ProximityAuthSystemTest, RestartSystem_UnregisteredUserFocused) {
 }
 
 TEST_F(ProximityAuthSystemTest, StopSystem_RegisteredUserFocused) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   EXPECT_CALL(*unlock_manager_, SetRemoteDeviceLifeCycle(NotNull()));
   FocusUser(kUser1);
 
@@ -458,7 +407,6 @@ TEST_F(ProximityAuthSystemTest, StopSystem_RegisteredUserFocused) {
 }
 
 TEST_F(ProximityAuthSystemTest, OnLifeCycleStateChanged) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   FocusUser(kUser1);
 
   EXPECT_CALL(*unlock_manager_, OnLifeCycleStateChanged());
@@ -473,14 +421,12 @@ TEST_F(ProximityAuthSystemTest, OnLifeCycleStateChanged) {
 }
 
 TEST_F(ProximityAuthSystemTest, OnAuthAttempted) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   FocusUser(kUser1);
   EXPECT_CALL(*unlock_manager_, OnAuthAttempted(_));
   proximity_auth_system_->OnAuthAttempted(AccountId::FromUserEmail(kUser1));
 }
 
 TEST_F(ProximityAuthSystemTest, Suspend_ScreenUnlocked) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   UnlockScreen();
   EXPECT_FALSE(life_cycle());
   SimulateSuspend();
@@ -488,13 +434,11 @@ TEST_F(ProximityAuthSystemTest, Suspend_ScreenUnlocked) {
 }
 
 TEST_F(ProximityAuthSystemTest, Suspend_UnregisteredUserFocused) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   SimulateSuspend();
   EXPECT_FALSE(life_cycle());
 }
 
 TEST_F(ProximityAuthSystemTest, Suspend_RegisteredUserFocused) {
-  InitializeTest(false /* multidevice_flags_enabled */);
   FocusUser(kUser1);
 
   {