Fix sharded QuickUnlockPrivateUnitTest

These tests would often fail when run in parallel, but would always pass
in isolation. This change fixes the tests via proper use of SetUp and
TearDown, and moves some initialization of EasyUnlockServiceRegular
from the ctor to InitializeInternal(). This also removes some virtual
method calls from the ctor, as per the style guide: "Constructors should
never call virtual functions"

Bug: none
Change-Id: Ibdb8628778105ed242c137ffa5ee12ffa018b06e
Reviewed-on: https://chromium-review.googlesource.com/c/1493072
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Auto-Submit: Evan Stade <estade@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636590}

chrome/browser/chromeos/extensions/quick_unlock_private/quick_unlock_private_api_unittest.cc

@@ -135,9 +135,8 @@ class QuickUnlockPrivateUnitTest
     : public ExtensionApiUnittest,
       public ::testing::WithParamInterface<TestType> {
  public:
-  QuickUnlockPrivateUnitTest()
-      : fake_user_manager_(new FakeChromeUserManager()),
-        scoped_user_manager_(base::WrapUnique(fake_user_manager_)) {}
+  QuickUnlockPrivateUnitTest() = default;
+  ~QuickUnlockPrivateUnitTest() override = default;
 
  protected:
   void SetUp() override {
@@ -152,6 +151,10 @@ class QuickUnlockPrivateUnitTest
         std::move(cryptohome_client));
     SystemSaltGetter::Initialize();
 
+    fake_user_manager_ = new FakeChromeUserManager();
+    scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
+        base::WrapUnique(fake_user_manager_));
+
     ExtensionApiUnittest::SetUp();
 
     SystemSaltGetter::Get()->SetRawSaltForTesting({1, 2, 3, 4, 5, 6, 7, 8});
@@ -190,10 +193,13 @@ class QuickUnlockPrivateUnitTest
 
     base::RunLoop().RunUntilIdle();
 
+    ExtensionApiUnittest::TearDown();
+
     fake_user_manager_ = nullptr;
+    scoped_user_manager_.reset();
 
-    ExtensionApiUnittest::TearDown();
     SystemSaltGetter::Shutdown();
+    DBusThreadManager::Shutdown();
     cryptohome::HomedirMethods::Shutdown();
   }
 
@@ -501,8 +507,8 @@ class QuickUnlockPrivateUnitTest
     expect_modes_changed_ = false;
   }
 
-  FakeChromeUserManager* fake_user_manager_;
-  user_manager::ScopedUserManager scoped_user_manager_;
+  FakeChromeUserManager* fake_user_manager_ = nullptr;
+  std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
   QuickUnlockPrivateSetModesFunction::ModesChangedEventHandler
       modes_changed_handler_;
   bool expect_modes_changed_ = false;

chrome/browser/chromeos/login/easy_unlock/easy_unlock_service_regular.cc

@@ -107,23 +107,9 @@ EasyUnlockServiceRegular::EasyUnlockServiceRegular(
     multidevice_setup::MultiDeviceSetupClient* multidevice_setup_client)
     : EasyUnlockService(profile, secure_channel_client),
       lock_screen_last_shown_timestamp_(base::TimeTicks::Now()),
-      deferring_device_load_(false),
       notification_controller_(std::move(notification_controller)),
       device_sync_client_(device_sync_client),
-      multidevice_setup_client_(multidevice_setup_client),
-      shown_pairing_changed_notification_(false),
-      weak_ptr_factory_(this) {
-  // If |device_sync_client_| is not ready yet, wait for it to call back on
-  // OnReady().
-  if (device_sync_client_->is_ready())
-    OnReady();
-
-  device_sync_client_->AddObserver(this);
-
-  OnFeatureStatesChanged(multidevice_setup_client_->GetFeatureStates());
-
-  multidevice_setup_client_->AddObserver(this);
-}
+      multidevice_setup_client_(multidevice_setup_client) {}
 
 EasyUnlockServiceRegular::~EasyUnlockServiceRegular() = default;
 
@@ -347,6 +333,17 @@ void EasyUnlockServiceRegular::RecordPasswordLoginEvent(
 }
 
 void EasyUnlockServiceRegular::InitializeInternal() {
+  // If |device_sync_client_| is not ready yet, wait for it to call back on
+  // OnReady().
+  if (device_sync_client_->is_ready())
+    OnReady();
+
+  device_sync_client_->AddObserver(this);
+
+  OnFeatureStatesChanged(multidevice_setup_client_->GetFeatureStates());
+
+  multidevice_setup_client_->AddObserver(this);
+
   proximity_auth::ScreenlockBridge::Get()->AddObserver(this);
 
   pref_manager_.reset(new proximity_auth::ProximityAuthProfilePrefManager(

chrome/browser/chromeos/login/easy_unlock/easy_unlock_service_regular.h

@@ -145,7 +145,7 @@ class EasyUnlockServiceRegular
   // loading the RemoteDevice until the screen is unlocked. For security,
   // this deferment prevents the lock screen from being changed by a network
   // event.
-  bool deferring_device_load_;
+  bool deferring_device_load_ = false;
 
   // Responsible for showing all the notifications used for EasyUnlock.
   std::unique_ptr<EasyUnlockNotificationController> notification_controller_;
@@ -170,12 +170,12 @@ class EasyUnlockServiceRegular
   // True if the pairing changed notification was shown, so that the next time
   // the Chromebook is unlocked, we can show the subsequent 'pairing applied'
   // notification.
-  bool shown_pairing_changed_notification_;
+  bool shown_pairing_changed_notification_ = false;
 
   // Listens to pref changes.
   PrefChangeRegistrar registrar_;
 
-  base::WeakPtrFactory<EasyUnlockServiceRegular> weak_ptr_factory_;
+  base::WeakPtrFactory<EasyUnlockServiceRegular> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(EasyUnlockServiceRegular);
 };