AppListController refactoring part 1: AppListShower implementation.

ui/app_list/app_list_shower_impl.h

Index: ui/app_list/app_list_shower_impl.h
diff --git a/ash/wm/app_list_controller.h b/ui/app_list/app_list_shower_impl.h
similarity index 44%
copy from ash/wm/app_list_controller.h
copy to ui/app_list/app_list_shower_impl.h
index 1f3114c37d4858c4bfda875bd5e4fb9696d9a4d5..9c2f238eade559bf5c2d1139aa5a1947505ac8c2 100644
--- a/ash/wm/app_list_controller.h
+++ b/ui/app_list/app_list_shower_impl.h
@@ -1,83 +1,71 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2016 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_WM_APP_LIST_CONTROLLER_H_
-#define ASH_WM_APP_LIST_CONTROLLER_H_
+#ifndef UI_APP_LIST_APP_LIST_SHOWER_IMPL_H_
+#define UI_APP_LIST_APP_LIST_SHOWER_IMPL_H_
 
-#include "ash/shelf/shelf_icon_observer.h"
-#include "ash/shell_observer.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "ui/app_list/app_list_shower.h"
+#include "ui/app_list/app_list_shower_delegate.h"
 #include "ui/app_list/pagination_model_observer.h"
 #include "ui/aura/client/focus_change_observer.h"
 #include "ui/aura/window_observer.h"
 #include "ui/compositor/layer_animation_observer.h"
-#include "ui/events/event_handler.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/keyboard/keyboard_controller_observer.h"
 #include "ui/views/widget/widget_observer.h"
 
 namespace app_list {
-class ApplicationDragAndDropHost;
 class AppListView;
-}
-
-namespace ui {
-class LocatedEvent;
-}
+class AppListViewDelegate;
+class AppListShowerDelegateFactory;
 
