Remove use of index in wallpaper picker code and some refactor

ash/desktop_background/desktop_background_controller.cc

Index: ash/desktop_background/desktop_background_controller.cc
diff --git a/ash/desktop_background/desktop_background_controller.cc b/ash/desktop_background/desktop_background_controller.cc
index 8575c1e0176941e4d0df8e02ec8923f397ce1539..857ee036905acf5b626f70affb2eee283e817270 100644
--- a/ash/desktop_background/desktop_background_controller.cc
+++ b/ash/desktop_background/desktop_background_controller.cc
@@ -18,6 +18,7 @@
 #include "base/synchronization/cancellation_flag.h"
 #include "base/threading/worker_pool.h"
 #include "content/public/browser/browser_thread.h"
+#include "grit/ash_wallpaper_resources.h"
 #include "ui/aura/root_window.h"
 #include "ui/aura/window.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -43,6 +44,24 @@ internal::RootWindowLayoutManager* GetRootWindowLayoutManager(
 }
 }  // namespace
 
+#if defined(GOOGLE_CHROME_BUILD)
+const WallpaperInfo kDefaultLargeWallpaper =
+    { IDR_AURA_WALLPAPERS_2_LANDSCAPE8_LARGE, CENTER_CROPPED };
+const WallpaperInfo kDefaultSmallWallpaper =
+    { IDR_AURA_WALLPAPERS_2_LANDSCAPE8_SMALL, CENTER };
+const WallpaperInfo kGuestLargeWallpaper =
+    { IDR_AURA_WALLPAPERS_2_LANDSCAPE7_LARGE, CENTER_CROPPED };
+const WallpaperInfo kGuestSmallWallpaper =
+    { IDR_AURA_WALLPAPERS_2_LANDSCAPE7_SMALL, CENTER };
+#else
+const WallpaperInfo kDefaultLargeWallpaper =
+    { IDR_AURA_WALLPAPERS_5_GRADIENT5_LARGE, TILE };
+const WallpaperInfo kDefaultSmallWallpaper =
+    { IDR_AURA_WALLPAPERS_5_GRADIENT5_SMALL, TILE };
+const WallpaperInfo kGuestLargeWallpaper = kDefaultLargeWallpaper;
+const WallpaperInfo kGuestSmallWallpaper = kDefaultSmallWallpaper;
+#endif
+
 const int kSmallWallpaperMaxWidth = 1366;
 const int kSmallWallpaperMaxHeight = 800;
 const int kLargeWallpaperMaxWidth = 2560;
@@ -50,21 +69,18 @@ const int kLargeWallpaperMaxHeight = 1700;
 
 // Stores the current wallpaper data.
 struct DesktopBackgroundController::WallpaperData {
-  WallpaperData(int index, WallpaperResolution resolution)
-      : is_default_wallpaper(true),
-        wallpaper_index(index),
-        wallpaper_layout(GetWallpaperViewInfo(index, resolution).layout),
+  explicit WallpaperData(const WallpaperInfo& info)
+      : wallpaper_idr(info.idr),
+        wallpaper_layout(info.layout),
         wallpaper_image(*(ui::ResourceBundle::GetSharedInstance().GetImageNamed(
-            GetWallpaperViewInfo(index, resolution).id).ToImageSkia())) {
+            info.idr).ToImageSkia())) {
   }
   WallpaperData(WallpaperLayout layout, const gfx::ImageSkia& image)
-      : is_default_wallpaper(false),
-        wallpaper_index(-1),
+      : wallpaper_idr(-1),
         wallpaper_layout(layout),
         wallpaper_image(image) {
   }
-  const bool is_default_wallpaper;
-  const int wallpaper_index;
+  int wallpaper_idr;

[sky, 2012/11/19 22:21:19]

Can this use WallpaperInfo?

[bshe, 2012/11/20 00:33:59]

Done.

   const WallpaperLayout wallpaper_layout;
   const gfx::ImageSkia wallpaper_image;
 };
