Revert "Remove HomeLauncherGestureHandlerObserver"

This reverts commit 433bbd52.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 711038 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzQzM2JiZDUyMTA0MmRjOTcyMmQ2ZDg2Nzk0MzNjODE0MGRmOWQwNGMM

Sample Failed Build: https://ci.chromium.org/b/8898131381528610928

Sample Failed Step: compile

Original change's description:
> Remove HomeLauncherGestureHandlerObserver
> 
> HomeLauncherGestureHandler currently has two observers:
> *   AppListControllerImpl, which is also a HomeScreenDelegate
>     implementation (on which HomeScreenGestureHandler already
>     depends, so additional observer interface just
>     obscures/complicates this dependency)
> *   HomeLauncherStateWaiter which is used by interactive UI/perf tests
>     to wait for launcher animations to complete
> 
> For the former case, moving the observer methods to HomeScreenDelegate
> interface will work fine.
> For latter case, the test waiter can register a callback with
> AppListControllerImpl to be run when the home launcher animations are
> complete - the same way it's done by LauncherStateWaiter, an equivalent
> test waiter used in clamshell mode (or for waiting for non-visibility
> app list state transitions)
> 
> Change-Id: Ia333bedab2b8683d35da2d00030cf40df2c619f5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1888652
> Reviewed-by: Manu Cornet <manucornet@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Commit-Queue: Toni Baržić <tbarzic@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#711038}


