Elim ash::Shell::GetLocalStatePrefService

In mash, Shell::local_state_ is loaded asynchronously. Components
that require access to local state should implement
ShellObserver::OnLocalStatePrefServiceInitialized.

Bug: 678949
Change-Id: I72917db3f83f56335333a3e7b40210c289f58a0c
Reviewed-on: https://chromium-review.googlesource.com/988213
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547440}

ash/shell.cc

@@ -486,10 +486,6 @@ void Shell::UpdateShelfVisibility() {
     Shelf::ForWindow(root)->UpdateVisibilityState();
 }
 
-PrefService* Shell::GetLocalStatePrefService() const {
-  return local_state_.get();
-}
-
 WebNotificationTray* Shell::GetWebNotificationTray() {
   return GetPrimaryRootWindowController()
       ->GetStatusAreaWidget()

ash/shell.h

@@ -509,10 +509,6 @@ class ASH_EXPORT Shell : public SessionObserver,
   // TODO(jamescook): Move to Shelf.
   void UpdateShelfVisibility();
 
-  // 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;
-
   // Returns WebNotificationTray on the primary root window.
   WebNotificationTray* GetWebNotificationTray();
 

ash/shell_unittest.cc

@@ -567,7 +567,7 @@ TEST_F(ShellLocalStateTest, LocalState) {
   TestingPrefServiceSimple* local_state_ptr = local_state.get();
   ShellTestApi().OnLocalStatePrefServiceInitialized(std::move(local_state));
   EXPECT_EQ(local_state_ptr, observer.last_local_state_);
-  EXPECT_EQ(local_state_ptr, Shell::Get()->GetLocalStatePrefService());
+  EXPECT_EQ(local_state_ptr, ash_test_helper()->GetLocalStatePrefService());
 
   Shell::Get()->RemoveShellObserver(&observer);
 }

ash/system/bluetooth/bluetooth_power_controller_unittest.cc

@@ -356,7 +356,7 @@ TEST_F(BluetoothPowerControllerIntegrationTest, Basics) {
   device::BluetoothAdapter* adapter = GetBluetoothAdapter();
 
   // Verify toggling bluetooth before login.
-  PrefService* local_state = Shell::Get()->GetLocalStatePrefService();
+  PrefService* local_state = ash_test_helper()->GetLocalStatePrefService();
   GetController()->ToggleBluetoothEnabled();
   EXPECT_TRUE(local_state->GetBoolean(prefs::kSystemBluetoothAdapterEnabled));
   EXPECT_TRUE(adapter->IsPowered());

ash/system/palette/palette_tray_unittest.cc

@@ -83,7 +83,7 @@ class PaletteTrayTest : public AshTestBase {
   }
 
   PrefService* local_state_pref_service() {
-    return Shell::Get()->GetLocalStatePrefService();
+    return ash_test_helper()->GetLocalStatePrefService();
   }
 
   PaletteTray* palette_tray_ = nullptr;  // not owned

ash/test/ash_test_helper.cc

@@ -269,6 +269,10 @@ void AshTestHelper::NotifyClientAboutAcceleratedWidgets() {
       shell->display_manager());
 }
 
+PrefService* AshTestHelper::GetLocalStatePrefService() {
+  return Shell::Get()->local_state_.get();
+}
+
 aura::Window* AshTestHelper::CurrentContext() {
   aura::Window* root_window = Shell::GetRootWindowForNewWindows();
   if (!root_window)

ash/test/ash_test_helper.h

@@ -19,6 +19,8 @@
 #include "base/test/scoped_command_line.h"
 #include "ui/aura/test/mus/test_window_tree_client_setup.h"
 
+class PrefService;
+
 namespace aura {
 class Window;
 class WindowTreeClientPrivate;
@@ -89,6 +91,8 @@ class AshTestHelper {
 
   void NotifyClientAboutAcceleratedWidgets();
 
+  PrefService* GetLocalStatePrefService();
+
   TestShellDelegate* test_shell_delegate() { return test_shell_delegate_; }
   void set_test_shell_delegate(TestShellDelegate* test_shell_delegate) {
     test_shell_delegate_ = test_shell_delegate;

ash/wallpaper/wallpaper_controller.cc

@@ -82,44 +82,6 @@ constexpr int kCompositorLockTimeoutMs = 750;
 // Default quality for encoding wallpaper.
 const int kDefaultEncodingQuality = 90;
 
-// Caches color calculation results in local state pref service.
-void CacheProminentColors(const std::vector<SkColor>& colors,
-                          const std::string& current_location) {
-  // Local state can be null in tests.
-  if (!Shell::Get()->GetLocalStatePrefService())
-    return;
-  DictionaryPrefUpdate wallpaper_colors_update(
-      Shell::Get()->GetLocalStatePrefService(), prefs::kWallpaperColors);
-  auto wallpaper_colors = std::make_unique<base::ListValue>();
-  for (SkColor color : colors)
-    wallpaper_colors->AppendDouble(static_cast<double>(color));
-  wallpaper_colors_update->SetWithoutPathExpansion(current_location,
-                                                   std::move(wallpaper_colors));
-}
-
-// Gets prominent color cache from local state pref service. Returns an empty
-// value if cache is not available.
-base::Optional<std::vector<SkColor>> GetCachedColors(
-    const std::string& current_location) {
-  base::Optional<std::vector<SkColor>> cached_colors_out;
-  const base::ListValue* prominent_colors = nullptr;
-  // Local state can be null in tests.
-  if (!Shell::Get()->GetLocalStatePrefService() ||
-      !Shell::Get()
-           ->GetLocalStatePrefService()
-           ->GetDictionary(prefs::kWallpaperColors)
-           ->GetListWithoutPathExpansion(current_location, &prominent_colors)) {
-    return cached_colors_out;
-  }
-  cached_colors_out = std::vector<SkColor>();
-  for (base::ListValue::const_iterator iter = prominent_colors->begin();
-       iter != prominent_colors->end(); ++iter) {
-    cached_colors_out.value().push_back(
-        static_cast<SkColor>(iter->GetDouble()));
-  }
-  return cached_colors_out;
-}
-
 // Returns true if a color should be extracted from the wallpaper based on the
 // command kAshShelfColor line arg.
 bool IsShelfColoringEnabled() {
@@ -468,7 +430,7 @@ base::FilePath WallpaperController::GetCustomWallpaperDir(
     const std::string& sub_dir) {
   DCHECK(!dir_chrome_os_custom_wallpapers_path.empty());
   return dir_chrome_os_custom_wallpapers_path.Append(sub_dir);
-  }
+}
 
 // static
 bool WallpaperController::ResizeAndSaveWallpaper(const gfx::ImageSkia& image,
@@ -840,6 +802,7 @@ void WallpaperController::OnRootWindowAdded(aura::Window* root_window) {
 
 void WallpaperController::OnLocalStatePrefServiceInitialized(
     PrefService* pref_service) {
+  local_state_ = pref_service;
   if (wallpaper_controller_client_) {
     wallpaper_controller_client_->OnReadyToSetWallpaper();
   } else {
@@ -927,20 +890,20 @@ bool WallpaperController::SetUserWallpaperInfo(const AccountId& account_id,
     return true;
   }
 
-  PrefService* local_state = Shell::Get()->GetLocalStatePrefService();
-  // Local state can be null in tests.
-  if (!local_state)
+  if (!local_state_)
     return false;
+
   WallpaperInfo old_info;
   if (GetUserWallpaperInfo(account_id, &old_info, is_ephemeral)) {
     // Remove the color cache of the previous wallpaper if it exists.
-    DictionaryPrefUpdate wallpaper_colors_update(local_state,
+    DictionaryPrefUpdate wallpaper_colors_update(local_state_,
                                                  prefs::kWallpaperColors);
     wallpaper_colors_update->RemoveWithoutPathExpansion(old_info.location,
                                                         nullptr);
   }
 
-  DictionaryPrefUpdate wallpaper_update(local_state, prefs::kUserWallpaperInfo);
+  DictionaryPrefUpdate wallpaper_update(local_state_,
+                                        prefs::kUserWallpaperInfo);
   auto wallpaper_info_dict = std::make_unique<base::DictionaryValue>();
   wallpaper_info_dict->SetString(
       kNewWallpaperDateNodeName,
@@ -967,12 +930,10 @@ bool WallpaperController::GetUserWallpaperInfo(const AccountId& account_id,
     return true;
   }
 
-  PrefService* local_state = Shell::Get()->GetLocalStatePrefService();
-  // Local state can be null in tests.
-  if (!local_state)
+  if (!local_state_)
     return false;
   const base::DictionaryValue* info_dict;
-  if (!local_state->GetDictionary(prefs::kUserWallpaperInfo)
+  if (!local_state_->GetDictionary(prefs::kUserWallpaperInfo)
            ->GetDictionaryWithoutPathExpansion(account_id.GetUserEmail(),
                                                &info_dict)) {
     return false;
@@ -1425,8 +1386,9 @@ void WallpaperController::OnColorCalculationComplete() {
   // the |kWallpaperColors| pref.
   // TODO(crbug.com/787134): The |prominent_colors_| of wallpapers with empty
   // location should be cached as well.
-  if (!current_wallpaper_->wallpaper_info().location.empty())
+  if (!current_wallpaper_->wallpaper_info().location.empty()) {
     CacheProminentColors(colors, current_wallpaper_->wallpaper_info().location);
+  }
   SetProminentColors(colors);
 }
 
@@ -1514,18 +1476,16 @@ void WallpaperController::RemoveUserWallpaperInfo(const AccountId& account_id,
   if (wallpaper_cache_map_.find(account_id) != wallpaper_cache_map_.end())
     wallpaper_cache_map_.erase(account_id);
 
-  PrefService* local_state = Shell::Get()->GetLocalStatePrefService();
-  // Local state can be null in tests.
-  if (!local_state)
+  if (!local_state_)
     return;
   WallpaperInfo info;
   GetUserWallpaperInfo(account_id, &info, is_ephemeral);
-  DictionaryPrefUpdate prefs_wallpapers_info_update(local_state,
+  DictionaryPrefUpdate prefs_wallpapers_info_update(local_state_,
                                                     prefs::kUserWallpaperInfo);
   prefs_wallpapers_info_update->RemoveWithoutPathExpansion(
       account_id.GetUserEmail(), nullptr);
   // Remove the color cache of the previous wallpaper if it exists.
-  DictionaryPrefUpdate wallpaper_colors_update(local_state,
+  DictionaryPrefUpdate wallpaper_colors_update(local_state_,
                                                prefs::kWallpaperColors);
   wallpaper_colors_update->RemoveWithoutPathExpansion(info.location, nullptr);
 }
@@ -1839,6 +1799,38 @@ bool WallpaperController::ShouldCalculateColors() const {
          !image.isNull();
 }
 
+void WallpaperController::CacheProminentColors(
+    const std::vector<SkColor>& colors,
+    const std::string& current_location) {
+  if (!local_state_)
+    return;
+  DictionaryPrefUpdate wallpaper_colors_update(local_state_,
+                                               prefs::kWallpaperColors);
+  auto wallpaper_colors = std::make_unique<base::ListValue>();
+  for (SkColor color : colors)
+    wallpaper_colors->AppendDouble(static_cast<double>(color));
+  wallpaper_colors_update->SetWithoutPathExpansion(current_location,
+                                                   std::move(wallpaper_colors));
+}
+
+base::Optional<std::vector<SkColor>> WallpaperController::GetCachedColors(
+    const std::string& current_location) {
+  base::Optional<std::vector<SkColor>> cached_colors_out;
+  const base::ListValue* prominent_colors = nullptr;
+  if (!local_state_ ||
+      !local_state_->GetDictionary(prefs::kWallpaperColors)
+           ->GetListWithoutPathExpansion(current_location, &prominent_colors)) {
+    return cached_colors_out;
+  }
+  cached_colors_out = std::vector<SkColor>();
+  for (base::ListValue::const_iterator iter = prominent_colors->begin();
+       iter != prominent_colors->end(); ++iter) {
+    cached_colors_out.value().push_back(
+        static_cast<SkColor>(iter->GetDouble()));
+  }
+  return cached_colors_out;
+}
+
 bool WallpaperController::MoveToLockedContainer() {
   if (locked_)
     return false;

ash/wallpaper/wallpaper_controller.h

@@ -496,6 +496,15 @@ class ASH_EXPORT WallpaperController : public mojom::WallpaperController,
   // system state (e.g. wallpaper image, SessionState, etc.).
   bool ShouldCalculateColors() const;
 
+  // Caches color calculation results in the local state pref service.
+  void CacheProminentColors(const std::vector<SkColor>& colors,
+                            const std::string& current_location);
+
+  // Gets prominent color cache from the local state pref service. Returns an
+  // empty value if the cache is not available.
+  base::Optional<std::vector<SkColor>> GetCachedColors(
+      const std::string& current_location);
+
   // Move all wallpaper widgets to the locked container.
   // Returns true if the wallpaper moved.
   bool MoveToLockedContainer();
@@ -614,6 +623,11 @@ class ASH_EXPORT WallpaperController : public mojom::WallpaperController,
   // default wallpaper decoding is initiated.)
   std::vector<base::FilePath> decode_requests_for_testing_;
 
+  // PrefService provided by Shell::OnLocalStatePrefServiceInitialized.
+  // Valid for the lifetime of ash::Shell which owns WallpaperController.
+  // May be null during intialization or in tests.
+  PrefService* local_state_ = nullptr;
+
   base::WeakPtrFactory<WallpaperController> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(WallpaperController);