Improve the backdrop behavior and code

- The backdrop used to show immediately even if the
  window-needing-it is still animating, making the animation
  look poor.
- There multiple recursive calls to UpdateBackdrop() constantly
  all the time, many of them are not necessary, and can be handled
  only by laying out the backdrop if needed.
- Even the browser's status bubble used to cause multiple called
  to UpdateBackdrop() as the user moves their mouse hovering over
  links, which causes the status bubble to be created, removed,
  and have visibility changes.
- The backdrop window itself causes calls to UpdateBackdrop as it
  gets shown and hidden.
- The backdrops weren't showing in the desks mini_views (a
  regression caused by me).
- WorkspaceLayoutManager and Desk were both updating the backdrops
  for the same event.

This CL fixes the above issues.

BUG=1031789
TEST=Manually, all backdrop tests should pass.

Change-Id: I5e25705ae50abdf1cf69b63b01f762044587f600
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1974593
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730046}

ash/shelf/shelf_widget.cc

@@ -781,14 +781,8 @@ void ShelfWidget::OnGestureEvent(ui::GestureEvent* event) {
 }
 
 void ShelfWidget::OnAccessibilityStatusChanged() {
-  // Only handles when the spoken feedback is disabled.
-  if (Shell::Get()->accessibility_controller()->spoken_feedback_enabled())
-    return;
-
-  if (!is_hotseat_forced_to_show_)
-    return;
-
-  is_hotseat_forced_to_show_ = false;
+  is_hotseat_forced_to_show_ =
+      Shell::Get()->accessibility_controller()->spoken_feedback_enabled();
   shelf_layout_manager_->UpdateVisibilityState();
 }
 

ash/wm/desks/desk.cc

