ash: Avoid reloading already-resized custom wallpaper.

Avoid a white flash at startup caused by
DesktopBackgroundController comparing the resized version of
a custom wallpaper image against the original image, seeing
that they are different, and then resizing the original
image again.

BUG=321402

Review URL: https://codereview.chromium.org/81393004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@236852 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 151 insertions, 52 deletions

diff --git a/ash/desktop_background/desktop_background_controller.cc b/ash/desktop_background/desktop_background_controller.cc
index 2f4e633..d1c67a9 100644
--- a/ash/desktop_background/desktop_background_controller.cc
+++ b/ash/desktop_background/desktop_background_controller.cc
@@ -169,7 +169,7 @@ DesktopBackgroundController::~DesktopBackgroundController() {
 
 gfx::ImageSkia DesktopBackgroundController::GetWallpaper() const {
   if (current_wallpaper_)
-    return current_wallpaper_->wallpaper_image();
+    return current_wallpaper_->image();
   return gfx::ImageSkia();
 }
 
@@ -189,18 +189,18 @@ WallpaperLayout DesktopBackgroundController::GetWallpaperLayout() const {
   return WALLPAPER_LAYOUT_CENTER_CROPPED;
 }
 
-void DesktopBackgroundController::OnRootWindowAdded(
-    aura::Window* root_window) {
+void DesktopBackgroundController::OnRootWindowAdded(aura::Window* root_window) {
   // The background hasn't been set yet.
   if (desktop_background_mode_ == BACKGROUND_NONE)
     return;
-  gfx::Size max_display_size = GetMaxDisplaySizeInNative();
+
   // Handle resolution change for "built-in" images.
-  if (BACKGROUND_IMAGE == desktop_background_mode_ &&
-      current_wallpaper_.get() &&
-      current_max_display_size_ != max_display_size) {
+  gfx::Size max_display_size = GetMaxDisplaySizeInNative();
+  if (current_max_display_size_ != max_display_size) {
     current_max_display_size_ = max_display_size;
-    UpdateWallpaper();
+    if (desktop_background_mode_ == BACKGROUND_IMAGE &&
+        current_wallpaper_.get())
+      UpdateWallpaper();
   }
 
   InstallDesktopController(root_window);
@@ -306,11 +306,14 @@ void DesktopBackgroundController::OnDisplayConfigurationChanged() {
   gfx::Size max_display_size = GetMaxDisplaySizeInNative();
   if (current_max_display_size_ != max_display_size) {
     current_max_display_size_ = max_display_size;
-    timer_.Stop();
-    timer_.Start(FROM_HERE,
-                 base::TimeDelta::FromMilliseconds(wallpaper_reload_delay_),
-                 this,
-                 &DesktopBackgroundController::UpdateWallpaper);
+    if (desktop_background_mode_ == BACKGROUND_IMAGE &&
+        current_wallpaper_.get()) {
+      timer_.Stop();
+      timer_.Start(FROM_HERE,
+                   base::TimeDelta::FromMilliseconds(wallpaper_reload_delay_),
+                   this,
+                   &DesktopBackgroundController::UpdateWallpaper);
+    }
   }
 }
 
@@ -327,7 +330,8 @@ bool DesktopBackgroundController::DefaultWallpaperIsAlreadyLoadingOrLoaded(
 bool DesktopBackgroundController::CustomWallpaperIsAlreadyLoaded(
     const gfx::ImageSkia& image) const {
   return current_wallpaper_.get() &&
-      current_wallpaper_->wallpaper_image().BackedBySameObjectAs(image);
+      (WallpaperResizer::GetImageId(image) ==
+       current_wallpaper_->original_image_id());
 }
 
 void DesktopBackgroundController::SetDesktopBackgroundImageMode() {
diff --git a/ash/desktop_background/desktop_background_controller_unittest.cc b/ash/desktop_background/desktop_background_controller_unittest.cc
index 6a7d19c..5f136d0 100644
--- a/ash/desktop_background/desktop_background_controller_unittest.cc
+++ b/ash/desktop_background/desktop_background_controller_unittest.cc
@@ -21,7 +21,9 @@
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/message_loop/message_loop.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_utils.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/aura/root_window.h"
@@ -131,6 +133,15 @@ class DesktopBackgroundControllerTest : public test::AshTestBase {
   // and writing images.
   static const int kWallpaperSize = 2;
 
+  // Creates an image of size |size|.
+  gfx::ImageSkia CreateImage(int width, int height) {
+    SkBitmap bitmap;
+    bitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height);
+    bitmap.allocPixels();
+    gfx::ImageSkia image = gfx::ImageSkia::CreateFrom1xBitmap(bitmap);
+    return image;
+  }
+
   // Runs kAnimatingDesktopController's animation to completion.
   // TODO(bshe): Don't require tests to run animations; it's slow.
   void RunDesktopControllerAnimation() {
@@ -426,82 +437,76 @@ TEST_F(DesktopBackgroundControllerTest, DisplayChange) {
   if (!SupportsHostWindowResize())
     return;
 
-  test::DisplayManagerTestApi display_manager_test_api(
-      Shell::GetInstance()->display_manager());
+  // Set the wallpaper to ensure that UpdateWallpaper() will be called when the
+  // display configuration changes.
+  gfx::ImageSkia image = CreateImage(640, 480);
+  wallpaper_delegate_->set_custom_wallpaper(image);
+  controller_->SetCustomWallpaper(image, WALLPAPER_LAYOUT_STRETCH);
 
   // Small wallpaper images should be used for configurations less than or
   // equal to kSmallWallpaperMaxWidth by kSmallWallpaperMaxHeight, even if
   // multiple displays are connected.
+  test::DisplayManagerTestApi display_manager_test_api(
+      Shell::GetInstance()->display_manager());
   display_manager_test_api.UpdateDisplay("800x600");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_SMALL,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(0,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(0, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   display_manager_test_api.UpdateDisplay("800x600,800x600");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_SMALL,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(0,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(0, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   display_manager_test_api.UpdateDisplay("1366x800");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_SMALL,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(1,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(1, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   // At larger sizes, large wallpapers should be used.
   display_manager_test_api.UpdateDisplay("1367x800");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_LARGE,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(1,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(1, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   display_manager_test_api.UpdateDisplay("1367x801");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_LARGE,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(1,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(1, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   display_manager_test_api.UpdateDisplay("2560x1700");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_LARGE,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(1,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(1, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   // Rotated smaller screen may use larger image.
   display_manager_test_api.UpdateDisplay("800x600/r");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_SMALL,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(1,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(1, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   display_manager_test_api.UpdateDisplay("800x600/r,800x600");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_SMALL,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(1,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(1, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
   display_manager_test_api.UpdateDisplay("1366x800/r");
   RunAllPendingInMessageLoop();
   EXPECT_EQ(WALLPAPER_RESOLUTION_LARGE,
             controller_->GetAppropriateResolution());
-  EXPECT_EQ(1,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
+  EXPECT_EQ(1, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 
   // Max display size didn't chagne.
   display_manager_test_api.UpdateDisplay("900x800/r,400x1366");
   RunAllPendingInMessageLoop();
-  EXPECT_EQ(0,
-            wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
-
+  EXPECT_EQ(0, wallpaper_delegate_->GetUpdateWallpaperCountAndReset());
 }
 
 // Test that DesktopBackgroundController loads the appropriate wallpaper
@@ -589,6 +594,35 @@ TEST_F(DesktopBackgroundControllerTest, LargeGuestWallpaper) {
                                kLargeGuestWallpaperColor));
 }
 
+TEST_F(DesktopBackgroundControllerTest, ResizeCustomWallpaper) {
+  if (!SupportsMultipleDisplays())
+    return;
+
+  test::DisplayManagerTestApi display_manager_test_api(
+      Shell::GetInstance()->display_manager());
+  display_manager_test_api.UpdateDisplay("320x200");
+
+  gfx::ImageSkia image = CreateImage(640, 480);
+
+  // Set the image as custom wallpaper, wait for the resize to finish, and check
+  // that the resized image is the expected size.
+  controller_->SetCustomWallpaper(image, WALLPAPER_LAYOUT_STRETCH);
+  EXPECT_TRUE(image.BackedBySameObjectAs(controller_->GetWallpaper()));
+  content::BrowserThread::GetBlockingPool()->FlushForTesting();
+  content::RunAllPendingInMessageLoop();
+  gfx::ImageSkia resized_image = controller_->GetWallpaper();
+  EXPECT_FALSE(image.BackedBySameObjectAs(resized_image));
+  EXPECT_EQ(gfx::Size(320, 200).ToString(), resized_image.size().ToString());
+
+  // Load the original wallpaper again and check that we're still using the
+  // previously-resized image instead of doing another resize
+  // (http://crbug.com/321402).
+  controller_->SetCustomWallpaper(image, WALLPAPER_LAYOUT_STRETCH);
+  content::BrowserThread::GetBlockingPool()->FlushForTesting();
+  content::RunAllPendingInMessageLoop();
+  EXPECT_TRUE(resized_image.BackedBySameObjectAs(controller_->GetWallpaper()));
+}
+
 TEST_F(DesktopBackgroundControllerTest, GetMaxDisplaySize) {
   // Device scale factor shouldn't affect the native size.
   UpdateDisplay("1000x300*2");
diff --git a/ash/desktop_background/wallpaper_resizer.cc b/ash/desktop_background/wallpaper_resizer.cc
index b75e262..06d8c97 100644
--- a/ash/desktop_background/wallpaper_resizer.cc
+++ b/ash/desktop_background/wallpaper_resizer.cc
@@ -10,7 +10,9 @@
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/worker_pool.h"
 #include "content/public/browser/browser_thread.h"
+#include "third_party/skia/include/core/SkImage.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/image/image_skia_rep.h"
 #include "ui/gfx/skia_util.h"
 
 using content::BrowserThread;
@@ -91,23 +93,33 @@ void Resize(SkBitmap orig_bitmap,
 
 }  // namespace
 
+// static
+uint32_t WallpaperResizer::GetImageId(const gfx::ImageSkia& image) {
+  const gfx::ImageSkiaRep& image_rep = image.GetRepresentation(1.0f);
+  return image_rep.is_null() ? 0 : image_rep.sk_bitmap().getGenerationID();
+}
+
 WallpaperResizer::WallpaperResizer(int image_resource_id,
                                    const gfx::Size& target_size,
                                    WallpaperLayout layout)
-    : wallpaper_image_(*(ui::ResourceBundle::GetSharedInstance().
+    : image_(*(ui::ResourceBundle::GetSharedInstance().
           GetImageNamed(image_resource_id).ToImageSkia())),
+      original_image_id_(GetImageId(image_)),
       target_size_(target_size),
       layout_(layout),
       weak_ptr_factory_(this) {
+  image_.MakeThreadSafe();
 }
 
 WallpaperResizer::WallpaperResizer(const gfx::ImageSkia& image,
                                    const gfx::Size& target_size,
                                    WallpaperLayout layout)
-    : wallpaper_image_(image),
+    : image_(image),
+      original_image_id_(GetImageId(image_)),
       target_size_(target_size),
       layout_(layout),
       weak_ptr_factory_(this) {
+  image_.MakeThreadSafe();
 }
 
 WallpaperResizer::~WallpaperResizer() {
@@ -118,7 +130,7 @@ void WallpaperResizer::StartResize() {
   SkBitmap* resized_bitmap = new SkBitmap;
   if (!content::BrowserThread::PostBlockingPoolTaskAndReply(
           FROM_HERE,
-          base::Bind(&Resize, *wallpaper_image_.bitmap(), target_size_,
+          base::Bind(&Resize, *image_.bitmap(), target_size_,
                      layout_, resized_bitmap),
           base::Bind(&WallpaperResizer::OnResizeFinished,
                      weak_ptr_factory_.GetWeakPtr(),
@@ -138,7 +150,7 @@ void WallpaperResizer::RemoveObserver(WallpaperResizerObserver* observer) {
 
 void WallpaperResizer::OnResizeFinished(SkBitmap* resized_bitmap) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  wallpaper_image_ = gfx::ImageSkia::CreateFrom1xBitmap(*resized_bitmap);
+  image_ = gfx::ImageSkia::CreateFrom1xBitmap(*resized_bitmap);
   FOR_EACH_OBSERVER(WallpaperResizerObserver, observers_,
                     OnWallpaperResized());
 }
diff --git a/ash/desktop_background/wallpaper_resizer.h b/ash/desktop_background/wallpaper_resizer.h
index e39dcd5..fadfb44 100644
--- a/ash/desktop_background/wallpaper_resizer.h
+++ b/ash/desktop_background/wallpaper_resizer.h
@@ -21,6 +21,11 @@ class WallpaperResizerObserver;
 // Stores the current wallpaper data and resize it to |target_size| if needed.
 class ASH_EXPORT WallpaperResizer {
  public:
+  // Returns a unique identifier corresponding to |image|, suitable for
+  // comparison against the value returned by original_image_id(). If the image
+  // is modified, its ID will change.
+  static uint32_t GetImageId(const gfx::ImageSkia& image);
+
   WallpaperResizer(int image_resource_id,
                    const gfx::Size& target_size,
                    WallpaperLayout layout);
@@ -31,8 +36,9 @@ class ASH_EXPORT WallpaperResizer {
 
   ~WallpaperResizer();
 
-  const gfx::ImageSkia& wallpaper_image() const { return wallpaper_image_; }
-  const WallpaperLayout layout() const { return layout_; }
+  const gfx::ImageSkia& image() const { return image_; }
+  uint32_t original_image_id() const { return original_image_id_; }
+  WallpaperLayout layout() const { return layout_; }
 
   // Called on the UI thread to run Resize() on the worker pool and post an
   // OnResizeFinished() task back to the UI thread on completion.
@@ -43,8 +49,8 @@ class ASH_EXPORT WallpaperResizer {
   void RemoveObserver(WallpaperResizerObserver* observer);
 
  private:
-  // Copies |resized_bitmap| to |wallpaper_image_| and notifies observers
-  // after Resize() has finished running.
+  // Copies |resized_bitmap| to |image_| and notifies observers after Resize()
+  // has finished running.
   void OnResizeFinished(SkBitmap* resized_bitmap);
 
   ObserverList<WallpaperResizerObserver> observers_;
@@ -52,7 +58,10 @@ class ASH_EXPORT WallpaperResizer {
   // Image that should currently be used for wallpaper. It initially
   // contains the original image and is updated to contain the resized
   // image by OnResizeFinished().
-  gfx::ImageSkia wallpaper_image_;
+  gfx::ImageSkia image_;
+
+  // Unique identifier corresponding to the original (i.e. pre-resize) |image_|.
+  uint32_t original_image_id_;
 
   gfx::Size target_size_;
 
diff --git a/ash/desktop_background/wallpaper_resizer_unittest.cc b/ash/desktop_background/wallpaper_resizer_unittest.cc
index 8447307..7ff0435 100644
--- a/ash/desktop_background/wallpaper_resizer_unittest.cc
+++ b/ash/desktop_background/wallpaper_resizer_unittest.cc
@@ -41,7 +41,8 @@ gfx::ImageSkia CreateTestImage(const gfx::Size& size) {
     }
   }
 
-  return gfx::ImageSkia::CreateFrom1xBitmap(src);
+  gfx::ImageSkia image = gfx::ImageSkia::CreateFrom1xBitmap(src);
+  return image;
 }
 
 bool IsColor(const gfx::ImageSkia& image, const uint32_t expect) {
@@ -74,7 +75,7 @@ class WallpaperResizerTest : public testing::Test,
     resizer->StartResize();
     WaitForResize();
     resizer->RemoveObserver(this);
-    return resizer->wallpaper_image();
+    return resizer->image();
   }
 
   void WaitForResize() {
@@ -141,5 +142,20 @@ TEST_F(WallpaperResizerTest, AllLayoutDifferent) {
   EXPECT_TRUE(IsColor(tile, kExpectedTile));
 }
 
+TEST_F(WallpaperResizerTest, ImageId) {
+  gfx::ImageSkia image = CreateTestImage(
+      gfx::Size(kTestImageWidth, kTestImageHeight));
+
+  // Create a WallpaperResizer and check that it reports an original image ID
+  // both pre- and post-resize that matches the ID returned by GetImageId().
+  WallpaperResizer resizer(image, gfx::Size(10, 20), WALLPAPER_LAYOUT_STRETCH);
+  EXPECT_EQ(WallpaperResizer::GetImageId(image), resizer.original_image_id());
+  resizer.AddObserver(this);
+  resizer.StartResize();
+  WaitForResize();
+  resizer.RemoveObserver(this);
+  EXPECT_EQ(WallpaperResizer::GetImageId(image), resizer.original_image_id());
+}
+
 }  // namespace internal
 }  // namespace ash
diff --git a/ash/test/test_user_wallpaper_delegate.cc b/ash/test/test_user_wallpaper_delegate.cc
index c3eb16e..4cef29b 100644
--- a/ash/test/test_user_wallpaper_delegate.cc
+++ b/ash/test/test_user_wallpaper_delegate.cc
@@ -4,11 +4,23 @@
 
 #include "ash/test/test_user_wallpaper_delegate.h"
 
+#include "ash/desktop_background/desktop_background_controller.h"
+#include "ash/shell.h"
+
 namespace ash {
 namespace test {
 
+TestUserWallpaperDelegate::TestUserWallpaperDelegate()
+    : update_wallpaper_count_(0) {}
+
+TestUserWallpaperDelegate::~TestUserWallpaperDelegate() {}
+
 void TestUserWallpaperDelegate::UpdateWallpaper() {
   DefaultUserWallpaperDelegate::UpdateWallpaper();
+  if (!custom_wallpaper_.isNull()) {
+    Shell::GetInstance()->desktop_background_controller()->SetCustomWallpaper(
+        custom_wallpaper_, WALLPAPER_LAYOUT_STRETCH);
+  }
   update_wallpaper_count_ ++;
 }
 
diff --git a/ash/test/test_user_wallpaper_delegate.h b/ash/test/test_user_wallpaper_delegate.h
index 56842357..83c2583 100644
--- a/ash/test/test_user_wallpaper_delegate.h
+++ b/ash/test/test_user_wallpaper_delegate.h
@@ -7,22 +7,33 @@
 
 #include "ash/default_user_wallpaper_delegate.h"
 
+#include "ui/gfx/image/image_skia.h"
+
 namespace ash {
 namespace test {
 
 class TestUserWallpaperDelegate : public DefaultUserWallpaperDelegate {
  public:
-  TestUserWallpaperDelegate() : update_wallpaper_count_(0) {}
-  virtual ~TestUserWallpaperDelegate() {}
+  TestUserWallpaperDelegate();
+  virtual ~TestUserWallpaperDelegate();
+
+  void set_custom_wallpaper(const gfx::ImageSkia& wallpaper) {
+    custom_wallpaper_ = wallpaper;
+  }
 
   // DefaultUserWallpaperDelegate overrides:
   virtual void UpdateWallpaper() OVERRIDE;
 
+  // Returns and clears |update_wallpaper_count_|.
   int GetUpdateWallpaperCountAndReset();
 
  private:
+  // Number of times that UpdateWallpaper() has been called.
   int update_wallpaper_count_;
 
+  // If non-null, used as custom wallpaper by UpdateWallpaper().
+  gfx::ImageSkia custom_wallpaper_;
+
   DISALLOW_COPY_AND_ASSIGN(TestUserWallpaperDelegate);
 };
 
diff --git a/chrome/browser/chromeos/login/wallpaper_manager_browsertest.cc b/chrome/browser/chromeos/login/wallpaper_manager_browsertest.cc
index ec24f4b..55c10b7 100644
--- a/chrome/browser/chromeos/login/wallpaper_manager_browsertest.cc
+++ b/chrome/browser/chromeos/login/wallpaper_manager_browsertest.cc
@@ -27,6 +27,7 @@
 #include "chrome/test/base/testing_browser_process.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/cryptohome_client.h"
+#include "content/public/test/test_utils.h"
 #include "ui/aura/env.h"
 #include "ui/base/resource/resource_bundle.h"
 
@@ -181,9 +182,9 @@ IN_PROC_BROWSER_TEST_F(WallpaperManagerBrowserTest,
   EXPECT_EQ(kSmallWallpaperWidth, wallpaper.width());
   EXPECT_EQ(kSmallWallpaperHeight, wallpaper.height());
 
-  // Hook up another 800x600 display.
+  // Hook up another 800x600 display. This shouldn't trigger a reload.
   UpdateDisplay("800x600,800x600");
-  WaitAsyncWallpaperLoad();
+  content::RunAllPendingInMessageLoop();
   // The small resolution custom wallpaper is expected.
   EXPECT_EQ(kSmallWallpaperWidth, wallpaper.width());
   EXPECT_EQ(kSmallWallpaperHeight, wallpaper.height());