-namespace ash {
 namespace test {
-class AppListControllerTestApi;
+class AppListShowerImplTestApi;
 }
 
-// AppListController is a controller that manages app list UI for shell.
-// It creates AppListView and schedules showing/hiding animation.
-// While the UI is visible, it monitors things such as app list widget's
-// activation state and desktop mouse click to auto dismiss the UI.
-class AppListController : public ui::EventHandler,
-                          public aura::client::FocusChangeObserver,
-                          public aura::WindowObserver,
-                          public ui::ImplicitAnimationObserver,
-                          public views::WidgetObserver,
-                          public keyboard::KeyboardControllerObserver,
-                          public ShellObserver,
-                          public ShelfIconObserver,
-                          public app_list::PaginationModelObserver {
+class AppListShowerImplTest;
+class AppListViewDelegate;
+
+// Manages app list UI. Creates AppListView and schedules showing/hiding
+// animation. While the UI is visible, it monitors things such as app list
+// activation state to auto dismiss the UI. Delegates the responsibility
+// for laying out the app list UI to ash::AppListLayoutDelegate.
+class APP_LIST_EXPORT AppListShowerImpl
+    : public AppListShower,
+      public aura::client::FocusChangeObserver,
+      public aura::WindowObserver,
+      public ui::ImplicitAnimationObserver,
+      public views::WidgetObserver,
+      public keyboard::KeyboardControllerObserver,
+      public PaginationModelObserver {
  public:
-  AppListController();
-  ~AppListController() override;
-
-  // Show/hide app list window. The |window| is used to deterime in
-  // which display (in which the |window| exists) the app list should
-  // be shown.
-  void Show(aura::Window* window);
-  void Dismiss();
-
-  // Whether app list window is visible (shown or being shown).
-  bool IsVisible() const;
-
-  // Returns target visibility. This differs from IsVisible() if an animation
-  // is ongoing.
-  bool GetTargetVisibility() const { return is_visible_; }
+  explicit AppListShowerImpl(AppListShowerDelegateFactory* factory);
+  ~AppListShowerImpl() override;
 
   // Returns app list window or NULL if it is not visible.
   aura::Window* GetWindow();
 
   // Returns app list view if one exists, or NULL otherwise.
-  app_list::AppListView* GetView() { return view_; }
+  AppListView* GetView() { return view_; }
 
- private:
-  friend class test::AppListControllerTestApi;
+  void set_view_delegate(AppListViewDelegate* view_delegate) {

[xiyuan, 2016/03/24 20:31:12]

Can we pass this in as an arg to ctor? Using a setter is more error prone. If we
want to use it, we need to at least DCHECK(view_) to make sure it is called
before |view_| is constructed.

[mfomitchev, 2016/03/29 17:33:31]

Consider the AppListServiceAsh in the complete implementation
(crrev.com/1770993002). Unfortunately we have a circular dependency:
app_list_shower_ needs the view delegate, the view delegate needs the
controller_delegate_, and the controller_delegate_needs app_list_shower_. We
break this dependency by adding this setter. We do DCHECK for view_delegate_ in
AppListShowerImpl::Show, which is the only way to create the view.

The code could probably be refactored to eliminate the circular dependency, but
I think that means changing AppListControllerDelegate interface. That would
affect a bunch of non-ash applist classes, which would be a waste of considering
the work planned for M51 that mgiuca@ talked about.

+    view_delegate_ = view_delegate;
+  }
 
-  // If |drag_and_drop_host| is not NULL it will be called upon drag and drop
-  // operations outside the application list.
-  void SetDragAndDropHostOfCurrentAppList(
-      app_list::ApplicationDragAndDropHost* drag_and_drop_host);
+  // AppListShower:
+  void Show(aura::Window* window) override;
+  void Dismiss() override;
+  bool IsVisible() const override;
+  bool GetTargetVisibility() const override;
+
+ private:
+  friend class test::AppListShowerImplTestApi;
 
   // Sets the app list view and attempts to show it.
-  void SetView(app_list::AppListView* view);
+  void SetView(AppListView* view);
 
   // Forgets the view.
   void ResetView();
@@ -85,15 +73,6 @@ class AppListController : public ui::EventHandler,
   // Starts show/hide animation.
   void ScheduleAnimation();
 
-  void ProcessLocatedEvent(ui::LocatedEvent* event);
-
-  // Makes app list bubble update its bounds.
-  void UpdateBounds();
-
-  // ui::EventHandler overrides:
-  void OnMouseEvent(ui::MouseEvent* event) override;
-  void OnGestureEvent(ui::GestureEvent* event) override;
-
   // aura::client::FocusChangeObserver overrides:
   void OnWindowFocused(aura::Window* gained_focus,
                        aura::Window* lost_focus) override;
@@ -112,28 +91,25 @@ class AppListController : public ui::EventHandler,
   // KeyboardControllerObserver overrides:
   void OnKeyboardBoundsChanging(const gfx::Rect& new_bounds) override;
 
-  // ShellObserver overrides:
-  void OnShelfAlignmentChanged(aura::Window* root_window) override;
-  void OnMaximizeModeStarted() override;
-  void OnMaximizeModeEnded() override;
-
-  // ShelfIconObserver overrides:
-  void OnShelfIconPositionsChanged() override;
-
-  // app_list::PaginationModelObserver overrides:
+  // PaginationModelObserver overrides:
   void TotalPagesChanged() override;
   void SelectedPageChanged(int old_selected, int new_selected) override;
   void TransitionStarted() override;
   void TransitionChanged() override;
 
+  AppListShowerDelegateFactory* factory_;

[xiyuan, 2016/03/24 20:31:12]

nit: document ownership and "AppListShowerDelegateFactory* const" if it is not
going to changed.

[mfomitchev, 2016/03/29 17:33:32]

Done.

+
+  // Responsible for laying out the app list UI.
+  scoped_ptr<AppListShowerDelegate> shower_delegate_;
+
+  // Not owned.
+  AppListViewDelegate* view_delegate_;

[xiyuan, 2016/03/24 20:31:12]

nit: prefer to use member initializer instead of in ctor.
i.e. 
  AppListViewDelegate* view_delegate_ = nullptr;


The C++11 style is allowed in:
https://chromium-cpp.appspot.com/ "Non-Static Class Member Initializers"

[mfomitchev, 2016/03/29 17:33:31]

Cool, done. Thanks for the link! I updated the other fields as well.

+
   // Whether we should show or hide app list widget.
   bool is_visible_;
 
-  // Whether the app list should remain centered.
-  bool is_centered_;
-
   // The AppListView this class manages, owned by its widget.
-  app_list::AppListView* view_;
+  AppListView* view_;
 
   // The current page of the AppsGridView of |view_|. This is stored outside of
   // the view's PaginationModel, so that it persists when the view is destroyed.
@@ -144,10 +120,8 @@ class AppListController : public ui::EventHandler,
 
   // Whether should schedule snap back animation.
   bool should_snap_back_;
-
-  DISALLOW_COPY_AND_ASSIGN(AppListController);

[xiyuan, 2016/03/24 20:31:12]

Why removing this?

[mfomitchev, 2016/03/29 17:33:32]

Fixed

 };
 
-}  // namespace ash
+}  // namespace app_list
 
-#endif  // ASH_WM_APP_LIST_CONTROLLER_H_
+#endif  // UI_APP_LIST_APP_LIST_SHOWER_IMPL_H_