Remove use of index in wallpaper picker code and some refactor

ash/desktop_background/desktop_background_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/desktop_background/desktop_background_controller.h"
 
 #include "ash/desktop_background/desktop_background_controller_observer.h"
 #include "ash/desktop_background/desktop_background_view.h"
 #include "ash/desktop_background/desktop_background_widget_controller.h"
 #include "ash/desktop_background/user_wallpaper_delegate.h"
 #include "ash/root_window_controller.h"
 #include "ash/shell.h"
 #include "ash/shell_factory.h"
 #include "ash/shell_window_ids.h"
 #include "ash/wm/root_window_layout_manager.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #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"
 #include "ui/compositor/layer.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/image/image.h"
 #include "ui/views/widget/widget.h"
 
 using ash::internal::DesktopBackgroundWidgetController;
 using ash::internal::kAnimatingDesktopController;
 using ash::internal::kDesktopController;
 using content::BrowserThread;
 
 namespace ash {
 namespace {
 
 const SkColor kTransparentColor = SkColorSetARGB(0x00, 0x00, 0x00, 0x00);
 
 internal::RootWindowLayoutManager* GetRootWindowLayoutManager(
     aura::RootWindow* root_window) {
   return static_cast<internal::RootWindowLayoutManager*>(
       root_window->layout_manager());
 }
 }  // 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;
 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;
 };
 
 // DesktopBackgroundController::WallpaperOperation wraps background wallpaper
 // loading.
 class DesktopBackgroundController::WallpaperOperation

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

If this loads, shouldn't the name be WallpaperLoader or something?

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

Done.

     : 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) {
     wo->LoadingWallpaper();

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

Add DCHECK not on UI thread. Similarly naming this to make it clear it isn't
invoked on the UI thread would be nice.

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

Done.

   }
 
   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() {
     return wallpaper_data_.release();
   }
 
  private:
   friend class base::RefCountedThreadSafe<
       DesktopBackgroundController::WallpaperOperation>;
 
   ~WallpaperOperation() {}
 
   base::CancellationFlag cancel_flag_;
 
   scoped_ptr<WallpaperData> wallpaper_data_;
 
-  const int index_;
-
-  const WallpaperResolution resolution_;
+  const WallpaperInfo info_;
 
   DISALLOW_COPY_AND_ASSIGN(WallpaperOperation);
 };
 
 DesktopBackgroundController::DesktopBackgroundController()
     : locked_(false),
       desktop_background_mode_(BACKGROUND_SOLID_COLOR),
       background_color_(kTransparentColor),
       weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
@@ skipping 23 matching lines @@
     return current_wallpaper_->wallpaper_layout;
   return CENTER_CROPPED;
 }
 
 gfx::ImageSkia DesktopBackgroundController::GetCurrentWallpaperImage() {
   if (desktop_background_mode_ != BACKGROUND_IMAGE)
     return gfx::ImageSkia();
   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."
   if (BACKGROUND_IMAGE == desktop_background_mode_ &&
       current_wallpaper_.get()) {
     gfx::Size root_window_size = root_window->GetHostSize();
     // Loads a higher resolution wallpaper if new root window is larger than
     // 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_),
       base::Bind(&DesktopBackgroundController::OnWallpaperLoadCompleted,
                  weak_ptr_factory_.GetWeakPtr(),
                  wallpaper_op_),
       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) {
   CancelPendingWallpaperOperation();
   if (current_wallpaper_.get() &&
       current_wallpaper_->wallpaper_image.BackedBySameObjectAs(wallpaper)) {
     return;
   }
 
   current_wallpaper_.reset(new WallpaperData(layout, wallpaper));
@@ skipping 180 matching lines @@
   }
   return moved;
 }
 
 int DesktopBackgroundController::GetBackgroundContainerId(bool locked) {
   return locked ? internal::kShellWindowId_LockScreenBackgroundContainer :
                   internal::kShellWindowId_DesktopBackgroundContainer;
 }
 
 }  // namespace ash