Shelf dimming animation should be canceled for auto hidden shelf.

The hotseat widget was refactored to calculate its own opacity. As
a result, we are seeing unintended behavior where SetDimmed() is setting
the shelf opacity to 0 because the hotseat widget's CalculateOpacity()
returns 0 in the auto hidden state.

We are fixing this by:
1. ignoring calls to SetDimmed when the shelf is in the hidden state. It
doesn't make any sense to modify animate opacity of something hidden.

2. setting up an auto-hide lock while setting up opacity animations.
This prevents the auto hide state from changing during animation setup.


3. Activating/undimming the shelf when it is no longer auto hidden.

Bug: 1081835
Change-Id: I87ccf82947d1ca98a154c718f63413ae60a03fb3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207976
Commit-Queue: Yulun Wu <yulunwu@chromium.org>
Reviewed-by: Yulun Wu <yulunwu@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770888}

ash/shelf/hotseat_widget.cc

@@ -419,11 +419,11 @@ void HotseatWidget::OnTabletModeChanged() {
   GetShelfView()->OnTabletModeChanged();
 }
 
-float HotseatWidget::CalculateOpacity() const {
+float HotseatWidget::CalculateShelfViewOpacity() const {
   const float target_opacity =
       GetShelfView()->shelf()->shelf_layout_manager()->GetOpacity();
-  return (state() == HotseatState::kExtended) ? 1.0f  // fully translucent
-                                              : target_opacity;
+  // Hotseat's shelf view should not be dimmed if hotseat is kExtended.
+  return (state() == HotseatState::kExtended) ? 1.0f : target_opacity;
 }
 
 void HotseatWidget::SetTranslucentBackground(
@@ -532,9 +532,10 @@ void HotseatWidget::UpdateLayout(bool animate) {
   if (!new_layout_inputs.is_active_session_state)
     Hide();
 
-  ui::Layer* layer = GetNativeView()->layer();
+  ui::Layer* shelf_view_layer = GetShelfView()->layer();
   {
-    ui::ScopedLayerAnimationSettings animation_setter(layer->GetAnimator());
+    ui::ScopedLayerAnimationSettings animation_setter(
+        shelf_view_layer->GetAnimator());
     animation_setter.SetTransitionDuration(
         animate ? ShelfConfig::Get()->shelf_animation_duration()
                 : base::TimeDelta::FromMilliseconds(0));
@@ -544,7 +545,26 @@ void HotseatWidget::UpdateLayout(bool animate) {
     animation_setter.SetAnimationMetricsReporter(
         shelf_->GetHotseatTransitionMetricsReporter(state_));
 
-    layer->SetOpacity(new_layout_inputs.opacity);
+    shelf_view_layer->SetOpacity(new_layout_inputs.shelf_view_opacity);
+  }
+
+  ui::Layer* hotseat_layer = GetNativeView()->layer();
+  {
+    ui::ScopedLayerAnimationSettings animation_setter(
+        hotseat_layer->GetAnimator());
+    animation_setter.SetTransitionDuration(
+        animate ? ShelfConfig::Get()->shelf_animation_duration()
+                : base::TimeDelta::FromMilliseconds(0));
+    animation_setter.SetTweenType(gfx::Tween::EASE_OUT);
+    animation_setter.SetPreemptionStrategy(
+        ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
+    animation_setter.SetAnimationMetricsReporter(
+        shelf_->GetHotseatTransitionMetricsReporter(state_));
+
+    // If shelf view is invisible, the hotseat should be as well. Otherwise the
+    // hotseat opacit should be 1.0f to preserve background blur.
+    hotseat_layer->SetOpacity(
+        new_layout_inputs.shelf_view_opacity == 0.0f ? 0.0f : 1.0f);
     SetBounds(new_layout_inputs.bounds);
     layout_inputs_ = new_layout_inputs;
     delegate_view_->UpdateTranslucentBackground();
@@ -552,7 +572,8 @@ void HotseatWidget::UpdateLayout(bool animate) {
 
   // Setting visibility during an animation causes the visibility property to
   // animate. Set the visibility property without an animation.
-  if (new_layout_inputs.opacity && new_layout_inputs.is_active_session_state) {
+  if (new_layout_inputs.shelf_view_opacity != 0.0f &&
+      new_layout_inputs.is_active_session_state) {
     ShowInactive();
   }
 }
@@ -640,7 +661,7 @@ void HotseatWidget::SetState(HotseatState state) {
 
 HotseatWidget::LayoutInputs HotseatWidget::GetLayoutInputs() const {
   const ShelfLayoutManager* layout_manager = shelf_->shelf_layout_manager();
-  return {target_bounds_, CalculateOpacity(),
+  return {target_bounds_, CalculateShelfViewOpacity(),
           layout_manager->is_active_session_state()};
 }
 

ash/shelf/hotseat_widget.h

@@ -60,8 +60,8 @@ class ASH_EXPORT HotseatWidget : public ShelfComponent,
   // Notifies children of tablet mode state changes.
   void OnTabletModeChanged();
 
-  // Returns the target opacity (between 0 and 1) given current conditions.
-  float CalculateOpacity() const;
+  // Returns the target opacity for the shelf view given current conditions.
+  float CalculateShelfViewOpacity() const;
 
   // Sets the bounds of the translucent background which functions as the
   // hotseat background.
@@ -138,11 +138,12 @@ class ASH_EXPORT HotseatWidget : public ShelfComponent,
 
   struct LayoutInputs {
     gfx::Rect bounds;
-    float opacity = 0.0f;
+    float shelf_view_opacity = 0.0f;
     bool is_active_session_state = false;
 
     bool operator==(const LayoutInputs& other) const {
-      return bounds == other.bounds && opacity == other.opacity &&
+      return bounds == other.bounds &&
+             shelf_view_opacity == other.shelf_view_opacity &&
              is_active_session_state == other.is_active_session_state;
     }
   };

ash/shelf/shelf.cc

@@ -270,21 +270,31 @@ class Shelf::AutoDimEventHandler : public ui::EventHandler {
     }
   }
 
-  void DimShelf() { shelf_->shelf_layout_manager()->SetDimmed(true); }
+  void StartDimShelfTimer() {
+    dim_shelf_timer_.Start(
+        FROM_HERE, kDimDelay,
+        base::BindOnce(&AutoDimEventHandler::DimShelf, base::Unretained(this)));
+  }
+
+  void DimShelf() {
+    // Attempt to dim the shelf. Stop the |dim_shelf_timer_| if successful.
+    if (shelf_->shelf_layout_manager()->SetDimmed(true))
+      dim_shelf_timer_.Stop();
+  }
 
   // Sets shelf as active and sets timer to mark shelf as inactive.
   void UndimShelf() {
     shelf_->shelf_layout_manager()->SetDimmed(false);
-    update_shelf_dim_state_timer_.Start(
-        FROM_HERE, kDimDelay,
-        base::BindOnce(&AutoDimEventHandler::DimShelf, base::Unretained(this)));
+    StartDimShelfTimer();
   }
 
+  bool HasDimShelfTimer() { return dim_shelf_timer_.IsRunning(); }
+
  private:
   // Unowned pointer to the shelf that owns this event handler.
   Shelf* shelf_;
   // OneShotTimer that dims shelf due to inactivity.
-  base::OneShotTimer update_shelf_dim_state_timer_;
+  base::OneShotTimer dim_shelf_timer_;
 
   // Delay before dimming the shelf.
   const base::TimeDelta kDimDelay = base::TimeDelta::FromSeconds(5);
@@ -702,6 +712,10 @@ void Shelf::UndimShelf() {
   auto_dim_event_handler_->UndimShelf();
 }
 
+bool Shelf::HasDimShelfTimer() {
+  return auto_dim_event_handler_->HasDimShelfTimer();
+}
+
 WorkAreaInsets* Shelf::GetWorkAreaInsets() const {
   const aura::Window* window = GetWindow();
   DCHECK(window);

ash/shelf/shelf.h

@@ -259,6 +259,7 @@ class ASH_EXPORT Shelf : public ShelfLayoutManagerObserver {
   // Uses Auto Dim Event Handler to update the shelf dim state.
   void DimShelf();
   void UndimShelf();
+  bool HasDimShelfTimer();
 
   // Returns work area insets object for the window with this shelf.
   WorkAreaInsets* GetWorkAreaInsets() const;

ash/shelf/shelf_layout_manager.cc

@@ -1310,6 +1310,10 @@ void ShelfLayoutManager::SetState(ShelfVisibilityState visibility_state) {
       observer.OnHotseatStateChanged(previous_hotseat_state, hotseat_state());
   }
 
+  // Shelf should be undimmed when it transitions into a visible state.
+  if (state_.IsShelfVisible())
+    SetDimmed(false);
+
   UpdateContextualNudges();
 }
 
@@ -1485,9 +1489,19 @@ ShelfVisibilityState ShelfLayoutManager::CalculateShelfVisibility() {
   return SHELF_VISIBLE;
 }
 
-void ShelfLayoutManager::SetDimmed(bool dimmed) {
+bool ShelfLayoutManager::SetDimmed(bool dimmed) {
+  // Do nothing if we are already in the correct dim state.
   if (dimmed_for_inactivity_ == dimmed)
-    return;
+    return false;
+
+  // We do not want the auto hide state to change while setting up animations.
+  std::unique_ptr<Shelf::ScopedAutoHideLock> auto_hide_lock =
+      std::make_unique<Shelf::ScopedAutoHideLock>(shelf_);
+
+  // We should not set the dim state if the shelf is hidden. Shelf will be
+  // undimmed when it transitions into a visible state.
+  if (!state_.IsShelfVisible())
+    return false;
 
   dimmed_for_inactivity_ = dimmed;
   CalculateTargetBoundsAndUpdateWorkArea();
@@ -1502,7 +1516,7 @@ void ShelfLayoutManager::SetDimmed(bool dimmed) {
                  shelf_->GetNavigationWidgetAnimationMetricsReporter());
 
   AnimateOpacity(shelf_->hotseat_widget()->GetShelfView()->layer(),
-                 shelf_->hotseat_widget()->CalculateOpacity(),
+                 shelf_->hotseat_widget()->CalculateShelfViewOpacity(),
                  dim_animation_duration, dim_animation_tween,
                  /*animation_metrics_reporter=*/nullptr);
 
@@ -1511,6 +1525,7 @@ void ShelfLayoutManager::SetDimmed(bool dimmed) {
                  /*animation_metrics_reporter=*/nullptr);
 
   shelf_widget_->SetLoginShelfButtonOpacity(target_opacity_);
+  return true;
 }
 
 void ShelfLayoutManager::UpdateBoundsAndOpacity(bool animate) {

ash/shelf/shelf_layout_manager.h

@@ -150,8 +150,8 @@ class ASH_EXPORT ShelfLayoutManager
   void ProcessGestureEventOfInAppHotseat(ui::GestureEvent* event,
                                          aura::Window* target);
 
-  // Updates the auto-dim state.
-  void SetDimmed(bool dimmed);
+  // Updates the auto-dim state. Returns true if successful.
+  bool SetDimmed(bool dimmed);
 
   void AddObserver(ShelfLayoutManagerObserver* observer);
   void RemoveObserver(ShelfLayoutManagerObserver* observer);

ash/shelf/shelf_layout_manager_unittest.cc

@@ -3760,6 +3760,8 @@ class DimShelfLayoutManagerTestBase : public ShelfLayoutManagerTestBase {
 
   void ResetDimShelf() { GetPrimaryShelf()->UndimShelf(); }
 
+  bool HasDimShelfTimer() { return GetPrimaryShelf()->HasDimShelfTimer(); }
+
   float GetWidgetOpacity(views::Widget* widget) {
     return widget->GetNativeView()->layer()->opacity();
   }
@@ -3857,9 +3859,9 @@ TEST_P(DimShelfLayoutManagerTest, MaximizedShelfDimAlpha) {
   ASSERT_FALSE(ShelfDimmed());
 
   if (dim_shelf_enabled) {
-    TriggerDimShelf();
     views::Widget* widget = CreateTestWidget();
     widget->Maximize();
+    TriggerDimShelf();
   }
 
   EXPECT_EQ(GetWidgetOpacity(GetPrimaryShelf()->shelf_widget()),
@@ -3868,8 +3870,11 @@ TEST_P(DimShelfLayoutManagerTest, MaximizedShelfDimAlpha) {
             dim_shelf_enabled ? kExpectedMaximizedShelfDimOpacity
                               : kExpectedDefaultShelfOpacity);
   EXPECT_EQ(GetWidgetOpacity(GetPrimaryShelf()->hotseat_widget()),
-            dim_shelf_enabled ? kExpectedMaximizedShelfDimOpacity
-                              : kExpectedDefaultShelfOpacity);
+            kExpectedDefaultShelfOpacity);
+  EXPECT_EQ(
+      GetPrimaryShelf()->hotseat_widget()->GetShelfView()->layer()->opacity(),
+      dim_shelf_enabled ? kExpectedMaximizedShelfDimOpacity
+                        : kExpectedDefaultShelfOpacity);
   EXPECT_EQ(
       GetWidgetOpacity(GetPrimaryShelf()->shelf_widget()->status_area_widget()),
       dim_shelf_enabled ? kExpectedMaximizedShelfDimOpacity
@@ -3953,7 +3958,7 @@ TEST_P(HotseatDimShelfLayoutManagerTest, InAppShelfDimAlpha) {
 TEST_P(HotseatDimShelfLayoutManagerTest, TabletModeHomeShelfDimAlpha) {
   ASSERT_TRUE(AutoDimEventHandlerInitialized());
   TabletModeControllerTestApi().EnterTabletMode();
-  ASSERT_FALSE(ShelfDimmed());
+  EXPECT_FALSE(ShelfDimmed());
 
   const bool shelf_hotseat_enabled = GetParam();
   EXPECT_EQ(shelf_hotseat_enabled,
@@ -3983,4 +3988,55 @@ TEST_P(HotseatDimShelfLayoutManagerTest, TabletModeHomeShelfDimAlpha) {
       kExpectedFloatingShelfDimOpacity);
 }
 
+// Shelf dimming should not trigger when shelf is hidden in tablet mode.
+TEST_P(HotseatDimShelfLayoutManagerTest, AutoHiddenShelfTabletModeDimAlpha) {
+  ASSERT_TRUE(AutoDimEventHandlerInitialized());
+  TabletModeControllerTestApi().EnterTabletMode();
+
+  Shelf* shelf = GetPrimaryShelf();
+  shelf->SetAutoHideBehavior(ShelfAutoHideBehavior::kAlways);
+  EXPECT_FALSE(ShelfDimmed());
+  views::Widget* widget = CreateTestWidget();
+  widget->Maximize();
+
+  // Shelf should not be dimmed when auto hidden.
+  EXPECT_EQ(SHELF_AUTO_HIDE_HIDDEN, shelf->GetAutoHideState());
+  TriggerDimShelf();
+  EXPECT_FALSE(ShelfDimmed());
+
+  // Minimizing the widget should show the shelf. The shelf can now be dimmed.
+  widget->Minimize();
+  EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
+  EXPECT_FALSE(ShelfDimmed());
+  TriggerDimShelf();
+  EXPECT_TRUE(ShelfDimmed());
+}
+
+// Shelf dimming should not trigger when shelf is hidden in clamshell mode.
+TEST_P(HotseatDimShelfLayoutManagerTest, AutoHiddenShelfClamshellModeDimAlpha) {
+  ASSERT_TRUE(AutoDimEventHandlerInitialized());
+
+  Shelf* shelf = GetPrimaryShelf();
+  shelf->SetAutoHideBehavior(ShelfAutoHideBehavior::kAlways);
+  EXPECT_FALSE(ShelfDimmed());
+  views::Widget* widget = CreateTestWidget();
+  widget->Maximize();
+
+  // Shelf should not be dimmed when auto hidden. The dim shelf timer should
+  // persist after failing to dim the shelf.
+  EXPECT_EQ(SHELF_AUTO_HIDE_HIDDEN, shelf->GetAutoHideState());
+  TriggerDimShelf();
+  EXPECT_FALSE(ShelfDimmed());
+  EXPECT_TRUE(HasDimShelfTimer());
+
+  // Minimizing the widget should show the shelf. The shelf can now be dimmed
+  // and the dim shelf timer should no longer be active.
+  widget->Minimize();
+  EXPECT_EQ(SHELF_AUTO_HIDE_SHOWN, shelf->GetAutoHideState());
+  ASSERT_FALSE(ShelfDimmed());
+  TriggerDimShelf();
+  EXPECT_TRUE(ShelfDimmed());
+  EXPECT_FALSE(HasDimShelfTimer());
+}
+
 }  // namespace ash