mash: Connect to the correct user profile pref service on startup

Previously the pref service was connected to the temporary login screen
profile prefs. Now it connects to the user profile on login and after
user switch.

Rename RegisterPrefs() to RegisterProfilePrefs() in ash to emphasize
that these are profile-associated and not local state.

Add ShellObserver notification of PrefService change. The lifetime is
tricky because the old PrefService must continue to exist so shell
observers can unregister their pref observers. Also, there is a window
using user switch where the user pref service is null.

This is based on https://chromium-review.googlesource.com/c/580115
and is a prerequisite for moving some system tray code out of chrome
into ash.

Bug: 747019, 721961, 736169
Test: added to ash_unittests
Change-Id: I186e55c1d88e0e2052b9cd76a490e26804e4b6d3
Reviewed-on: https://chromium-review.googlesource.com/590356
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490804}

ash/shell.cc

@@ -327,8 +327,8 @@ bool Shell::ShouldUseIMEService() {
 }
 
 // static
-void Shell::RegisterPrefs(PrefRegistrySimple* registry) {
-  NightLightController::RegisterPrefs(registry);
+void Shell::RegisterProfilePrefs(PrefRegistrySimple* registry) {
+  NightLightController::RegisterProfilePrefs(registry);
 }
 
 views::NonClientFrameView* Shell::CreateDefaultNonClientFrameView(
@@ -413,13 +413,6 @@ void Shell::UpdateShelfVisibility() {
     Shelf::ForWindow(root)->UpdateVisibilityState();
 }
 
-PrefService* Shell::GetActiveUserPrefService() const {
-  if (shell_port_->GetAshConfig() == Config::MASH)
-    return pref_service_.get();
-
-  return shell_delegate_->GetActiveUserPrefService();
-}
-
 PrefService* Shell::GetLocalStatePrefService() const {
   if (shell_port_->GetAshConfig() == Config::MASH)
     return local_state_.get();
@@ -830,7 +823,7 @@ Shell::~Shell() {
   // NightLightController depeneds on the PrefService and must be destructed
   // before it. crbug.com/724231.
   night_light_controller_ = nullptr;
-  pref_service_ = nullptr;
+  profile_pref_service_ = nullptr;
   shell_delegate_.reset();
 
   for (auto& observer : shell_observers_)
@@ -843,23 +836,17 @@ Shell::~Shell() {
 void Shell::Init(const ShellInitParams& init_params) {
   const Config config = shell_port_->GetAshConfig();
 
-  if (NightLightController::IsFeatureEnabled()) {
-    night_light_controller_ =
-        base::MakeUnique<NightLightController>(session_controller_.get());
-  }
+  if (NightLightController::IsFeatureEnabled())
+    night_light_controller_ = base::MakeUnique<NightLightController>();
 
   wallpaper_delegate_ = shell_delegate_->CreateWallpaperDelegate();
 
-  // Can be null in tests.
+  // Connector can be null in tests.
   if (config == Config::MASH && shell_delegate_->GetShellConnector()) {
+    // Connect to local state prefs now, but wait for an active user before
+    // connecting to the profile pref service. The login screen has a temporary
+    // user profile that is not associated with a real user.
     auto pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
-    Shell::RegisterPrefs(pref_registry.get());
-    prefs::ConnectToPrefService(shell_delegate_->GetShellConnector(),
-                                std::move(pref_registry),
-                                base::Bind(&Shell::OnPrefServiceInitialized,
-                                           weak_factory_.GetWeakPtr()),
-                                prefs::mojom::kForwarderServiceName);
-    pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
     prefs::ConnectToPrefService(
         shell_delegate_->GetShellConnector(), std::move(pref_registry),
         base::Bind(&Shell::OnLocalStatePrefServiceInitialized,
@@ -1243,6 +1230,34 @@ void Shell::OnWindowActivated(
     root_window_for_new_windows_ = gained_active->GetRootWindow();
 }
 
+void Shell::OnActiveUserSessionChanged(const AccountId& account_id) {
+  if (GetAshConfig() == Config::MASH && shell_delegate_->GetShellConnector()) {
+    // Profile pref service is null while connecting after profile switch.
+    if (profile_pref_service_) {
+      for (auto& observer : shell_observers_)
+        observer.OnActiveUserPrefServiceChanged(nullptr);
+      // Reset after notification so clients can unregister pref observers on
+      // the old PrefService.
+      profile_pref_service_.reset();
+    }
+
+    auto pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
+    RegisterProfilePrefs(pref_registry.get());
+    prefs::ConnectToPrefService(
+        shell_delegate_->GetShellConnector(), pref_registry,
+        base::Bind(&Shell::OnProfilePrefServiceInitialized,
+                   weak_factory_.GetWeakPtr()),
+        prefs::mojom::kForwarderServiceName);
+    return;
+  }
+
+  // On classic ash user profile prefs are available immediately after login.
+  // The login screen temporary profile is never available.
+  PrefService* profile_prefs = shell_delegate_->GetActiveUserPrefService();
+  for (auto& observer : shell_observers_)
+    observer.OnActiveUserPrefServiceChanged(profile_prefs);
+}
+
 void Shell::OnSessionStateChanged(session_manager::SessionState state) {
   // Initialize the shelf when a session becomes active. It's safe to do this
   // multiple times (e.g. initial login vs. multiprofile add session).
@@ -1296,16 +1311,20 @@ void Shell::InitializeShelf() {
     root->InitializeShelf();
 }
 
-void Shell::OnPrefServiceInitialized(
+void Shell::OnProfilePrefServiceInitialized(
     std::unique_ptr<::PrefService> pref_service) {
-  // |pref_service_| is null if can't connect to Chrome (as happens when
+  // |pref_service| can be null if can't connect to Chrome (as happens when
   // running mash outside of chrome --mash and chrome isn't built).
-  pref_service_ = std::move(pref_service);
+  for (auto& observer : shell_observers_)
+    observer.OnActiveUserPrefServiceChanged(pref_service.get());
+  // Reset after notifying clients so they can unregister pref observers on the
+  // old PrefService.
+  profile_pref_service_ = std::move(pref_service);
 }
 
 void Shell::OnLocalStatePrefServiceInitialized(
     std::unique_ptr<::PrefService> pref_service) {
-  // |pref_service_| is null if can't connect to Chrome (as happens when
+  // |pref_service| is null if can't connect to Chrome (as happens when
   // running mash outside of chrome --mash and chrome isn't built).
   local_state_ = std::move(pref_service);
 }

ash/shell.h

@@ -253,8 +253,8 @@ class ASH_EXPORT Shell : public SessionObserver,
   static Config GetAshConfig();
   static bool ShouldUseIMEService();
 
-  // Registers all ash related prefs to the given |registry|.
-  static void RegisterPrefs(PrefRegistrySimple* registry);
+  // Registers all ash related user profile prefs to the given |registry|.
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   // Creates a default views::NonClientFrameView for use by windows in the
   // Ash environment.
@@ -423,17 +423,6 @@ class ASH_EXPORT Shell : public SessionObserver,
   // TODO(jamescook): Move to Shelf.
   void UpdateShelfVisibility();
 
-  // Gets the current active user pref service.
-  // In classic ash, it will be null if there's no active user.
-  // In the case of mash, it can be null if it failed to or hasn't yet
-  // connected to the pref service.
-  //
-  // NOTE: Users of this pref service MUST listen to
-  // ash::SessionObserver::OnActiveUserSessionChanged() and recall this function
-  // to get the newly activated user's pref service, and use it to re-read the
-  // desired stored settings.
-  PrefService* GetActiveUserPrefService() const;
-
   // Gets the local state pref service. It can be null in mash if connecting to
   // local state pref service has not completed successfully.
   PrefService* GetLocalStatePrefService() const;
@@ -652,6 +641,7 @@ class ASH_EXPORT Shell : public SessionObserver,
                          aura::Window* lost_active) override;
 
   // SessionObserver:
+  void OnActiveUserSessionChanged(const AccountId& account_id) override;
   void OnSessionStateChanged(session_manager::SessionState state) override;
   void OnLoginStatusChanged(LoginStatus login_status) override;
   void OnLockStateChanged(bool locked) override;
@@ -661,7 +651,8 @@ class ASH_EXPORT Shell : public SessionObserver,
   void InitializeShelf();
 
   // Callbacks for prefs::ConnectToPrefService.
-  void OnPrefServiceInitialized(std::unique_ptr<::PrefService> pref_service);
+  void OnProfilePrefServiceInitialized(
+      std::unique_ptr<::PrefService> pref_service);
   void OnLocalStatePrefServiceInitialized(
       std::unique_ptr<::PrefService> pref_service);
 
@@ -724,8 +715,13 @@ class ASH_EXPORT Shell : public SessionObserver,
   std::unique_ptr<::wm::VisibilityController> visibility_controller_;
   std::unique_ptr<::wm::WindowModalityController> window_modality_controller_;
   std::unique_ptr<app_list::AppList> app_list_;
-  std::unique_ptr<::PrefService> pref_service_;
+
+  // Only initialized for mash. Can be null in ash_standalone (when chrome is
+  // not running) or when reconnecting to the mojo pref service after
+  // multiuser profile switch.
+  std::unique_ptr<::PrefService> profile_pref_service_;
   std::unique_ptr<::PrefService> local_state_;
+
   std::unique_ptr<views::corewm::TooltipController> tooltip_controller_;
   LinkHandlerModelFactory* link_handler_model_factory_;
   std::unique_ptr<PowerButtonController> power_button_controller_;

ash/shell_observer.h

@@ -12,6 +12,8 @@ namespace aura {
 class Window;
 }
 
+class PrefService;
+
 namespace ash {
 
 class ASH_EXPORT ShellObserver {
@@ -76,6 +78,15 @@ class ASH_EXPORT ShellObserver {
   // most of Shell's state has been deleted.
   virtual void OnShellDestroyed() {}
 
+  // Called when the user profile pref service is available. Also called after
+  // multiprofile user switch. Never called with the login screen profile.
+  // On mash will be called with null at the start of user switch then again
+  // with a pref service once the connection to the mojo pref service is made.
+  // TODO(jamescook): Either maintain pref service connections for all multiuser
+  // profiles or make the pref service switch atomic with active user switch.
+  // http://crbug.com/705347
+  virtual void OnActiveUserPrefServiceChanged(PrefService* pref_service) {}
+
  protected:
   virtual ~ShellObserver() {}
 };

ash/shell_unittest.cc

@@ -19,12 +19,16 @@
 #include "ash/shelf/shelf_widget.h"
 #include "ash/shell_test_api.h"
 #include "ash/test/ash_test_base.h"
+#include "ash/test/ash_test_helper.h"
+#include "ash/test_shell_delegate.h"
 #include "ash/wallpaper/wallpaper_widget_controller.h"
 #include "ash/wm/window_util.h"
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "components/prefs/testing_pref_service.h"
+#include "components/signin/core/account_id/account_id.h"
 #include "ui/aura/client/aura_constants.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window.h"
@@ -129,6 +133,22 @@ class SimpleMenuDelegate : public ui::SimpleMenuModel::Delegate {
   DISALLOW_COPY_AND_ASSIGN(SimpleMenuDelegate);
 };
 
+class TestShellObserver : public ShellObserver {
+ public:
+  TestShellObserver() = default;
+  ~TestShellObserver() override = default;
+
+  // ShellObserver:
+  void OnActiveUserPrefServiceChanged(PrefService* pref_service) override {
+    last_pref_service_ = pref_service;
+  }
+
+  PrefService* last_pref_service_ = nullptr;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestShellObserver);
+};
+
 }  // namespace
 
 class ShellTest : public AshTestBase {
@@ -529,4 +549,49 @@ TEST_F(ShellTest2, DontCrashWhenWindowDeleted) {
   window_->Init(ui::LAYER_NOT_DRAWN);
 }
 
+class ShellPrefsTest : public NoSessionAshTestBase {
+ public:
+  ShellPrefsTest() = default;
+  ~ShellPrefsTest() override = default;
+
+  // testing::Test:
+  void SetUp() override {
+    NoSessionAshTestBase::SetUp();
+    Shell::RegisterProfilePrefs(pref_service1_.registry());
+    Shell::RegisterProfilePrefs(pref_service2_.registry());
+  }
+
+  // Must outlive Shell.
+  TestingPrefServiceSimple pref_service1_;
+  TestingPrefServiceSimple pref_service2_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ShellPrefsTest);
+};
+
+// Verifies that ShellObserver is notified for PrefService changes.
+TEST_F(ShellPrefsTest, Observer) {
+  TestShellObserver observer;
+  Shell::Get()->AddShellObserver(&observer);
+
+  // Setup 2 users.
+  TestSessionControllerClient* session = GetSessionControllerClient();
+  session->AddUserSession("user1@test.com");
+  session->AddUserSession("user2@test.com");
+
+  // Login notifies observers of the user pref service.
+  ash_test_helper()->test_shell_delegate()->set_active_user_pref_service(
+      &pref_service1_);
+  session->SwitchActiveUser(AccountId::FromUserEmail("user1@test.com"));
+  EXPECT_EQ(&pref_service1_, observer.last_pref_service_);
+
+  // Switching users notifies observers of the new user pref service.
+  ash_test_helper()->test_shell_delegate()->set_active_user_pref_service(
+      &pref_service2_);
+  session->SwitchActiveUser(AccountId::FromUserEmail("user2@test.com"));
+  EXPECT_EQ(&pref_service2_, observer.last_pref_service_);
+
+  Shell::Get()->RemoveShellObserver(&observer);
+}
+
 }  // namespace ash

ash/system/night_light/night_light_controller.cc

@@ -116,16 +116,14 @@ void ApplyColorTemperatureToLayers(float layer_temperature,
 
 }  // namespace
 
-NightLightController::NightLightController(
-    SessionController* session_controller)
-    : session_controller_(session_controller),
-      delegate_(base::MakeUnique<NightLightControllerDelegateImpl>()),
+NightLightController::NightLightController()
+    : delegate_(base::MakeUnique<NightLightControllerDelegateImpl>()),
       binding_(this) {
-  session_controller_->AddObserver(this);
+  Shell::Get()->AddShellObserver(this);
 }
 
 NightLightController::~NightLightController() {
-  session_controller_->RemoveObserver(this);
+  Shell::Get()->RemoveShellObserver(this);
 }
 
 // static
@@ -135,7 +133,7 @@ bool NightLightController::IsFeatureEnabled() {
 }
 
 // static
-void NightLightController::RegisterPrefs(PrefRegistrySimple* registry) {
+void NightLightController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(prefs::kNightLightEnabled, false);
   registry->RegisterDoublePref(prefs::kNightLightTemperature,
                                kDefaultColorTemperature);
@@ -244,11 +242,10 @@ void NightLightController::Toggle() {
   SetEnabled(!GetEnabled(), AnimationDuration::kShort);
 }
 
-void NightLightController::OnActiveUserSessionChanged(
-    const AccountId& account_id) {
+void NightLightController::OnActiveUserPrefServiceChanged(
+    PrefService* pref_service) {
   // Initial login and user switching in multi profiles.
-  pref_change_registrar_.reset();
-  active_user_pref_service_ = Shell::Get()->GetActiveUserPrefService();
+  active_user_pref_service_ = pref_service;
   InitFromUserPrefs();
 }
 
@@ -319,10 +316,11 @@ void NightLightController::StartWatchingPrefsChanges() {
 }
 
 void NightLightController::InitFromUserPrefs() {
-  if (!active_user_pref_service_) {
-    // The pref_service can be null in ash_unittests.
+  pref_change_registrar_.reset();
+
+  // Pref service can be null during multiprofile switch and in tests.
+  if (!active_user_pref_service_)
     return;
-  }
 
   StartWatchingPrefsChanges();
   Refresh(true /* did_schedule_change */);

ash/system/night_light/night_light_controller.h

@@ -9,7 +9,7 @@
 
 #include "ash/ash_export.h"
 #include "ash/public/interfaces/night_light_controller.mojom.h"
-#include "ash/session/session_observer.h"
+#include "ash/shell_observer.h"
 #include "ash/system/night_light/time_of_day.h"
 #include "base/observer_list.h"
 #include "base/time/time.h"
@@ -22,13 +22,11 @@ class PrefService;
 
 namespace ash {
 
-class SessionController;
-
 // Controls the NightLight feature that adjusts the color temperature of the
 // screen.
 class ASH_EXPORT NightLightController
     : public NON_EXPORTED_BASE(mojom::NightLightController),
-      public SessionObserver {
+      public ShellObserver {
  public:
   using ScheduleType = mojom::NightLightController::ScheduleType;
 
@@ -72,13 +70,13 @@ class ASH_EXPORT NightLightController
     virtual ~Observer() {}
   };
 
-  explicit NightLightController(SessionController* session_controller);
+  NightLightController();
   ~NightLightController() override;
 
   // Returns true if the NightLight feature is enabled in the flags.
   static bool IsFeatureEnabled();
 
-  static void RegisterPrefs(PrefRegistrySimple* registry);
+  static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   AnimationDuration animation_duration() const { return animation_duration_; }
   AnimationDuration last_animation_duration() const {
@@ -109,8 +107,8 @@ class ASH_EXPORT NightLightController
   // AnimationDurationType::kShort.
   void Toggle();
 
-  // ash::SessionObserver:
-  void OnActiveUserSessionChanged(const AccountId& account_id) override;
+  // ShellObserver:
+  void OnActiveUserPrefServiceChanged(PrefService* pref_service) override;
 
   // ash::mojom::NightLightController:
   void SetCurrentGeoposition(mojom::SimpleGeopositionPtr position) override;
@@ -163,10 +161,6 @@ class ASH_EXPORT NightLightController
   // AnimationDurationType::kLong.
   void ScheduleNextToggle(base::TimeDelta delay);
 
-  // The observed session controller instance from which we know when to
-  // initialize the NightLight settings from the user preferences.
-  SessionController* const session_controller_;
-
   std::unique_ptr<Delegate> delegate_;
 
   // The pref service of the currently active user. Can be null in

ash/system/night_light/night_light_controller_unittest.cc

@@ -133,8 +133,8 @@ class NightLightTest : public AshTestBase {
 
     AshTestBase::SetUp();
     CreateTestUserSessions();
-    Shell::RegisterPrefs(user1_pref_service_.registry());
-    Shell::RegisterPrefs(user2_pref_service_.registry());
+    Shell::RegisterProfilePrefs(user1_pref_service_.registry());
+    Shell::RegisterProfilePrefs(user2_pref_service_.registry());
 
     // Simulate user 1 login.
     InjectTestPrefService(&user1_pref_service_);
@@ -177,11 +177,6 @@ class NightLightTest : public AshTestBase {
 // Tests toggling NightLight on / off and makes sure the observer is updated and
 // the layer temperatures are modified.
 TEST_F(NightLightTest, TestToggle) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   UpdateDisplay("800x600,800x600");
 
   TestObserver observer;
@@ -201,11 +196,6 @@ TEST_F(NightLightTest, TestToggle) {
 
 // Tests setting the temperature in various situations.
 TEST_F(NightLightTest, TestSetTemperature) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   UpdateDisplay("800x600,800x600");
 
   TestObserver observer;
@@ -250,11 +240,6 @@ TEST_F(NightLightTest, TestSetTemperature) {
 // Tests that switching users retrieves NightLight settings for the active
 // user's prefs.
 TEST_F(NightLightTest, TestUserSwitchAndSettingsPersistence) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   // Test start with user1 logged in.
   NightLightController* controller = GetController();
   SetNightLightEnabled(true);
@@ -289,11 +274,6 @@ TEST_F(NightLightTest, TestUserSwitchAndSettingsPersistence) {
 // Tests that changes from outside NightLightControlled to the NightLight
 // Preferences are seen by the controlled and applied properly.
 TEST_F(NightLightTest, TestOutsidePreferencesChangesAreApplied) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   // Test start with user1 logged in.
   NightLightController* controller = GetController();
   user1_pref_service()->SetBoolean(prefs::kNightLightEnabled, true);
@@ -312,11 +292,6 @@ TEST_F(NightLightTest, TestOutsidePreferencesChangesAreApplied) {
 
 // Tests transitioning from kNone to kCustom and back to kNone schedule types.
 TEST_F(NightLightTest, TestScheduleNoneToCustomTransition) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   NightLightController* controller = GetController();
   // Now is 6:00 PM.
   delegate()->SetFakeNow(TimeOfDay(18 * 60));
@@ -360,11 +335,6 @@ TEST_F(NightLightTest, TestScheduleNoneToCustomTransition) {
 // Tests what happens when the time now reaches the end of the NightLight
 // interval when NightLight mode is on.
 TEST_F(NightLightTest, TestCustomScheduleReachingEndTime) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   NightLightController* controller = GetController();
   delegate()->SetFakeNow(TimeOfDay(18 * 60));
   controller->SetCustomStartTime(TimeOfDay(15 * 60));
@@ -398,11 +368,6 @@ TEST_F(NightLightTest, TestCustomScheduleReachingEndTime) {
 // Tests that user toggles from the system menu or system settings override any
 // status set by an automatic schedule.
 TEST_F(NightLightTest, TestExplicitUserTogglesWhileScheduleIsActive) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   // Start with the below custom schedule, where NightLight is off.
   //
   //      15:00               20:00          23:00
@@ -450,11 +415,6 @@ TEST_F(NightLightTest, TestExplicitUserTogglesWhileScheduleIsActive) {
 // shouldn't change the current status, only updates the timer but doesn't
 // change the status.
 TEST_F(NightLightTest, TestChangingStartTimesThatDontChangeTheStatus) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   //       16:00        18:00         22:00
   // <----- + ----------- + ----------- + ----->
   //        |             |             |
@@ -498,11 +458,6 @@ TEST_F(NightLightTest, TestChangingStartTimesThatDontChangeTheStatus) {
 
 // Tests the behavior of the sunset to sunrise automatic schedule type.
 TEST_F(NightLightTest, TestSunsetSunrise) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   //      16:00         18:00     20:00      22:00              5:00
   // <----- + ----------- + ------- + -------- + --------------- + ------->
   //        |             |         |          |                 |
@@ -555,11 +510,6 @@ TEST_F(NightLightTest, TestSunsetSunrise) {
 // Tests the behavior of the sunset to sunrise automatic schedule type when the
 // client sets the geoposition.
 TEST_F(NightLightTest, TestSunsetSunriseGeoposition) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   // Position 1 sunset and sunrise times.
   //
   //      16:00       20:00               4:00

ash/system/night_light/tray_night_light_unittest.cc

@@ -36,7 +36,7 @@ class TrayNightLightTest : public AshTestBase {
     AshTestBase::SetUp();
     GetSessionControllerClient()->Reset();
     GetSessionControllerClient()->AddUserSession(kFakeUserEmail);
-    Shell::RegisterPrefs(pref_service_.registry());
+    Shell::RegisterProfilePrefs(pref_service_.registry());
 
     ash_test_helper()->test_shell_delegate()->set_active_user_pref_service(
         &pref_service_);
@@ -53,11 +53,6 @@ class TrayNightLightTest : public AshTestBase {
 // Tests that when NightLight is active, its tray icon in the System Tray is
 // visible.
 TEST_F(TrayNightLightTest, TestNightLightTrayVisibility) {
-  if (Shell::GetAshConfig() == Config::MASH) {
-    // PrefChangeRegistrar doesn't work on mash. crbug.com/721961.
-    return;
-  }
-
   SystemTray* tray = GetPrimarySystemTray();
   TrayNightLight* tray_night_light = tray->tray_night_light();
   NightLightController* controller = Shell::Get()->night_light_controller();

chrome/browser/chromeos/preferences.cc

@@ -140,7 +140,7 @@ void Preferences::RegisterProfilePrefs(
 
   // Register ash prefs.
   if (!ash_util::IsRunningInMash())
-    ash::Shell::RegisterPrefs(registry);
+    ash::Shell::RegisterProfilePrefs(registry);
 
   registry->RegisterBooleanPref(prefs::kPerformanceTracingEnabled, false);
 

chrome/browser/prefs/active_profile_pref_service.cc

@@ -10,7 +10,8 @@
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/service_manager/public/cpp/service_context.h"
 
-ActiveProfilePrefService::ActiveProfilePrefService() {
+ActiveProfilePrefService::ActiveProfilePrefService()
+    : connector_binding_(this) {
   registry_.AddInterface<prefs::mojom::PrefStoreConnector>(
       base::Bind(&ActiveProfilePrefService::Create, base::Unretained(this)));
 }
@@ -26,7 +27,8 @@ void ActiveProfilePrefService::Connect(
 
 void ActiveProfilePrefService::Create(
     prefs::mojom::PrefStoreConnectorRequest request) {
-  connector_bindings_.AddBinding(this, std::move(request));
+  connector_binding_.Close();
+  connector_binding_.Bind(std::move(request));
 }
 
 void ActiveProfilePrefService::OnStart() {}
@@ -50,7 +52,7 @@ void ActiveProfilePrefService::OnBindInterface(
 }
 
 void ActiveProfilePrefService::OnConnectError() {
-  connector_bindings_.CloseAllBindings();
+  connector_binding_.Close();
 }
 
 prefs::mojom::PrefStoreConnector&

chrome/browser/prefs/active_profile_pref_service.h

@@ -9,7 +9,7 @@
 
 #include "base/macros.h"
 #include "components/prefs/pref_value_store.h"
-#include "mojo/public/cpp/bindings/binding_set.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "services/preferences/public/interfaces/preferences.mojom.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/service.h"
@@ -47,7 +47,7 @@ class ActiveProfilePrefService : public prefs::mojom::PrefStoreConnector,
 
   prefs::mojom::PrefStoreConnectorPtr connector_ptr_;
   service_manager::BinderRegistry registry_;
-  mojo::BindingSet<prefs::mojom::PrefStoreConnector> connector_bindings_;
+  mojo::Binding<prefs::mojom::PrefStoreConnector> connector_binding_;
 
   DISALLOW_COPY_AND_ASSIGN(ActiveProfilePrefService);
 };