@@ -75,9 +91,8 @@ class DesktopBackgroundController::WallpaperOperation
     : public base::RefCountedThreadSafe<
           DesktopBackgroundController::WallpaperOperation> {
  public:
-  WallpaperOperation(int index, WallpaperResolution resolution)
-      : index_(index),
-        resolution_(resolution) {
+  explicit WallpaperOperation(const WallpaperInfo& info)
+      : info_(info) {
   }
 
   static void Run(scoped_refptr<WallpaperOperation> wo) {
@@ -87,15 +102,15 @@ class DesktopBackgroundController::WallpaperOperation
   void LoadingWallpaper() {

[sky, 2012/11/19 22:21:19]

Does this need to be public?

[bshe, 2012/11/20 00:33:59]

Done.

     if (cancel_flag_.IsSet())
       return;
-    wallpaper_data_.reset(new WallpaperData(index_, resolution_));
+    wallpaper_data_.reset(new WallpaperData(info_));
   }
 
   void Cancel() {
     cancel_flag_.Set();
   }
 
-  int index() const {
-    return index_;
+  int idr() {

[sky, 2012/11/19 22:21:19]

how come you removed the const?

[bshe, 2012/11/20 00:33:59]

Done.

+    return info_.idr;
   }
 
   WallpaperData* ReleaseWallpaperData() {
@@ -112,9 +127,7 @@ class DesktopBackgroundController::WallpaperOperation
 
   scoped_ptr<WallpaperData> wallpaper_data_;
 
-  const int index_;
-
-  const WallpaperResolution resolution_;
+  const WallpaperInfo info_;
 
   DISALLOW_COPY_AND_ASSIGN(WallpaperOperation);
 };
@@ -158,6 +171,15 @@ gfx::ImageSkia DesktopBackgroundController::GetCurrentWallpaperImage() {
   return GetWallpaper();
 }
 
+int DesktopBackgroundController::GetWallpaperIDR() const {
+  if (wallpaper_op_.get())
+    return wallpaper_op_->idr();
+  else if (current_wallpaper_.get())
+    return current_wallpaper_->wallpaper_idr;
+  else
+    return -1;
+}
+
 void DesktopBackgroundController::OnRootWindowAdded(
     aura::RootWindow* root_window) {
   // Handle resolution change for "built-in" images."
@@ -168,54 +190,20 @@ void DesktopBackgroundController::OnRootWindowAdded(
     // small screen.
     if (kSmallWallpaperMaxWidth < root_window_size.width() ||
         kSmallWallpaperMaxHeight < root_window_size.height()) {
-      if (current_wallpaper_->is_default_wallpaper) {
-        ReloadDefaultWallpaper();
-      } else {
-        ash::Shell::GetInstance()->user_wallpaper_delegate()->
-            UpdateWallpaper();
-      }
+      current_wallpaper_.reset(NULL);
+      ash::Shell::GetInstance()->user_wallpaper_delegate()->
+          UpdateWallpaper();
     }
   }
 
   InstallDesktopController(root_window);
 }
 
-void DesktopBackgroundController::CacheDefaultWallpaper(int index) {
-  DCHECK(index >= 0);
-
-  WallpaperResolution resolution = GetAppropriateResolution();
-  scoped_refptr<WallpaperOperation> wallpaper_op =
-      new WallpaperOperation(index, resolution);
-  base::WorkerPool::PostTask(
-      FROM_HERE,
-      base::Bind(&WallpaperOperation::Run, wallpaper_op),
-      true);
-}
-
-void DesktopBackgroundController::SetDefaultWallpaper(int index) {
-  // We should not change background when index is invalid. For instance, at
-  // login screen or stub_user login.
-  if (index == GetInvalidWallpaperIndex()) {
-    CreateEmptyWallpaper();
-    return;
-  } else if (index == GetSolidColorIndex()) {
-    SetDesktopBackgroundSolidColorMode(kLoginWallpaperColor);
-    return;
-  }
-
-  // Prevents loading of the same wallpaper as the currently loading/loaded
-  // one.
-  if ((wallpaper_op_.get() && wallpaper_op_->index() == index) ||
-      (current_wallpaper_.get() &&
-       current_wallpaper_->wallpaper_index == index)) {
-    return;
-  }
+void DesktopBackgroundController::SetDefaultWallpaper(WallpaperInfo info) {
+  DCHECK(GetWallpaperIDR() != info.idr);

[sky, 2012/11/19 22:21:19]

DCHECK_NE
How come this is a DCHECK? Isn't it possible for someone to invoke twice?

[bshe, 2012/11/20 00:33:59]

I have moved the logic to prevent reloading same wallpaper to
wallpaper_manager.cc file. And this function is called from wallpaper manager
for chrome. So I add a DCHECK here. I am expecting that we shouldn't try to call
this twice on the same wallpaper from wallpaper manager.

For ash_shell in ash/shell/content_client/shell_browser_main_parts.cc. We are
calling it from function PreMainMessageLoopRun, it looks like we should only
call it once as well?




On 2012/11/19 22:21:19, sky wrote:

 
   CancelPendingWallpaperOperation();
-
-  WallpaperResolution resolution = GetAppropriateResolution();
-
-  wallpaper_op_ = new WallpaperOperation(index, resolution);
+  wallpaper_op_ = new WallpaperOperation(info);
   base::WorkerPool::PostTaskAndReply(
       FROM_HERE,
       base::Bind(&WallpaperOperation::Run, wallpaper_op_),
@@ -225,12 +213,6 @@ void DesktopBackgroundController::SetDefaultWallpaper(int index) {
       true /* task_is_slow */);
 }
 
-void DesktopBackgroundController::ReloadDefaultWallpaper() {
-  int index = current_wallpaper_->wallpaper_index;
-  current_wallpaper_.reset(NULL);
-  SetDefaultWallpaper(index);
-}
-
 void DesktopBackgroundController::SetCustomWallpaper(
     const gfx::ImageSkia& wallpaper,
     WallpaperLayout layout) {