Fix black wallpaper issue when quickly select a bunch of wallpapers

BUG=155631, 156542
TEST=
(1) Open "set wallpaper" dialog.
(2) Go to the solid colors section.
(3) Quickly click on a color, then on another, then on another.

desktop background should not stuck in a black wallpaper.

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

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

7 files changed, 84 insertions, 2 deletions

diff --git a/ash/desktop_background/desktop_background_controller.cc b/ash/desktop_background/desktop_background_controller.cc
index ca697d6..73ea434 100644
--- a/ash/desktop_background/desktop_background_controller.cc
+++ b/ash/desktop_background/desktop_background_controller.cc
@@ -331,6 +331,10 @@ void DesktopBackgroundController::InstallDesktopController(
   if (!root_window->HasObserver(this))
     root_window->AddObserver(this);
 
+  internal::AnimatingDesktopController* animating_controller =
+      root_window->GetProperty(kAnimatingDesktopController);
+  if (animating_controller)
+    animating_controller->StopAnimating();
   root_window->SetProperty(kAnimatingDesktopController,
                            new internal::AnimatingDesktopController(component));
 }
diff --git a/ash/desktop_background/desktop_background_controller_unittest.cc b/ash/desktop_background/desktop_background_controller_unittest.cc
index 3b04bc4..7857231 100644
--- a/ash/desktop_background/desktop_background_controller_unittest.cc
+++ b/ash/desktop_background/desktop_background_controller_unittest.cc
@@ -178,5 +178,52 @@ TEST_F(DesktopBackgroundControllerTest, BackgroundMovementDuringUnlock) {
   EXPECT_EQ(0, ChildCountForContainer(kLockScreenBackgroundId));
 }
 
+// Test for crbug.com/156542. Animating wallpaper should immediately finish
+// animation and replace current wallpaper before next animation starts.
+TEST_F(DesktopBackgroundControllerTest, ChangeWallpaperQuick) {
+  // We cannot short-circuit animations for this test.
+  ui::LayerAnimator::set_disable_animations_for_test(false);
+
+  // Reset wallpaper state, see ControllerOwnership above.
+  DesktopBackgroundController* controller =
+      Shell::GetInstance()->desktop_background_controller();
+  controller->CreateEmptyWallpaper();
+
+  // Run wallpaper show animation to completion.
+  RootWindow* root = Shell::GetPrimaryRootWindow();
+  RunAnimationForWidget(
+      root->GetProperty(kAnimatingDesktopController)->GetController(false)->
+              widget());
+
+  // Change to a new wallpaper.
+  controller->CreateEmptyWallpaper();
+
+  DesktopBackgroundWidgetController* animatingController =
+      root->GetProperty(kAnimatingDesktopController)->GetController(false);
+  EXPECT_TRUE(animatingController);
+  EXPECT_TRUE(root->GetProperty(kDesktopController));
+
+  // Change to another wallpaper before animation finished.
+  controller->CreateEmptyWallpaper();
+
+  // The animating controller should immediately move to desktop controller.
+  EXPECT_EQ(animatingController, root->GetProperty(kDesktopController));
+
+  // Cache the new animating controller.
+  animatingController =
+      root->GetProperty(kAnimatingDesktopController)->GetController(false);
+
+  // Run wallpaper show animation to completion.
+  RunAnimationForWidget(
+      root->GetProperty(kAnimatingDesktopController)->GetController(false)->
+              widget());
+
+  EXPECT_TRUE(root->GetProperty(kDesktopController));
+  EXPECT_FALSE(
+      root->GetProperty(kAnimatingDesktopController)->GetController(false));
+  // The desktop controller should be the last created animating controller.
+  EXPECT_EQ(animatingController, root->GetProperty(kDesktopController));
+}
+
 }  // namespace internal
 }  // namespace ash
diff --git a/ash/desktop_background/desktop_background_view.cc b/ash/desktop_background/desktop_background_view.cc
index 311532e..aefe985 100644
--- a/ash/desktop_background/desktop_background_view.cc
+++ b/ash/desktop_background/desktop_background_view.cc
@@ -41,6 +41,7 @@ class ShowWallpaperAnimationObserver : public ui::ImplicitAnimationObserver,
   }
 
   virtual ~ShowWallpaperAnimationObserver() {
+    StopObservingImplicitAnimations();
     if (desktop_widget_)
       desktop_widget_->RemoveObserver(this);
   }
