wallpaper refactoring: Add UpdateCustomWallpaperLayout method

Created |UpdateCustomWallpaperLayout| so that |SetWallpaperInfo| is
made private. Also, WallpaperManager::UpdateWallpaper can be deprecated
due to this CL and the previous CL.

Bug: 779221
Change-Id: I27ec8095e0ca21eee8bea3216b930d9bef9e4f4e
Reviewed-on: https://chromium-review.googlesource.com/828696Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527045}

ash/public/interfaces/wallpaper.mojom

@@ -138,6 +138,13 @@ interface WallpaperController {
   // clears the device policy controlled wallpaper if applicable.
   SetDeviceWallpaperPolicyEnforced(bool enforced);
 
+  // Updates the layout for the user's custom wallpaper and reloads the
+  // wallpaper with the new layout.
+  // |user_info|: The user's information related to wallpaper.
+  // |layout|: The new layout of the wallpaper.
+  UpdateCustomWallpaperLayout(WallpaperUserInfo user_info,
+                              WallpaperLayout layout);
+
   // Shows the user's wallpaper, which is determined in the following order:
   // 1) Use device policy wallpaper if it exists AND we are at the login screen.
   // 2) Use user policy wallpaper if it exists.

ash/wallpaper/wallpaper_controller.cc

@@ -1184,6 +1184,32 @@ void WallpaperController::SetDeviceWallpaperPolicyEnforced(bool enforced) {
   }
 }
 
+void WallpaperController::UpdateCustomWallpaperLayout(
+    mojom::WallpaperUserInfoPtr user_info,
+    wallpaper::WallpaperLayout layout) {
+  // This method has a very specific use case: the user should be active and
+  // have a custom wallpaper.
+  // The currently active user has index 0.
+  const mojom::UserSession* const active_user_session =
+      Shell::Get()->session_controller()->GetUserSession(0 /*user index=*/);
+  if (!active_user_session ||
+      active_user_session->user_info->account_id != user_info->account_id) {
+    return;
+  }
+  WallpaperInfo info;
+  if (!GetUserWallpaperInfo(user_info->account_id, &info,
+                            !user_info->is_ephemeral) ||
+      info.type != wallpaper::CUSTOMIZED) {
+    return;
+  }
+  if (info.layout == layout)
+    return;
+
+  info.layout = layout;
+  SetUserWallpaperInfo(user_info->account_id, info, !user_info->is_ephemeral);
+  ShowUserWallpaper(std::move(user_info));
+}
+
 void WallpaperController::ShowUserWallpaper(
     mojom::WallpaperUserInfoPtr user_info) {
   current_user_ = std::move(user_info);

ash/wallpaper/wallpaper_controller.h

@@ -322,8 +322,7 @@ class ASH_EXPORT WallpaperController
   // Returns whether the current wallpaper is blurred.
   bool IsWallpaperBlurred() const { return is_wallpaper_blurred_; }
 
-  // TODO(crbug.com/776464): Make this private. WallpaperInfo should be an
-  // internal concept. In addition, change |is_persistent| to |is_ephemeral| to
+  // TODO(crbug.com/776464): Change |is_persistent| to |is_ephemeral| to
   // be consistent with |mojom::WallpaperUserInfo|.
   // Sets wallpaper info for |account_id| and saves it to local state if
   // |is_persistent| is true.
@@ -402,6 +401,8 @@ class ASH_EXPORT WallpaperController
       const base::FilePath& file_path,
       const base::FilePath& resized_directory) override;
   void SetDeviceWallpaperPolicyEnforced(bool enforced) override;
+  void UpdateCustomWallpaperLayout(mojom::WallpaperUserInfoPtr user_info,
+                                   wallpaper::WallpaperLayout layout) override;
   // TODO(crbug.com/776464): |ShowUserWallpaper| is incomplete until device
   // policy wallpaper is migrated. Callers from chrome should use
   // |WallpaperManager::ShowUserWallpaper| instead. Callers in ash can't use it

ash/wallpaper/wallpaper_controller_unittest.cc

@@ -1301,6 +1301,69 @@ TEST_F(WallpaperControllerTest, PreventReloadingSameWallpaper) {
   EXPECT_EQ(0, GetWallpaperCount());
 }
 
