Restart expand arrow animation when AppList is re-shown

Previously the animation was only initited from the ExpandArrowView
constructor which is now being called only once upon the AppView
construction.

Introduced a MaybeEnableHintingAnimation method to be called
every time the launcher state is changing.

Replaced PostTask with OneShotTimer to prevent multiple tasks
being scheduled in parallel. The timer is canceled whenever AppList
is closed.

Added a unit test.

Bug: 989181
Test: manual
Change-Id: I042ed4916497d2d70c900bdddcfce5a3b809342b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1775069
Commit-Queue: Vladislav Kaznacheev <kaznacheev@chromium.org>
Auto-Submit: Vladislav Kaznacheev <kaznacheev@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691683}

ash/app_list/views/app_list_view_unittest.cc

@@ -2021,6 +2021,27 @@ TEST_F(AppListViewTest, AppsGridViewVisibilityOnReopening) {
   EXPECT_TRUE(IsViewVisibleOnScreen(apps_grid_view()));
 }
 
+TEST_F(AppListViewTest, AppsGridViewExpandHintingOnReopening) {
+  AppListView::SetShortAnimationForTesting(false);
+  Initialize(false /*is_tablet_mode*/);
+
+  Show();
+  view_->SetState(ash::AppListViewState::kPeeking);
+  EXPECT_TRUE(
+      contents_view()->expand_arrow_view()->IsHintingAnimationRunningForTest());
+
+  view_->SetState(ash::AppListViewState::kClosed);
+  EXPECT_FALSE(
+      contents_view()->expand_arrow_view()->IsHintingAnimationRunningForTest());
+
+  Show();
+  view_->SetState(ash::AppListViewState::kPeeking);
+  EXPECT_TRUE(
+      contents_view()->expand_arrow_view()->IsHintingAnimationRunningForTest());
+
+  AppListView::SetShortAnimationForTesting(true);
+}
+
 // Tests that going into a folder view, then setting the AppListState to PEEKING
 // hides the folder view.
 TEST_F(AppListViewTest, FolderViewToPeeking) {

ash/app_list/views/contents_view.cc

@@ -181,9 +181,10 @@ void ContentsView::OnAppListViewTargetStateChanged(
   target_view_state_ = target_state;
   if (target_state == ash::AppListViewState::kClosed) {
     CancelDrag();
+    expand_arrow_view_->MaybeEnableHintingAnimation(false);
     return;
   }
-  UpdateExpandArrowFocusBehavior(target_state);
+  UpdateExpandArrowBehavior(target_state);
 }
 
 void ContentsView::SetActiveState(ash::AppListState state) {
@@ -413,7 +414,7 @@ void ContentsView::UpdateExpandArrowOpacity(ash::AppListState target_state,
   expand_arrow_view_->layer()->SetOpacity(expand_arrow_target_opacity);
 }
 
-void ContentsView::UpdateExpandArrowFocusBehavior(
+void ContentsView::UpdateExpandArrowBehavior(
     ash::AppListViewState target_state) {
   const bool expand_arrow_enabled =
       target_state == ash::AppListViewState::kPeeking;
@@ -430,6 +431,8 @@ void ContentsView::UpdateExpandArrowFocusBehavior(
       !expand_arrow_enabled);
   expand_arrow_view_->GetViewAccessibility().NotifyAccessibilityEvent(
       ax::mojom::Event::kTreeChanged);
+
+  expand_arrow_view_->MaybeEnableHintingAnimation(expand_arrow_enabled);
 }
 
 void ContentsView::UpdateSearchBoxVisibility(ash::AppListState current_state) {

ash/app_list/views/contents_view.h

@@ -250,8 +250,8 @@ class APP_LIST_EXPORT ContentsView : public views::View,
   // to true when the opacity changes gradually.
   void UpdateExpandArrowOpacity(ash::AppListState target_state, bool animate);
 
-  // Updates the expand arrow's focus behavior based on AppListViewState.
-  void UpdateExpandArrowFocusBehavior(ash::AppListViewState target_state);
+  // Updates the expand arrow's behavior based on AppListViewState.
+  void UpdateExpandArrowBehavior(ash::AppListViewState target_state);
 
   // Updates search box visibility based on the current state.
   void UpdateSearchBoxVisibility(ash::AppListState current_state);

ash/app_list/views/expand_arrow_view.cc

@@ -14,7 +14,6 @@
 #include "ash/public/cpp/app_list/vector_icons/vector_icons.h"
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
-#include "base/threading/thread_task_runner_handle.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/animation/slide_animation.h"
 #include "ui/gfx/canvas.h"
@@ -108,15 +107,6 @@ ExpandArrowView::ExpandArrowView(ContentsView* contents_view,
   animation_ = std::make_unique<gfx::SlideAnimation>(this);
   animation_->SetTweenType(gfx::Tween::LINEAR);
   animation_->SetSlideDuration(kCycleDurationInMs * 2 + kCycleIntervalInMs);
-  ResetHintingAnimation();
-  // When side shelf or tablet mode is enabled, the peeking launcher won't be
-  // shown, so the hint animation is unnecessary. Also, do not run the animation
-  // during test since we are not testing the animation and it might cause msan
-  // crash when spoken feedbacke is enabled (See https://crbug.com/926038).
-  if (!app_list_view_->is_side_shelf() && !app_list_view_->is_tablet_mode() &&
-      !AppListView::ShortAnimationsForTesting()) {
-    ScheduleHintingAnimation(true);
-  }
   SetEventTargeter(std::make_unique<views::ViewTargeter>(this));
 }
 
@@ -356,15 +346,29 @@ void ExpandArrowView::TransitToFullscreenAllAppsState() {
   app_list_view_->SetState(ash::AppListViewState::kFullscreenAllApps);
 }
 
+void ExpandArrowView::MaybeEnableHintingAnimation(bool enabled) {
+  button_pressed_ = false;
+  ResetHintingAnimation();
+  // When side shelf or tablet mode is enabled, the peeking launcher won't be
+  // shown, so the hint animation is unnecessary. Also, do not run the animation
+  // during test since we are not testing the animation and it might cause msan
+  // crash when spoken feedback is enabled (See https://crbug.com/926038).
+  if (enabled && !app_list_view_->is_side_shelf() &&
+      !app_list_view_->is_tablet_mode() &&
+      !AppListView::ShortAnimationsForTesting()) {
+    ScheduleHintingAnimation(true);
+  } else {
+    hinting_animation_timer_.Stop();
+  }
+}
+
 void ExpandArrowView::ScheduleHintingAnimation(bool is_first_time) {
   int delay_in_sec = kAnimationIntervalInSec;
   if (is_first_time)
     delay_in_sec = kAnimationInitialWaitTimeInSec;
-  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
-      FROM_HERE,
-      base::BindOnce(&ExpandArrowView::StartHintingAnimation,
-                     weak_ptr_factory_.GetWeakPtr()),
-      base::TimeDelta::FromSeconds(delay_in_sec));
+  hinting_animation_timer_.Start(FROM_HERE,
+                                 base::TimeDelta::FromSeconds(delay_in_sec),
+                                 this, &ExpandArrowView::StartHintingAnimation);
 }
 
 void ExpandArrowView::StartHintingAnimation() {

ash/app_list/views/expand_arrow_view.h

@@ -9,6 +9,7 @@
 
 #include "ash/app_list/app_list_export.h"
 #include "base/memory/weak_ptr.h"
+#include "base/timer/timer.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/view_targeter_delegate.h"
 
@@ -53,6 +54,12 @@ class APP_LIST_EXPORT ExpandArrowView : public views::Button,
   std::unique_ptr<views::InkDropMask> CreateInkDropMask() const override;
   std::unique_ptr<views::InkDropRipple> CreateInkDropRipple() const override;
 
+  void MaybeEnableHintingAnimation(bool enabled);
+
+  bool IsHintingAnimationRunningForTest() {
+    return hinting_animation_timer_.IsRunning();
+  }
+
  private:
   // gfx::AnimationDelegate overrides:
   void AnimationProgressed(const gfx::Animation* animation) override;
@@ -86,6 +93,8 @@ class APP_LIST_EXPORT ExpandArrowView : public views::Button,
   // The y position offset of the arrow in this view.
   int arrow_y_offset_;
 
+  base::OneShotTimer hinting_animation_timer_;
+
   base::WeakPtrFactory<ExpandArrowView> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(ExpandArrowView);