Wait for animated wallpaper changes to finish before suspending displays

PowerEventObserver delays display suspend until lock screen UI is shown
and compositors submit enough frames to know the UI changes have reached
displays.
This does not account for wallpaper changes due to screen lock:
  * On screen lock, the active wallpaper will be changed with its blurred
    version.
  * If screen lock changes the active user (e.g. if the screen is locked
    from the secondary user), the wallpaper will change to the new
    active user's blurred wallpaper.
Given that PowerEventObserver does not ensure wallpaper switch is done
before it stops drawing to the display, the first frame shown on the
device resume might still contain the UI from before wallpaper change.

This changes PowerEventObserver to additionally wait for any active
wallpaper changes (which can be detected by checking
WallpaperWidgetController::IsAnimating - set while wallpaper widget is
being changed) to finish before it starts waiting for compositor frames
to get composited, thus ensuring the correct wallpaper is set when the
display/compositing is suspended.

Adds the following methods to WallpaperWidgetController:
  * AddPendingAnimationEndCallback - to provide a way for
    PowerEventObserver to get notified when the pending wallpaper change
    completes
  * EndPendingAnimation which can be used to force immediate wallpaper
    change when device starts suspending (and prevents suspend flow from
    being blocked on unnecessarily waiting for new wallpaper animation)

BUG=820436,803762

Change-Id: Iad2074280ebba740b95c50b9d60a028bf5a0d45c
Reviewed-on: https://chromium-review.googlesource.com/956757Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Wenzhao (Colin) Zang <wzang@chromium.org>
Commit-Queue: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542945}

ash/system/power/power_event_observer.cc

@@ -8,9 +8,11 @@
 #include <utility>
 
 #include "ash/public/cpp/config.h"
+#include "ash/root_window_controller.h"
 #include "ash/session/session_controller.h"
 #include "ash/shell.h"
 #include "ash/system/tray/system_tray_notifier.h"
+#include "ash/wallpaper/wallpaper_widget_controller.h"
 #include "ash/wm/lock_state_controller.h"
 #include "ash/wm/lock_state_observer.h"
 #include "base/bind.h"