+TEST_F(WallpaperControllerTest, UpdateCustomWallpaperLayout) {
+  gfx::ImageSkia image = CreateImage(640, 480, kSmallCustomWallpaperColor);
+  WallpaperLayout layout = WALLPAPER_LAYOUT_CENTER;
+  wallpaper::WallpaperType type = wallpaper::CUSTOMIZED;
+  SimulateUserLogin(user_1);
+
+  // Set a custom wallpaper for the user. Verify that it's set successfully
+  // and the wallpaper info is updated.
+  controller_->SetCustomWallpaper(
+      InitializeUser(account_id_1), wallpaper_files_id_1, file_name_1, layout,
+      type, *image.bitmap(), true /*show_wallpaper=*/);
+  RunAllTasksUntilIdle();
+  EXPECT_EQ(1, GetWallpaperCount());
+  EXPECT_EQ(controller_->GetWallpaperLayout(), layout);
+  wallpaper::WallpaperInfo wallpaper_info;
+  EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
+                                                true /* is_persistent */));
+  wallpaper::WallpaperInfo expected_wallpaper_info(
+      base::FilePath(wallpaper_files_id_1).Append(file_name_1).value(), layout,
+      type, base::Time::Now().LocalMidnight());
+  EXPECT_EQ(wallpaper_info, expected_wallpaper_info);
+
+  // Now change to a different layout. Verify that the layout is updated for
+  // both the current wallpaper and the saved wallpaper info.
+  layout = WALLPAPER_LAYOUT_CENTER_CROPPED;
+  controller_->UpdateCustomWallpaperLayout(InitializeUser(account_id_1),
+                                           layout);
+  RunAllTasksUntilIdle();
+  EXPECT_EQ(1, GetWallpaperCount());
+  EXPECT_EQ(controller_->GetWallpaperLayout(), layout);
+  EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
+                                                true /* is_persistent */));
+  expected_wallpaper_info.layout = layout;
+  EXPECT_EQ(wallpaper_info, expected_wallpaper_info);
+
+  // Now set an online wallpaper. Verify that it's set successfully and the
+  // wallpaper info is updated.
+  const std::string url = "dummy_url";
+  image = CreateImage(640, 480, kWallpaperColor);
+  controller_->SetOnlineWallpaper(InitializeUser(account_id_1), *image.bitmap(),
+                                  url, layout, true /*show_wallpaper=*/);
+  RunAllTasksUntilIdle();
+  EXPECT_EQ(1, GetWallpaperCount());
+  EXPECT_EQ(controller_->GetWallpaperType(), wallpaper::ONLINE);
+  EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
+                                                true /*is_persistent=*/));
+  expected_wallpaper_info.type = wallpaper::ONLINE;
+  expected_wallpaper_info.location = url;
+  EXPECT_EQ(wallpaper_info, expected_wallpaper_info);
+
+  // Now change the layout of the online wallpaper. Verify that it's a no-op.
+  controller_->UpdateCustomWallpaperLayout(InitializeUser(account_id_1),
+                                           WALLPAPER_LAYOUT_CENTER);
+  RunAllTasksUntilIdle();
+  // The wallpaper is not updated.
+  EXPECT_EQ(0, GetWallpaperCount());
+  EXPECT_EQ(controller_->GetWallpaperLayout(), layout);
+  // The saved wallpaper info is not updated.
+  EXPECT_TRUE(controller_->GetUserWallpaperInfo(account_id_1, &wallpaper_info,
+                                                true /* is_persistent */));
+  EXPECT_EQ(wallpaper_info, expected_wallpaper_info);
+}
+
 // Tests that if a user who has a custom wallpaper is removed from the device,
 // only the directory that contains the user's custom wallpapers gets removed.
 // The other user's custom wallpaper is not affected.

chrome/browser/chromeos/extensions/wallpaper_private_api.cc

@@ -575,24 +575,12 @@ bool WallpaperPrivateSetCustomWallpaperLayoutFunction::RunAsync() {
       set_custom_wallpaper_layout::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params);
 