@@ -183,8 +183,9 @@ void Desk::AddWindowToDesk(aura::Window* window) {
   windows_.push_back(window);
   // No need to refresh the mini_views if the destroyed window doesn't show up
   // there in the first place.
+  // The WorkspaceLayoutManager updates the backdrop for us.
   if (!window->GetProperty(kHideInDeskMiniViewKey))
-    NotifyContentChanged();
+    NotifyContentChanged(/*update_backdrops=*/false);
 }
 
 void Desk::RemoveWindowFromDesk(aura::Window* window) {
@@ -192,8 +193,9 @@ void Desk::RemoveWindowFromDesk(aura::Window* window) {
   base::Erase(windows_, window);
   // No need to refresh the mini_views if the destroyed window doesn't show up
   // there in the first place.
+  // The WorkspaceLayoutManager updates the backdrop for us.
   if (!window->GetProperty(kHideInDeskMiniViewKey))
-    NotifyContentChanged();
+    NotifyContentChanged(/*update_backdrops=*/false);
 }
 
 base::AutoReset<bool> Desk::GetScopedNotifyContentChangedDisabler() {
@@ -278,8 +280,8 @@ void Desk::MoveWindowsToDesk(Desk* target_desk) {
     }
   }
 
-  NotifyContentChanged();
-  target_desk->NotifyContentChanged();
+  NotifyContentChanged(/*update_backdrops=*/true);
+  target_desk->NotifyContentChanged(/*update_backdrops=*/true);
 }
 
 void Desk::MoveWindowToDesk(aura::Window* window, Desk* target_desk) {
@@ -314,8 +316,8 @@ void Desk::MoveWindowToDesk(aura::Window* window, Desk* target_desk) {
       window_state->Unminimize();
   }
 
-  NotifyContentChanged();
-  target_desk->NotifyContentChanged();
+  NotifyContentChanged(/*update_backdrops=*/true);
+  target_desk->NotifyContentChanged(/*update_backdrops=*/true);
 }
 
 aura::Window* Desk::GetDeskContainerForRoot(aura::Window* root) const {
@@ -324,13 +326,15 @@ aura::Window* Desk::GetDeskContainerForRoot(aura::Window* root) const {
   return root->GetChildById(container_id_);
 }
 
-void Desk::NotifyContentChanged() {
+void Desk::NotifyContentChanged(bool update_backdrops) {
   if (!should_notify_content_changed_)
     return;
 
-  // Update the backdrop availability and visibility first before notifying
-  // observers.
-  UpdateDeskBackdrops();
+  // If requested, update the backdrop availability and visibility first before
+  // notifying observers, so that the mini_views update *after* the backdrops
+  // do.
+  if (update_backdrops)
+    UpdateDeskBackdrops();
 
   for (auto& observer : observers_)
     observer.OnContentChanged();

ash/wm/desks/desk.h

@@ -88,10 +88,18 @@ class ASH_EXPORT Desk {
 
   aura::Window* GetDeskContainerForRoot(aura::Window* root) const;
 
-  void NotifyContentChanged();
-
-  // Updates the backdrop availability and visibility on the containers (on all
-  // roots) associated with this desk.
+  // Notifies observers that the desk's contents (list of application windows on
+  // the desk) have changed.
+  // If |update_backdrops| is true, the backdrops of all containers associated
+  // with this desk will be updated (even if overview is active).
+  // This is *only* needed if the WorkspaceLayoutManager won't take care of this
+  // for us in desk-modifying operations that happen within overview, such as
+  // removing desks (and move its windows out) or dragging a window and dropping
+  // in another desk.
+  void NotifyContentChanged(bool update_backdrops);
+
+  // Update (even if overview is active) the backdrop availability and
+  // visibility on the containers (on all roots) associated with this desk.
   void UpdateDeskBackdrops();
 
  private:

ash/wm/desks/desks_controller.cc

@@ -803,7 +803,7 @@ void DesksController::RemoveDeskInternal(const Desk* desk,
   // if windows from the removed desk moved to it.
   DCHECK(active_desk_->should_notify_content_changed());
   if (!removed_desk_windows.empty())
-    active_desk_->NotifyContentChanged();
+    active_desk_->NotifyContentChanged(/*force_update_backdrops=*/true);
 
   for (auto& observer : observers_)
     observer.OnDeskRemoved(removed_desk.get());

ash/wm/workspace/backdrop_controller.h

@@ -52,11 +52,12 @@ class ASH_EXPORT BackdropController : public AccessibilityObserver,
   explicit BackdropController(aura::Window* container);
   ~BackdropController() override;
 
-  void OnWindowAddedToLayout();
-  void OnWindowRemovedFromLayout();
-  void OnChildWindowVisibilityChanged();
-  void OnWindowStackingChanged();
-  void OnPostWindowStateTypeChange();
+  void OnWindowAddedToLayout(aura::Window* window);
+  void OnWindowRemovedFromLayout(aura::Window* window);
+  void OnChildWindowVisibilityChanged(aura::Window* window);
+  void OnBackdropWindowModePropertyChanged(aura::Window* window);
+  void OnWindowStackingChanged(aura::Window* window);
+  void OnPostWindowStateTypeChange(aura::Window* window);
   void OnDisplayMetricsChanged();
 
   // Called when the desk content is changed in order to update the state of the
@@ -96,6 +97,7 @@ class ASH_EXPORT BackdropController : public AccessibilityObserver,
   void OnTabletModeEnded() override;
 
  private:
+  class WindowAnimationWaiter;
   friend class WorkspaceControllerTestApi;
 
   // Reenables updates previously pause by calling PauseUpdates().
@@ -103,7 +105,7 @@ class ASH_EXPORT BackdropController : public AccessibilityObserver,
 
   void UpdateBackdropInternal();
 
-  void EnsureBackdropWidget(BackdropWindowMode mode);
+  void EnsureBackdropWidget();
 
   void UpdateAccessibilityMode();
 
@@ -111,7 +113,9 @@ class ASH_EXPORT BackdropController : public AccessibilityObserver,
 
   bool WindowShouldHaveBackdrop(aura::Window* window);
 
-  // Show the backdrop window.
+  // Show the backdrop window if the |window_having_backdrop_| is not animating,
+  // otherwise it will wait for that animation to finish. If it can show the
+  // backdrop, it will update its bounds and stacking order before its shown.
   void Show();
 
   // Hide the backdrop window. If |destroy| is true, the backdrop widget will be
@@ -130,8 +134,17 @@ class ASH_EXPORT BackdropController : public AccessibilityObserver,
   // backdrop bounds should be the bounds of the snapped window.
   gfx::Rect GetBackdropBounds();
 
-  // Sets the animtion type of |backdrop_window_| to |type|.
-  void SetBackdropAnimationType(int type);
+  // If |window_having_backdrop_| is animating such that we shouldn't update the
+  // backdrop until that animation is complete, starts observing this animation
+  // (if not already done) and returns true. Returns false otherwise.
+  bool MaybeWaitForWindowAnimation();
+
+  // Updates the layout of the backdrop if one exists and is visible.
+  void MaybeUpdateLayout();
+
+  // Returns true if changes to |window| may require updating the backdrop
+  // visibility and availability.
+  bool DoesWindowCauseBackdropUpdates(aura::Window* window) const;
 
   aura::Window* root_window_;
 
@@ -141,9 +154,16 @@ class ASH_EXPORT BackdropController : public AccessibilityObserver,
   // aura::Window for |backdrop_|.
   aura::Window* backdrop_window_ = nullptr;
 
+  // The window for which a backdrop has been installed.
+  aura::Window* window_having_backdrop_ = nullptr;
+
   // The container of the window that should have a backdrop.
   aura::Window* container_;
 
+  // If |window_having_backdrop_| is animating while we're trying to show the
+  // backdrop, we postpone showing it until the animation completes.
+  std::unique_ptr<WindowAnimationWaiter> window_animation_waiter_;
+
   // Event hanlder used to implement actions for accessibility.
   std::unique_ptr<ui::EventHandler> backdrop_event_handler_;
   ui::EventHandler* original_event_handler_ = nullptr;

ash/wm/workspace/workspace_layout_manager.cc

@@ -148,7 +148,7 @@ void WorkspaceLayoutManager::OnWindowAddedToLayout(aura::Window* child) {
   UpdateShelfVisibility();
   UpdateFullscreenState();
 
-  backdrop_controller_->OnWindowAddedToLayout();
+  backdrop_controller_->OnWindowAddedToLayout(child);
   WindowPositioner::RearrangeVisibleWindowOnShow(child);
   if (Shell::Get()->screen_pinning_controller()->IsPinned())
     WindowState::Get(child)->DisableZOrdering(nullptr);
@@ -178,7 +178,7 @@ void WorkspaceLayoutManager::OnWillRemoveWindowFromLayout(aura::Window* child) {
 void WorkspaceLayoutManager::OnWindowRemovedFromLayout(aura::Window* child) {
   UpdateShelfVisibility();
   UpdateFullscreenState();
-  backdrop_controller_->OnWindowRemovedFromLayout();
+  backdrop_controller_->OnWindowRemovedFromLayout(child);
 }
 
 void WorkspaceLayoutManager::OnChildWindowVisibilityChanged(aura::Window* child,
@@ -194,7 +194,7 @@ void WorkspaceLayoutManager::OnChildWindowVisibilityChanged(aura::Window* child,
     WindowPositioner::RearrangeVisibleWindowOnHideOrRemove(child);
   UpdateFullscreenState();
   UpdateShelfVisibility();
-  backdrop_controller_->OnChildWindowVisibilityChanged();
+  backdrop_controller_->OnChildWindowVisibilityChanged(child);
 }
 
 void WorkspaceLayoutManager::SetChildBounds(aura::Window* child,
@@ -320,14 +320,14 @@ void WorkspaceLayoutManager::OnWindowPropertyChanged(aura::Window* window,
         container->AddChild(window);
     }
   } else if (key == kBackdropWindowMode) {
-    backdrop_controller_->UpdateBackdrop();
+    backdrop_controller_->OnBackdropWindowModePropertyChanged(window);
   }
 }
 
 void WorkspaceLayoutManager::OnWindowStackingChanged(aura::Window* window) {
   UpdateShelfVisibility();
   UpdateFullscreenState();
-  backdrop_controller_->OnWindowStackingChanged();
+  backdrop_controller_->OnWindowStackingChanged(window);
 }
 
 void WorkspaceLayoutManager::OnWindowDestroying(aura::Window* window) {
@@ -386,7 +386,7 @@ void WorkspaceLayoutManager::OnPostWindowStateTypeChange(
   }
 
   UpdateShelfVisibility();
-  backdrop_controller_->OnPostWindowStateTypeChange();
+  backdrop_controller_->OnPostWindowStateTypeChange(window_state->window());
 }
 
 //////////////////////////////////////////////////////////////////////////////