Change-Id: I449d4c15018c05c09953d445a50d1c1dc6f7c1a0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1890735
Cr-Commit-Position: refs/heads/master@{#711044}

ash/BUILD.gn

@@ -347,6 +347,7 @@ component("ash") {
     "home_screen/drag_window_from_shelf_controller.h",
     "home_screen/home_launcher_gesture_handler.cc",
     "home_screen/home_launcher_gesture_handler.h",
+    "home_screen/home_launcher_gesture_handler_observer.h",
     "home_screen/home_screen_controller.cc",
     "home_screen/home_screen_controller.h",
     "home_screen/home_screen_delegate.h",

ash/app_list/app_list_controller_impl.cc

@@ -167,6 +167,8 @@ AppListControllerImpl::AppListControllerImpl()
     shell->assistant_controller()->AddObserver(this);
     shell->assistant_controller()->ui_controller()->AddModelObserver(this);
   }
+  shell->home_screen_controller()->home_launcher_gesture_handler()->AddObserver(
+      this);
 }
 
 AppListControllerImpl::~AppListControllerImpl() {
@@ -792,9 +794,6 @@ void AppListControllerImpl::OnHomeLauncherAnimationComplete(
   // visibility is correct first.
   OnVisibilityWillChange(shown, display_id);
   OnVisibilityChanged(shown, display_id);
-
-  if (!home_launcher_animation_callback_.is_null())
-    home_launcher_animation_callback_.Run(shown);
 }
 
 void AppListControllerImpl::OnHomeLauncherTargetPositionChanged(
@@ -933,16 +932,11 @@ void AppListControllerImpl::SetAppListModelForTest(
   model_->AddObserver(this);
 }
 
-void AppListControllerImpl::SetStateTransitionAnimationCallbackForTesting(
+void AppListControllerImpl::SetStateTransitionAnimationCallback(
     StateTransitionAnimationCallback callback) {
   state_transition_animation_callback_ = std::move(callback);
 }
 
-void AppListControllerImpl::SetHomeLauncherAnimationCallbackForTesting(
-    HomeLauncherAnimationCallback callback) {
-  home_launcher_animation_callback_ = std::move(callback);
-}
-
 void AppListControllerImpl::RecordShelfAppLaunched(
     base::Optional<AppListViewState> recorded_app_list_view_state,
     base::Optional<bool> recorded_home_launcher_shown) {
@@ -1560,6 +1554,9 @@ void AppListControllerImpl::Shutdown() {
   is_shutdown_ = true;
 
   Shell* shell = Shell::Get();
+  shell->home_screen_controller()
+      ->home_launcher_gesture_handler()
+      ->RemoveObserver(this);
   if (app_list_features::IsEmbeddedAssistantUIEnabled()) {
     shell->assistant_controller()->RemoveObserver(this);
     shell->assistant_controller()->ui_controller()->RemoveModelObserver(this);

ash/app_list/app_list_controller_impl.h

@@ -20,6 +20,7 @@
 #include "ash/assistant/assistant_controller_observer.h"
 #include "ash/assistant/model/assistant_ui_model_observer.h"
 #include "ash/display/window_tree_host_manager.h"
+#include "ash/home_screen/home_launcher_gesture_handler_observer.h"
 #include "ash/home_screen/home_screen_delegate.h"
 #include "ash/public/cpp/app_list/app_list_controller.h"
 #include "ash/public/cpp/keyboard/keyboard_controller_observer.h"
@@ -63,6 +64,7 @@ class ASH_EXPORT AppListControllerImpl
       public ash::MruWindowTracker::Observer,
       public AssistantControllerObserver,
       public AssistantUiModelObserver,
+      public HomeLauncherGestureHandlerObserver,
       public HomeScreenDelegate {
  public:
   AppListControllerImpl();
@@ -268,6 +270,10 @@ class ASH_EXPORT AppListControllerImpl
       base::Optional<AssistantEntryPoint> entry_point,
       base::Optional<AssistantExitPoint> exit_point) override;
 
+  // HomeLauncherGestureHandlerObserver:
+  void OnHomeLauncherAnimationComplete(bool shown, int64_t display_id) override;
+  void OnHomeLauncherTargetPositionChanged(bool showing,
+                                           int64_t display_id) override;
 
   // HomeScreenDelegate:
   void ShowHomeScreenView() override;
@@ -284,9 +290,6 @@ class ASH_EXPORT AppListControllerImpl
   base::Optional<base::TimeDelta> GetOptionalAnimationDuration() override;
   void NotifyHomeLauncherAnimationTransition(AnimationTrigger trigger,
                                              bool launcher_will_show) override;
-  void OnHomeLauncherAnimationComplete(bool shown, int64_t display_id) override;
-  void OnHomeLauncherTargetPositionChanged(bool showing,
-                                           int64_t display_id) override;
   bool IsHomeScreenVisible() override;
   gfx::Rect GetInitialAppListItemScreenBoundsForWindow(
       aura::Window* window) override;
@@ -324,14 +327,9 @@ class ASH_EXPORT AppListControllerImpl
   using StateTransitionAnimationCallback =
       base::RepeatingCallback<void(ash::AppListViewState)>;
 
-  void SetStateTransitionAnimationCallbackForTesting(
+  void SetStateTransitionAnimationCallback(
       StateTransitionAnimationCallback callback);
 
-  using HomeLauncherAnimationCallback =
-      base::RepeatingCallback<void(bool shown)>;
-  void SetHomeLauncherAnimationCallbackForTesting(
-      HomeLauncherAnimationCallback callback);
-
   void RecordShelfAppLaunched(
       base::Optional<AppListViewState> recorded_app_list_view_state,
       base::Optional<bool> home_launcher_shown);
@@ -425,14 +423,8 @@ class ASH_EXPORT AppListControllerImpl
   // each profile has its own AppListModelUpdater to manipulate app list items.
   int profile_id_ = kAppListInvalidProfileID;
 
-  // A callback that can be registered by a test to wait for the app list state
-  // transition animation to finish.
   StateTransitionAnimationCallback state_transition_animation_callback_;
 
-  // A callback that can be registered by a test to wait for the home launcher
-  // visibility animation to finish. Should only be used in tablet mode.
-  HomeLauncherAnimationCallback home_launcher_animation_callback_;
-
   // Used to prevent ShelfLayoutManager updating visibility state when overview
   // is showing over the AppList.
   std::unique_ptr<ShelfLayoutManager::ScopedSuspendVisibilityUpdate>

ash/home_screen/home_launcher_gesture_handler.cc

@@ -9,6 +9,7 @@
 #include "ash/app_list/app_list_controller_impl.h"
 #include "ash/display/screen_orientation_controller.h"
 #include "ash/home_screen/drag_window_from_shelf_controller.h"
+#include "ash/home_screen/home_launcher_gesture_handler_observer.h"
 #include "ash/home_screen/home_screen_controller.h"
 #include "ash/home_screen/swipe_home_to_overview_controller.h"
 #include "ash/root_window_controller.h"
@@ -397,17 +398,28 @@ bool HomeLauncherGestureHandler::IsDragInProgress() const {
   return mode_ != Mode::kNone;
 }
 
+void HomeLauncherGestureHandler::AddObserver(
+    HomeLauncherGestureHandlerObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void HomeLauncherGestureHandler::RemoveObserver(
+    HomeLauncherGestureHandlerObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 void HomeLauncherGestureHandler::NotifyHomeLauncherTargetPositionChanged(
     bool showing,
     int64_t display_id) {
-  GetHomeScreenDelegate()->OnHomeLauncherTargetPositionChanged(showing,
-                                                               display_id);
+  for (auto& observer : observers_)
+    observer.OnHomeLauncherTargetPositionChanged(showing, display_id);
 }
 
 void HomeLauncherGestureHandler::NotifyHomeLauncherAnimationComplete(
     bool shown,
     int64_t display_id) {
-  GetHomeScreenDelegate()->OnHomeLauncherAnimationComplete(shown, display_id);
+  for (auto& observer : observers_)
+    observer.OnHomeLauncherAnimationComplete(shown, display_id);
 }
 
 void HomeLauncherGestureHandler::OnWindowDestroying(aura::Window* window) {

ash/home_screen/home_launcher_gesture_handler.h

@@ -24,6 +24,7 @@
 
 namespace ash {
 
+class HomeLauncherGestureHandlerObserver;
 class SwipeHomeToOverviewController;
 
 // HomeLauncherGestureHandler makes modifications to a window's transform and
@@ -79,6 +80,9 @@ class ASH_EXPORT HomeLauncherGestureHandler
 
   bool IsDragInProgress() const;
 
+  void AddObserver(HomeLauncherGestureHandlerObserver* observer);
+  void RemoveObserver(HomeLauncherGestureHandlerObserver* obsever);
+
   void NotifyHomeLauncherTargetPositionChanged(bool showing,
                                                int64_t display_id);
   void NotifyHomeLauncherAnimationComplete(bool shown, int64_t display_id);
@@ -209,6 +213,8 @@ class ASH_EXPORT HomeLauncherGestureHandler
   std::unique_ptr<SwipeHomeToOverviewController>
       swipe_home_to_overview_controller_;
 
+  base::ObserverList<HomeLauncherGestureHandlerObserver> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(HomeLauncherGestureHandler);
 };
 

ash/home_screen/home_launcher_gesture_handler_observer.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_HOME_SCREEN_HOME_LAUNCHER_GESTURE_HANDLER_OBSERVER_H_
+#define ASH_HOME_SCREEN_HOME_LAUNCHER_GESTURE_HANDLER_OBSERVER_H_
+
+#include "base/observer_list_types.h"
+
+namespace ash {
+
+class HomeLauncherGestureHandlerObserver : public base::CheckedObserver {
+ public:
+  // Called when the HomeLauncher has started to be dragged, or a positional
+  // animation has begun.
+  virtual void OnHomeLauncherTargetPositionChanged(bool showing,
+                                                   int64_t display_id) {}
+
+  // Called when the HomeLauncher positional animation has completed.
+  virtual void OnHomeLauncherAnimationComplete(bool shown, int64_t display_id) {
+  }
+};
+
+}  // namespace ash
+
+#endif  // ASH_HOME_SCREEN_HOME_LAUNCHER_GESTURE_HANDLER_OBSERVER_H_

ash/home_screen/home_screen_delegate.h

@@ -98,15 +98,6 @@ class HomeScreenDelegate {
   // whether the launcher will show by the end of animation.
   virtual void NotifyHomeLauncherAnimationTransition(AnimationTrigger trigger,
                                                      bool launcher_will_show) {}
-
-  // Called when the HomeLauncher has started to be dragged, or a positional
-  // animation has begun.
-  virtual void OnHomeLauncherTargetPositionChanged(bool showing,
-                                                   int64_t display_id) {}
-
-  // Called when the HomeLauncher positional animation has completed.
-  virtual void OnHomeLauncherAnimationComplete(bool shown, int64_t display_id) {
-  }
 };
 
 }  // namespace ash

ash/shell_test_api.cc

@@ -12,6 +12,8 @@
 #include "ash/app_list/app_list_controller_impl.h"
 #include "ash/app_list/app_list_presenter_impl.h"
 #include "ash/app_list/views/app_list_view.h"
+#include "ash/home_screen/home_launcher_gesture_handler.h"
+#include "ash/home_screen/home_launcher_gesture_handler_observer.h"
 #include "ash/home_screen/home_screen_controller.h"
 #include "ash/keyboard/keyboard_controller_impl.h"
 #include "ash/public/cpp/app_list/app_list_types.h"
@@ -72,26 +74,26 @@ class PointerMoveLoopWaiter : public ui::CompositorObserver {
   DISALLOW_COPY_AND_ASSIGN(PointerMoveLoopWaiter);
 };
 
-class HomeLauncherStateWaiter {
+class HomeLauncherStateWaiter : public HomeLauncherGestureHandlerObserver {
  public:
   HomeLauncherStateWaiter(bool target_shown, base::OnceClosure closure)
       : target_shown_(target_shown), closure_(std::move(closure)) {
     Shell::Get()
-        ->app_list_controller()
-        ->SetHomeLauncherAnimationCallbackForTesting(base::BindRepeating(
-            &HomeLauncherStateWaiter::OnHomeLauncherAnimationCompleted,
-            base::Unretained(this)));
+        ->home_screen_controller()
+        ->home_launcher_gesture_handler()
+        ->AddObserver(this);
   }
-  ~HomeLauncherStateWaiter() {
+  ~HomeLauncherStateWaiter() override {
     Shell::Get()
-        ->app_list_controller()
-        ->SetHomeLauncherAnimationCallbackForTesting(base::NullCallback());
+        ->home_screen_controller()
+        ->home_launcher_gesture_handler()
+        ->RemoveObserver(this);
   }
 
  private:
-  // Passed to AppListControllerImpl as a callback to run when home launcher
-  // transition animation is complete.
-  void OnHomeLauncherAnimationCompleted(bool shown) {
+  // HomeLauncherGestureHandlerObserver:
+  void OnHomeLauncherAnimationComplete(bool shown,
+                                       int64_t display_id) override {
     if (shown == target_shown_) {
       std::move(closure_).Run();
       delete this;
@@ -110,15 +112,13 @@ class LauncherStateWaiter {
  public:
   LauncherStateWaiter(ash::AppListViewState state, base::OnceClosure closure)
       : target_state_(state), closure_(std::move(closure)) {
-    Shell::Get()
-        ->app_list_controller()
-        ->SetStateTransitionAnimationCallbackForTesting(base::BindRepeating(
-            &LauncherStateWaiter::OnStateChanged, base::Unretained(this)));
+    Shell::Get()->app_list_controller()->SetStateTransitionAnimationCallback(
+        base::BindRepeating(&LauncherStateWaiter::OnStateChanged,
+                            base::Unretained(this)));
   }
   ~LauncherStateWaiter() {
-    Shell::Get()
-        ->app_list_controller()
-        ->SetStateTransitionAnimationCallbackForTesting(base::NullCallback());
+    Shell::Get()->app_list_controller()->SetStateTransitionAnimationCallback(
+        base::NullCallback());
   }
 
   void OnStateChanged(ash::AppListViewState state) {