-  chromeos::WallpaperManager* wallpaper_manager =
-      chromeos::WallpaperManager::Get();
-  wallpaper::WallpaperInfo info;
-  wallpaper_manager->GetLoggedInUserWallpaperInfo(&info);
-  if (info.type != wallpaper::CUSTOMIZED) {
-    SetError("Only custom wallpaper can change layout.");
-    return false;
-  }
-  info.layout = wallpaper_api_util::GetLayoutEnum(
+  wallpaper::WallpaperLayout new_layout = wallpaper_api_util::GetLayoutEnum(
       wallpaper_base::ToString(params->layout));
-  wallpaper_api_util::RecordCustomWallpaperLayout(info.layout);
-
-  const AccountId& account_id =
-      user_manager::UserManager::Get()->GetActiveUser()->GetAccountId();
-  bool is_persistent = !user_manager::UserManager::Get()
-                            ->IsCurrentUserNonCryptohomeDataEphemeral();
-  wallpaper_manager->SetUserWallpaperInfo(account_id, info, is_persistent);
-  wallpaper_manager->UpdateWallpaper(false /* clear_cache */);
+  wallpaper_api_util::RecordCustomWallpaperLayout(new_layout);
+  WallpaperControllerClient::Get()->UpdateCustomWallpaperLayout(
+      user_manager::UserManager::Get()->GetActiveUser()->GetAccountId(),
+      new_layout);
   SendResponse(true);
 
   return true;

chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.cc

@@ -356,20 +356,6 @@ void WallpaperManager::ShowSigninWallpaper() {
   WallpaperControllerClient::Get()->ShowSigninWallpaper();
 }
 
-void WallpaperManager::SetUserWallpaperInfo(const AccountId& account_id,
-                                            const WallpaperInfo& info,
-                                            bool is_persistent) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (!ash::Shell::HasInstance() || ash_util::IsRunningInMash()) {
-    // Some unit tests come here without a Shell instance.
-    // TODO(crbug.com/776464): This is intended not to work under mash. Make it
-    // work again after WallpaperManager is removed.
-    return;
-  }
-  ash::Shell::Get()->wallpaper_controller()->SetUserWallpaperInfo(
-      account_id, info, is_persistent);
-}
-
 void WallpaperManager::InitializeWallpaper() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ -605,19 +591,18 @@ void WallpaperManager::ResizeCustomizedDefaultWallpaper(
       large_wallpaper_image);
 }
 
-void WallpaperManager::InitializeUserWallpaperInfo(
-    const AccountId& account_id) {
+void WallpaperManager::SetUserWallpaperInfo(const AccountId& account_id,
+                                            const WallpaperInfo& info,
+                                            bool is_persistent) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (!ash::Shell::HasInstance() || ash_util::IsRunningInMash()) {
     // Some unit tests come here without a Shell instance.
     // TODO(crbug.com/776464): This is intended not to work under mash. Make it
     // work again after WallpaperManager is removed.
     return;
   }
-  bool is_persistent =
-      !user_manager::UserManager::Get()->IsUserNonCryptohomeDataEphemeral(
-          account_id);
-  ash::Shell::Get()->wallpaper_controller()->InitializeUserWallpaperInfo(
-      account_id, is_persistent);
+  ash::Shell::Get()->wallpaper_controller()->SetUserWallpaperInfo(
+      account_id, info, is_persistent);
 }
 
 bool WallpaperManager::GetWallpaperFromCache(const AccountId& account_id,

chrome/browser/chromeos/login/users/wallpaper/wallpaper_manager.h

@@ -160,11 +160,6 @@ class WallpaperManager : public wm::ActivationChangeObserver,
   // device policy wallpaper or the default wallpaper.
   void ShowSigninWallpaper();
 
-  // A wrapper of |WallpaperController::SetUserWallpaperInfo|.
-  void SetUserWallpaperInfo(const AccountId& account_id,
-                            const wallpaper::WallpaperInfo& info,
-                            bool is_persistent);
-
   // Initializes wallpaper. If logged in, loads user's wallpaper. If not logged
   // in, uses a solid color wallpaper. If logged in as a stub user, uses an
   // empty wallpaper.
@@ -232,8 +227,10 @@ class WallpaperManager : public wm::ActivationChangeObserver,
       gfx::ImageSkia* small_wallpaper_image,
       gfx::ImageSkia* large_wallpaper_image);
 