@@ -50,6 +52,10 @@ bool ShouldLockOnSuspend() {
 // will not start drawing the next frame before the previous swap happens, when
 // the second compositing cycle ends, it should be safe to assume the required
 // buffer swap happened at that point.
+// Note that the compositor watcher will wait for any pending wallpaper
+// animation for a root window to finish before it starts observing compositor
+// cycles, to ensure it picks up wallpaper state from after the animation ends,
+// and avoids issues like https://crbug.com/820436.
 class CompositorWatcher : public ui::CompositorObserver {
  public:
   // |callback| - called when all visible root window compositors complete
@@ -117,6 +123,7 @@ class CompositorWatcher : public ui::CompositorObserver {
   // This enum is used to track this cycle. Compositing goes through the
   // following states: DidCommit -> CompositingStarted -> CompositingEnded.
   enum class CompositingState {
+    kWaitingForWallpaperAnimation,
     kWaitingForCommit,
     kWaitingForStarted,
     kWaitingForEnded,
@@ -142,12 +149,21 @@ class CompositorWatcher : public ui::CompositorObserver {
         continue;
 
       DCHECK(!pending_compositing_.count(compositor));
-      pending_compositing_[compositor].state =
-          CompositingState::kWaitingForCommit;
-      compositor_observer_.Add(compositor);
 
-      // Schedule a draw to force at least one more compositing cycle.
-      compositor->ScheduleDraw();
+      compositor_observer_.Add(compositor);
+      pending_compositing_[compositor].state =
+          CompositingState::kWaitingForWallpaperAnimation;
+
+      WallpaperWidgetController* wallpaper_widget_controller =
+          RootWindowController::ForWindow(window)
+              ->wallpaper_widget_controller();
+      if (wallpaper_widget_controller->IsAnimating()) {
+        wallpaper_widget_controller->AddPendingAnimationEndCallback(
+            base::BindOnce(&CompositorWatcher::StartObservingCompositing,
+                           weak_ptr_factory_.GetWeakPtr(), compositor));
+      } else {
+        StartObservingCompositing(compositor);
+      }
     }
 
     // Post task to make sure callback is not invoked synchronously as watcher
@@ -158,6 +174,22 @@ class CompositorWatcher : public ui::CompositorObserver {
                        weak_ptr_factory_.GetWeakPtr()));
   }
 
+  // Called when the wallpaper animations end for the root window associated
+  // with the compositor. It starts observing the compositor's compositing
+  // cycles.
+  void StartObservingCompositing(ui::Compositor* compositor) {
+    if (!pending_compositing_.count(compositor) ||
+        pending_compositing_[compositor].state !=
+            CompositingState::kWaitingForWallpaperAnimation) {
+      return;
+    }
+
+    pending_compositing_[compositor].state =
+        CompositingState::kWaitingForCommit;
+    // Schedule a draw to force at least one more compositing cycle.
+    compositor->ScheduleDraw();
+  }
+
   // If all observed root window compositors have gone through a compositing
   // cycle, runs |callback_|.
   void RunCallbackIfAllCompositingEnded() {
@@ -204,6 +236,11 @@ void PowerEventObserver::OnLockAnimationsComplete() {
 
   lock_state_ = LockState::kLockedCompositingPending;
 
+  // If suspending, run pending animations to the end immediately, as there is
+  // no point in waiting for them to finish given that the device is suspending.
+  if (displays_suspended_callback_)
+    EndPendingWallpaperAnimations();
+
   // The |compositor_watcher_| is owned by this, and the callback passed to it
   // won't be called  after |compositor_watcher_|'s destruction, so
   // base::Unretained is safe here.
@@ -233,6 +270,11 @@ void PowerEventObserver::SuspendImminent(
       VLOG(1) << "Requesting screen lock from PowerEventObserver";
       lock_state_ = LockState::kLocking;
       Shell::Get()->lock_state_controller()->LockWithoutAnimation();
+    } else if (lock_state_ != LockState::kLocking) {
+      // If the screen is still being locked (i.e. in kLocking state),
+      // EndPendingWallpaperAnimations() will be called in
+      // OnLockAnimationsComplete().
+      EndPendingWallpaperAnimations();
     }
   }
 }
@@ -316,6 +358,15 @@ void PowerEventObserver::StopCompositingAndSuspendDisplays() {
   }
 }
 
+void PowerEventObserver::EndPendingWallpaperAnimations() {
+  for (aura::Window* window : Shell::GetAllRootWindows()) {
+    WallpaperWidgetController* wallpaper_widget_controller =
+        RootWindowController::ForWindow(window)->wallpaper_widget_controller();
+    if (wallpaper_widget_controller->IsAnimating())
+      wallpaper_widget_controller->EndPendingAnimation();
+  }
+}
+
 void PowerEventObserver::OnCompositorsReadyForSuspend() {
   compositor_watcher_.reset();
   lock_state_ = LockState::kLocked;

ash/system/power/power_event_observer.h

@@ -28,7 +28,8 @@ namespace ash {
 // screen is being locked during suspend - display compositing will not be
 // stopped before:
 //  1. lock screen window is shown
-//  2. the compositor goes through at least two compositing cycles after the
+//  2. wallpaper changes due to screen lock are finished
+//  3. the compositor goes through at least two compositing cycles after the
 //     screen lock
 // This is done to ensure that displays have picked up frames from after the
 // screen was locked. Without this, displays might initially show
@@ -84,6 +85,12 @@ class ASH_EXPORT PowerEventObserver
   // have gone through compositing cycle after the screen was locked.
   void StopCompositingAndSuspendDisplays();
 
+  // If any of the root windows have pending wallpaper animations, it stops
+  // them - this is used to stop wallpaper animations during suspend, and thus
+  // improve the suspend time (given that suspend will be delayed until the
+  // wallpaper animations finish).
+  void EndPendingWallpaperAnimations();
+
   // Callback run by |compositor_watcher_| when it detects that composting
   // can be stopped for all root windows when device suspends.
   void OnCompositorsReadyForSuspend();

ash/system/power/power_event_observer_unittest.cc

@@ -6,10 +6,12 @@
 
 #include <memory>
 
+#include "ash/root_window_controller.h"
 #include "ash/session/session_controller.h"
 #include "ash/shell.h"
 #include "ash/system/power/power_event_observer_test_api.h"
 #include "ash/test/ash_test_base.h"
+#include "ash/wallpaper/wallpaper_widget_controller.h"
 #include "ash/wm/lock_state_controller.h"
 #include "ash/wm/lock_state_controller_test_api.h"
 #include "ash/wm/test_session_state_animator.h"
@@ -20,6 +22,7 @@
 #include "ui/aura/window.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/compositor/compositor.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
 
 namespace ash {
 
@@ -381,4 +384,223 @@ TEST_F(PowerEventObserverTest, ImmediateLockAnimations) {
   EXPECT_FALSE(lock_state_test_api.is_animating_lock());
 }
 
+// Tests that displays will not be considered ready to suspend until the
+// animated wallpaper change finishes (if the wallpaper is being animated to
+// another wallpaper after the screen is locked).
+TEST_F(PowerEventObserverTest,
+       DisplaysNotReadyForSuspendUntilWallpaperAnimationEnds) {
+  chromeos::PowerManagerClient* client =
+      chromeos::DBusThreadManager::Get()->GetPowerManagerClient();
+  ASSERT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+
+  SetCanLockScreen(true);
+  SetShouldLockScreenAutomatically(true);
+
+  // Set up animation state so wallpaper widget animations are not ended on
+  // their creation.
+  ui::ScopedAnimationDurationScaleMode test_duration_mode(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+
+  // Lock screen - this is expected to start wallpaper change (e.g. to a
+  // widget with a blurred wallpaper).
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
+  observer_->OnLockAnimationsComplete();
+
+  WallpaperWidgetController* wallpaper_widget_controller =
+      Shell::GetPrimaryRootWindowController()->wallpaper_widget_controller();
+  // Assert that the wallpaper is being animated here - otherwise the test will
+  // not work.
+  ASSERT_TRUE(wallpaper_widget_controller->IsAnimating());
+
+  ui::Compositor* compositor =
+      Shell::GetPrimaryRootWindow()->GetHost()->compositor();
+  PowerEventObserverTestApi test_api(observer_.get());
+
+  // Simulate a single frame getting composited before the wallpaper animation
+  // is done - this frame is expected to be ignored by power event observer's
+  // compositing state observer.
+  test_api.CompositeFrame(compositor);
+
+  // Simulate wallpaper animation finishing - for the purpose of this test,
+  // before suspend begins.
+  wallpaper_widget_controller->EndPendingAnimation();
+
+  // Expect that two compositing cycles are completed before suspend continues,
+  // and displays get suspended.
+  test_api.CompositeFrame(compositor);
+  observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
+  EXPECT_EQ(1, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(1, GetNumVisibleCompositors());
+
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(0, GetNumVisibleCompositors());
+}
+
+// Tests that animated wallpaper changes will be finished immediately when
+// suspend starts (if the screen was locked when suspend started).
+TEST_F(PowerEventObserverTest, EndWallpaperAnimationOnSuspendWhileLocked) {
+  chromeos::PowerManagerClient* client =
+      chromeos::DBusThreadManager::Get()->GetPowerManagerClient();
+  ASSERT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+
+  SetCanLockScreen(true);
+  SetShouldLockScreenAutomatically(true);
+
+  // Set up animation state so wallpaper widget animations are not ended on
+  // their creation.
+  ui::ScopedAnimationDurationScaleMode test_duration_mode(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+
+  // Lock screen - this is expected to start wallpaper change (e.g. to a
+  // widget with a blurred wallpaper).
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
+  observer_->OnLockAnimationsComplete();
+
+  // Wallpaper animation should be stopped immediately on suspend.
+  observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
+
+  WallpaperWidgetController* wallpaper_widget_controller =
+      Shell::GetPrimaryRootWindowController()->wallpaper_widget_controller();
+  EXPECT_FALSE(wallpaper_widget_controller->IsAnimating());
+
+  ui::Compositor* compositor =
+      Shell::GetPrimaryRootWindow()->GetHost()->compositor();
+  PowerEventObserverTestApi test_api(observer_.get());
+
+  // Expect that two compositing cycles are completed before suspend continues,
+  // and displays get suspended.
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(1, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(1, GetNumVisibleCompositors());
+
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(0, GetNumVisibleCompositors());
+}
+
+// Tests that animated wallpaper changes will be finished immediately when
+// suspend starts (if the screen lock started before suspend).
+TEST_F(PowerEventObserverTest, EndWallpaperAnimationOnSuspendWhileLocking) {
+  chromeos::PowerManagerClient* client =
+      chromeos::DBusThreadManager::Get()->GetPowerManagerClient();
+  ASSERT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+
+  SetCanLockScreen(true);
+  SetShouldLockScreenAutomatically(true);
+
+  // Set up animation state so wallpaper widget animations are not ended on
+  // their creation.
+  ui::ScopedAnimationDurationScaleMode test_duration_mode(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+
+  // Lock screen - this is expected to start wallpaper change (e.g. to a
+  // widget with a blurred wallpaper).
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
+
+  // If suspend starts, wallpaper animation should be stopped after screen lock
+  // completes.
+  observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
+  observer_->OnLockAnimationsComplete();
+
+  WallpaperWidgetController* wallpaper_widget_controller =
+      Shell::GetPrimaryRootWindowController()->wallpaper_widget_controller();
+  EXPECT_FALSE(wallpaper_widget_controller->IsAnimating());
+
+  ui::Compositor* compositor =
+      Shell::GetPrimaryRootWindow()->GetHost()->compositor();
+  PowerEventObserverTestApi test_api(observer_.get());
+
+  // Expect that two compositing cycles are completed before suspend continues,
+  // and displays get suspended.
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(1, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(1, GetNumVisibleCompositors());
+
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(0, GetNumVisibleCompositors());
+}
+
+// Tests that animated wallpaper changes will be finished immediately when
+// suspend starts and causes a screen lock.
+TEST_F(PowerEventObserverTest, EndWallpaperAnimationAfterLockDueToSuspend) {
+  chromeos::PowerManagerClient* client =
+      chromeos::DBusThreadManager::Get()->GetPowerManagerClient();
+  ASSERT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+
+  SetCanLockScreen(true);
+  SetShouldLockScreenAutomatically(true);
+
+  // Set up animation state so wallpaper widget animations are not ended on
+  // their creation.
+  ui::ScopedAnimationDurationScaleMode test_duration_mode(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+
+  // Start suspend (which should start screen lock) - verify that wallpaper is
+  // not animating after the screen lock animations are reported as complete.
+  observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
+  observer_->OnLockAnimationsComplete();
+
+  WallpaperWidgetController* wallpaper_widget_controller =
+      Shell::GetPrimaryRootWindowController()->wallpaper_widget_controller();
+  EXPECT_FALSE(wallpaper_widget_controller->IsAnimating());
+
+  ui::Compositor* compositor =
+      Shell::GetPrimaryRootWindow()->GetHost()->compositor();
+  PowerEventObserverTestApi test_api(observer_.get());
+
+  // Expect that two compositing cycles are completed before suspend continues,
+  // and displays get suspended.
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(1, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(1, GetNumVisibleCompositors());
+
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(0, GetNumVisibleCompositors());
+}
+
+// Tests that removing a display while power event observer is waiting for the
+// wallpaper animation does not cause suspend to hang.
+TEST_F(PowerEventObserverTest, DisplayRemovedDuringWallpaperAnimation) {
+  chromeos::PowerManagerClient* client =
+      chromeos::DBusThreadManager::Get()->GetPowerManagerClient();
+  ASSERT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+
+  SetCanLockScreen(true);
+  SetShouldLockScreenAutomatically(true);
+
+  UpdateDisplay("100x100,200x200");
+
+  // Set up animation state so wallpaper widget animations are not ended on
+  // their creation.
+  ui::ScopedAnimationDurationScaleMode test_duration_mode(
+      ui::ScopedAnimationDurationScaleMode::NON_ZERO_DURATION);
+
+  // Lock screen - this is expected to start wallpaper change (e.g. to a
+  // widget with a blurred wallpaper).
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
+  observer_->OnLockAnimationsComplete();
+
+  // Remove a display before wallpaper animation ends.
+  UpdateDisplay("100x100");
+  base::RunLoop().RunUntilIdle();
+
+  // Start suspend and verify the suspend proceeds when the primary window's
+  // compositors go throug two compositing cycles.
+  observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
+
+  ui::Compositor* compositor =
+      Shell::GetPrimaryRootWindow()->GetHost()->compositor();
+  PowerEventObserverTestApi test_api(observer_.get());
+
+  // Expect that two compositing cycles are completed before suspend continues,
+  // and displays get suspended.
+  test_api.CompositeFrame(compositor);
+  test_api.CompositeFrame(compositor);
+  EXPECT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
+  EXPECT_EQ(0, GetNumVisibleCompositors());
+}
+
 }  // namespace ash

ash/wallpaper/wallpaper_widget_controller.cc

@@ -178,6 +178,18 @@ bool WallpaperWidgetController::IsAnimating() const {
   return animating_widget_.get();
 }
 
+void WallpaperWidgetController::EndPendingAnimation() {
+  if (!IsAnimating())
+    return;
+  animating_widget_->StopAnimating();
+}
+
+void WallpaperWidgetController::AddPendingAnimationEndCallback(
+    base::OnceClosure callback) {
+  DCHECK(IsAnimating());
+  pending_animation_end_callbacks_.emplace_back(std::move(callback));
+}
+
 void WallpaperWidgetController::SetWallpaperWidget(views::Widget* widget,
                                                    float blur_sigma) {
   DCHECK(widget);
@@ -227,8 +239,10 @@ void WallpaperWidgetController::WidgetHandlerReset(WidgetHandler* widget) {
 }
 
 void WallpaperWidgetController::WidgetFinishedAnimating(WidgetHandler* widget) {
-  if (widget == animating_widget_.get())
-    SetAnimatingWidgetAsActive();
+  if (widget != animating_widget_.get())
+    return;
+
+  SetAnimatingWidgetAsActive();
 }
 
 void WallpaperWidgetController::SetAnimatingWidgetAsActive() {
@@ -241,7 +255,15 @@ void WallpaperWidgetController::SetAnimatingWidgetAsActive() {
     std::move(wallpaper_set_callback_).Run();
 
   // Notify observers that animation finished.
+  RunPendingAnimationEndCallbacks();
   Shell::Get()->wallpaper_controller()->OnWallpaperAnimationFinished();
 }
 
+void WallpaperWidgetController::RunPendingAnimationEndCallbacks() {
+  std::list<base::OnceClosure> callbacks;
+  pending_animation_end_callbacks_.swap(callbacks);
+  for (auto& callback : callbacks)
+    std::move(callback).Run();
+}
+
 }  // namespace ash

ash/wallpaper/wallpaper_widget_controller.h

@@ -5,6 +5,7 @@
 #ifndef ASH_WALLPAPER_WALLPAPER_WIDGET_CONTROLLER_H_
 #define ASH_WALLPAPER_WALLPAPER_WIDGET_CONTROLLER_H_
 
+#include <list>
 #include <memory>
 
 #include "ash/ash_export.h"
@@ -36,6 +37,17 @@ class ASH_EXPORT WallpaperWidgetController {
   // |animating_widget_| exists.
   bool IsAnimating() const;
 
+  // If an animating wallpaper change is in progress, it ends the animation and
+  // changes the wallpaper immediately.
+  // No-op if IsAnimation() returns false.
+  void EndPendingAnimation();
+
+  // If an animating wallpaper change is in progress, it adds a callback that
+  // will be run when the pending animation ends.
+  // The callback will not be run if a wallpaper animation is not in progress -
+  // in that case the method will return false.
+  void AddPendingAnimationEndCallback(base::OnceClosure callback);
+
   // Sets a new wallpaper widget - this will not change the primary widget
   // immediately. The primary widget will be switched when |widget|'s showing
   // animation finishes (during which |widget| will be kept by
@@ -71,6 +83,9 @@ class ASH_EXPORT WallpaperWidgetController {
   // Moves |animated_widget_| to |active_widget_|.
   void SetAnimatingWidgetAsActive();
 
+  // Runs callbacks in |pending_animation_end_callbacks_|.
+  void RunPendingAnimationEndCallbacks();
+
   // Callback that will be run when |active_widget_| is first set.
   base::OnceClosure wallpaper_set_callback_;
 
@@ -81,6 +96,10 @@ class ASH_EXPORT WallpaperWidgetController {
   // shown.
   std::unique_ptr<WidgetHandler> animating_widget_;
 
+  // Callbacks to be run when the |animating_widget_| stops animating and gets
+  // set as the active widget.
+  std::list<base::OnceClosure> pending_animation_end_callbacks_;
+
   DISALLOW_COPY_AND_ASSIGN(WallpaperWidgetController);
 };