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/+/1888652Reviewed-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}

ash/BUILD.gn

@@ -347,7 +347,6 @@ 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,8 +167,6 @@ 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() {
@@ -794,6 +792,9 @@ 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(
@@ -932,11 +933,16 @@ void AppListControllerImpl::SetAppListModelForTest(
   model_->AddObserver(this);
 }
 
-void AppListControllerImpl::SetStateTransitionAnimationCallback(
+void AppListControllerImpl::SetStateTransitionAnimationCallbackForTesting(
     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) {
@@ -1554,9 +1560,6 @@ 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,7 +20,6 @@
 #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"
@@ -64,7 +63,6 @@ class ASH_EXPORT AppListControllerImpl
       public ash::MruWindowTracker::Observer,
       public AssistantControllerObserver,
       public AssistantUiModelObserver,
-      public HomeLauncherGestureHandlerObserver,
       public HomeScreenDelegate {
  public:
   AppListControllerImpl();
@@ -270,10 +268,6 @@ 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;
@@ -290,6 +284,9 @@ 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;
@@ -327,9 +324,14 @@ class ASH_EXPORT AppListControllerImpl
   using StateTransitionAnimationCallback =
       base::RepeatingCallback<void(ash::AppListViewState)>;
 
-  void SetStateTransitionAnimationCallback(
+  void SetStateTransitionAnimationCallbackForTesting(
       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);
@@ -423,8 +425,14 @@ 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,7 +9,6 @@
 #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"
@@ -398,28 +397,17 @@ 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) {
-  for (auto& observer : observers_)
-    observer.OnHomeLauncherTargetPositionChanged(showing, display_id);
+  GetHomeScreenDelegate()->OnHomeLauncherTargetPositionChanged(showing,
+                                                               display_id);
 }
 
 void HomeLauncherGestureHandler::NotifyHomeLauncherAnimationComplete(
     bool shown,
     int64_t display_id) {
-  for (auto& observer : observers_)
-    observer.OnHomeLauncherAnimationComplete(shown, display_id);
+  GetHomeScreenDelegate()->OnHomeLauncherAnimationComplete(shown, display_id);
 }
 
 void HomeLauncherGestureHandler::OnWindowDestroying(aura::Window* window) {

ash/home_screen/home_launcher_gesture_handler.h

@@ -24,7 +24,6 @@
 
 namespace ash {
 
-class HomeLauncherGestureHandlerObserver;
 class SwipeHomeToOverviewController;
 
 // HomeLauncherGestureHandler makes modifications to a window's transform and
@@ -80,9 +79,6 @@ 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);
@@ -213,8 +209,6 @@ 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,6 +98,15 @@ 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,8 +12,6 @@
 #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"
@@ -74,26 +72,26 @@ class PointerMoveLoopWaiter : public ui::CompositorObserver {
   DISALLOW_COPY_AND_ASSIGN(PointerMoveLoopWaiter);
 };
 
-class HomeLauncherStateWaiter : public HomeLauncherGestureHandlerObserver {
+class HomeLauncherStateWaiter {
  public:
   HomeLauncherStateWaiter(bool target_shown, base::OnceClosure closure)
       : target_shown_(target_shown), closure_(std::move(closure)) {
     Shell::Get()
-        ->home_screen_controller()
-        ->home_launcher_gesture_handler()
-        ->AddObserver(this);
+        ->app_list_controller()
+        ->SetHomeLauncherAnimationCallbackForTesting(base::BindRepeating(
+            &HomeLauncherStateWaiter::OnHomeLauncherAnimationCompleted,
+            base::Unretained(this)));
   }
-  ~HomeLauncherStateWaiter() override {
+  ~HomeLauncherStateWaiter() {
     Shell::Get()
-        ->home_screen_controller()
-        ->home_launcher_gesture_handler()
-        ->RemoveObserver(this);
+        ->app_list_controller()
+        ->SetHomeLauncherAnimationCallbackForTesting(base::NullCallback());
   }
 
  private:
-  // HomeLauncherGestureHandlerObserver:
-  void OnHomeLauncherAnimationComplete(bool shown,
-                                       int64_t display_id) override {
+  // Passed to AppListControllerImpl as a callback to run when home launcher
+  // transition animation is complete.
+  void OnHomeLauncherAnimationCompleted(bool shown) {
     if (shown == target_shown_) {
       std::move(closure_).Run();
       delete this;
@@ -112,13 +110,15 @@ class LauncherStateWaiter {
  public:
   LauncherStateWaiter(ash::AppListViewState state, base::OnceClosure closure)
       : target_state_(state), closure_(std::move(closure)) {
-    Shell::Get()->app_list_controller()->SetStateTransitionAnimationCallback(
-        base::BindRepeating(&LauncherStateWaiter::OnStateChanged,
-                            base::Unretained(this)));
+    Shell::Get()
+        ->app_list_controller()
+        ->SetStateTransitionAnimationCallbackForTesting(base::BindRepeating(
+            &LauncherStateWaiter::OnStateChanged, base::Unretained(this)));
   }
   ~LauncherStateWaiter() {
-    Shell::Get()->app_list_controller()->SetStateTransitionAnimationCallback(
-        base::NullCallback());
+    Shell::Get()
+        ->app_list_controller()
+        ->SetStateTransitionAnimationCallbackForTesting(base::NullCallback());
   }
 
   void OnStateChanged(ash::AppListViewState state) {