-  // A wrapper of |WallpaperController::InitializeUserWallpaperInfo|.
-  void InitializeUserWallpaperInfo(const AccountId& account_id);
+  // A wrapper of |WallpaperController::SetUserWallpaperInfo|.
+  void SetUserWallpaperInfo(const AccountId& account_id,
+                            const wallpaper::WallpaperInfo& info,
+                            bool is_persistent);
 
   // A wrapper of |WallpaperController::GetWallpaperFromCache|.
   bool GetWallpaperFromCache(const AccountId& account_id,

chrome/browser/ui/ash/test_wallpaper_controller.cc

@@ -66,6 +66,12 @@ void TestWallpaperController::SetDeviceWallpaperPolicyEnforced(bool enforced) {
   NOTIMPLEMENTED();
 }
 
+void TestWallpaperController::UpdateCustomWallpaperLayout(
+    ash::mojom::WallpaperUserInfoPtr user_info,
+    wallpaper::WallpaperLayout layout) {
+  NOTIMPLEMENTED();
+}
+
 void TestWallpaperController::ShowUserWallpaper(
     ash::mojom::WallpaperUserInfoPtr user_info) {
   NOTIMPLEMENTED();

chrome/browser/ui/ash/test_wallpaper_controller.h

@@ -57,6 +57,8 @@ class TestWallpaperController : ash::mojom::WallpaperController {
       const base::FilePath& file_path,
       const base::FilePath& resized_directory) override;
   void SetDeviceWallpaperPolicyEnforced(bool enforced) override;
+  void UpdateCustomWallpaperLayout(ash::mojom::WallpaperUserInfoPtr user_info,
+                                   wallpaper::WallpaperLayout layout) override;
   void ShowUserWallpaper(ash::mojom::WallpaperUserInfoPtr user_info) override;
   void ShowSigninWallpaper() override;
   void RemoveUserWallpaper(ash::mojom::WallpaperUserInfoPtr user_info,

chrome/browser/ui/ash/wallpaper_controller_client.cc

@@ -191,6 +191,17 @@ void WallpaperControllerClient::SetCustomizedDefaultWallpaper(
                                                        resized_directory);
 }
 
+void WallpaperControllerClient::UpdateCustomWallpaperLayout(
+    const AccountId& account_id,
+    wallpaper::WallpaperLayout layout) {
+  ash::mojom::WallpaperUserInfoPtr user_info =
+      AccountIdToWallpaperUserInfo(account_id);
+  if (!user_info)
+    return;
+  wallpaper_controller_->UpdateCustomWallpaperLayout(std::move(user_info),
+                                                     layout);
+}
+
 void WallpaperControllerClient::ShowUserWallpaper(const AccountId& account_id) {
   ash::mojom::WallpaperUserInfoPtr user_info =
       AccountIdToWallpaperUserInfo(account_id);

chrome/browser/ui/ash/wallpaper_controller_client.h

@@ -56,6 +56,8 @@ class WallpaperControllerClient : public ash::mojom::WallpaperControllerClient,
   void SetCustomizedDefaultWallpaper(const GURL& wallpaper_url,
                                      const base::FilePath& file_path,
                                      const base::FilePath& resized_directory);
+  void UpdateCustomWallpaperLayout(const AccountId& account_id,
+                                   wallpaper::WallpaperLayout layout);
   void ShowUserWallpaper(const AccountId& account_id);
   void ShowSigninWallpaper();
   void RemoveUserWallpaper(const AccountId& account_id);

chrome/test/data/extensions/api_test/wallpaper_manager/test.js

@@ -39,13 +39,8 @@ chrome.test.getConfig(function(config) {
       requestImage(url, function(requestStatus, response) {
         if (requestStatus === 200) {
           wallpaperJpeg = response;
-          chrome.wallpaperPrivate.setWallpaper(wallpaperJpeg,
-                                               'CENTER_CROPPED',
-                                               url,
-                                               pass(function() {
-            chrome.wallpaperPrivate.setCustomWallpaperLayout('CENTER',
-                fail('Only custom wallpaper can change layout.'));
-          }));
+          chrome.wallpaperPrivate.setWallpaper(
+              wallpaperJpeg, 'CENTER_CROPPED', url, pass());
         } else {
           chrome.test.fail('Failed to load test.jpg from local server.');
         }