@@ -48,7 +49,9 @@ class ShowWallpaperAnimationObserver : public ui::ImplicitAnimationObserver,
  private:
   // Overridden from ui::ImplicitAnimationObserver:
   virtual void OnImplicitAnimationsCompleted() OVERRIDE {
+    DCHECK(desktop_widget_);
     ash::Shell* shell = ash::Shell::GetInstance();
+    // TODO(oshima): fix this for extended desktop.
     shell->GetPrimaryRootWindowController()->HandleDesktopBackgroundVisible();
     shell->user_wallpaper_delegate()->OnWallpaperAnimationFinished();
     // Only removes old component when wallpaper animation finished. If we
@@ -58,6 +61,10 @@ class ShowWallpaperAnimationObserver : public ui::ImplicitAnimationObserver,
       DesktopBackgroundWidgetController* controller =
           root_window_->GetProperty(kAnimatingDesktopController)->
               GetController(true);
+      // |desktop_widget_| should be the same animating widget we try to move
+      // to |kDesktopController|. Otherwise, we may close |desktop_widget_|
+      // before move it to |kDesktopController|.
+      DCHECK_EQ(controller->widget(), desktop_widget_);
       // Release the old controller and close its background widget.
       root_window_->SetProperty(kDesktopController, controller);
     }
diff --git a/ash/desktop_background/desktop_background_widget_controller.cc b/ash/desktop_background/desktop_background_widget_controller.cc
index dd247ee..f91ee3a 100644
--- a/ash/desktop_background/desktop_background_widget_controller.cc
+++ b/ash/desktop_background/desktop_background_widget_controller.cc
@@ -80,6 +80,14 @@ AnimatingDesktopController::AnimatingDesktopController(
 AnimatingDesktopController::~AnimatingDesktopController() {
 }
 
+void AnimatingDesktopController::StopAnimating() {
+  if (controller_) {
+    ui::Layer* layer = controller_->layer() ? controller_->layer() :
+        controller_->widget()->GetNativeView()->layer();
+    layer->GetAnimator()->StopAnimating();
+  }
+}
+
 DesktopBackgroundWidgetController* AnimatingDesktopController::GetController(
     bool pass_ownership) {
   if (pass_ownership)
diff --git a/ash/desktop_background/desktop_background_widget_controller.h b/ash/desktop_background/desktop_background_widget_controller.h
index 31a48bd..272b3ce 100644
--- a/ash/desktop_background/desktop_background_widget_controller.h
+++ b/ash/desktop_background/desktop_background_widget_controller.h
@@ -65,6 +65,21 @@ class ASH_EXPORT AnimatingDesktopController {
       DesktopBackgroundWidgetController* component);
   ~AnimatingDesktopController();
 
+  // Stops animation and makes sure OnImplicitAnimationsCompleted() is called if
+  // current animation is not finished yet.
+  // Note we have to make sure this function is called before we set
+  // kAnimatingDesktopController to a new controller. If it is not called, the
+  // animating widget/layer is closed immediately and the new one is animating
+  // from the widget/layer before animation. For instance, if a user quickly
+  // switches between red, green and blue wallpapers. The green wallpaper will
+  // first fade in from red wallpaper. And in the middle of the animation, blue
+  // wallpaper also wants to fade in. If the green wallpaper animation does not
+  // finish immediately, the green wallpaper widget will be removed and the red
+  // widget will show again. As a result, the blue wallpaper fades in from red
+  // wallpaper. This is a bad user experience. See bug http://crbug.com/156542
+  // for more details.
+  void StopAnimating();
+
   // Gets the wrapped DesktopBackgroundWidgetController pointer. Caller should
   // take ownership of the pointer if |pass_ownership| is true.
   DesktopBackgroundWidgetController* GetController(bool pass_ownership);
diff --git a/ash/wm/system_gesture_event_filter.cc b/ash/wm/system_gesture_event_filter.cc
index 27e1aa4..bf8279b 100644
--- a/ash/wm/system_gesture_event_filter.cc
+++ b/ash/wm/system_gesture_event_filter.cc
@@ -63,7 +63,8 @@ bool SystemGestureEventFilter::PreHandleMouseEvent(aura::Window* target,
                                                    ui::MouseEvent* event) {
 #if defined(OS_CHROMEOS)
   if (event->type() == ui::ET_MOUSE_PRESSED && event->native_event() &&
-      ui::TouchFactory::GetInstance()->IsTouchDevicePresent()) {
+      ui::TouchFactory::GetInstance()->IsTouchDevicePresent() &&
+      Shell::GetInstance()->delegate()) {
     Shell::GetInstance()->delegate()->RecordUserMetricsAction(
       UMA_MOUSE_DOWN);
   }
diff --git a/ash/wm/window_animations.cc b/ash/wm/window_animations.cc
index 133f095..282567c 100644
--- a/ash/wm/window_animations.cc
+++ b/ash/wm/window_animations.cc
@@ -238,6 +238,7 @@ void AnimateShowWindowCommon(aura::Window* window,
   window->layer()->set_delegate(window);
   window->layer()->SetOpacity(kWindowAnimation_HideOpacity);
   window->layer()->SetTransform(start_transform);
+  window->layer()->SetVisible(true);
 
   {
     // Property sets within this scope will be implicitly animated.
@@ -246,7 +247,6 @@ void AnimateShowWindowCommon(aura::Window* window,
     if (duration.ToInternalValue() > 0)
       settings.SetTransitionDuration(duration);
 
-    window->layer()->SetVisible(true);
     window->layer()->SetTransform(end_transform);
     window->layer()->SetOpacity(kWindowAnimation_ShowOpacity);
   }