Refactor: Shelf Widget

ash/launcher/launcher.h

Index: ash/launcher/launcher.h
diff --git a/ash/launcher/launcher.h b/ash/launcher/launcher.h
index 79389b0137fbf2d8202a1dbbdf226f7ff30bd8f2..4e6b7951ad1495803f64fdd53d8316e837349bf5 100644
--- a/ash/launcher/launcher.h
+++ b/ash/launcher/launcher.h
@@ -6,9 +6,8 @@
 #define ASH_LAUNCHER_LAUNCHER_H_
 
 #include "ash/ash_export.h"
-#include "ash/launcher/background_animator.h"
 #include "ash/launcher/launcher_types.h"
-#include "ash/shelf_types.h"
+#include "ash/shelf/shelf_types.h"
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/gfx/size.h"
@@ -24,7 +23,6 @@ class Rect;
 
 namespace views {
 class View;
-class Widget;
 }
 
 namespace ash {
@@ -38,13 +36,13 @@ class ShelfLayoutManager;
 class LauncherIconObserver;
 class LauncherDelegate;
 class LauncherModel;
+class ShelfWidget;
 
-class ASH_EXPORT Launcher: public views::WidgetObserver  {
+class ASH_EXPORT Launcher {
  public:
   Launcher(LauncherModel* launcher_model,
            LauncherDelegate* launcher_delegate,
-           aura::Window* window_container,
-           internal::ShelfLayoutManager* shelf_layout_manager);
+           ShelfWidget* shelf_widget);
   virtual ~Launcher();
 
   // Return the launcher for the primary display. NULL if no user is
@@ -56,30 +54,9 @@ class ASH_EXPORT Launcher: public views::WidgetObserver  {
   // is disabled. NULL if no user is logged in yet.
   static Launcher* ForWindow(aura::Window* window);
 
-  // Sets the focus cycler.  Also adds the launcher to the cycle.
-  void SetFocusCycler(internal::FocusCycler* focus_cycler);
-  internal::FocusCycler* GetFocusCycler();
-
   void SetAlignment(ShelfAlignment alignment);
   ShelfAlignment alignment() const { return alignment_; }
 
-  // Sets whether the launcher paints a background. Default is false, but is set
-  // to true if a window overlaps the shelf.
-  void SetPaintsBackground(
-      bool value,
-      internal::BackgroundAnimator::ChangeType change_type);
-  bool paints_background() const {
-    return background_animator_.paints_background();
-  }
-
-  // Causes shelf items to be slightly dimmed.
-  void SetDimsShelf(bool value);
-  bool GetDimsShelf() const;
-
-  // Sets the size of the status area.
-  void SetStatusSize(const gfx::Size& size);
-  const gfx::Size& status_size() const { return status_size_; }
-
   // Returns the screen bounds of the item for the specified window. If there is
   // no item for the specified window an empty rect is returned.
   gfx::Rect GetScreenBoundsOfItemIconForWindow(aura::Window* window);
@@ -103,12 +80,10 @@ class ASH_EXPORT Launcher: public views::WidgetObserver  {
   bool IsShowingOverflowBubble() const;
 
   void SetVisible(bool visible) const;
+  bool visible() const;
 
   views::View* GetAppListButtonView() const;
 
-  // Sets the bounds of the launcher widget, and the dimmer if visible.
-  void SetWidgetBounds(const gfx::Rect bounds);
-
   // Switches to a 0-indexed (in order of creation) window.
   // A negative index switches to the last window in the list.
   void SwitchToWindow(int window_index);
@@ -119,33 +94,12 @@ class ASH_EXPORT Launcher: public views::WidgetObserver  {
 
   LauncherDelegate* delegate() { return delegate_; }
 
-  views::Widget* widget() { return widget_.get(); }
-
-  views::Widget* GetDimmerWidgetForTest() { return dimmer_.get(); }
-
-  aura::Window* window_container() { return window_container_; }
+  ShelfWidget* shelf_widget() const { return shelf_widget_; }
 
-  // Called by the activation delegate, before the launcher is activated
-  // when no other windows are visible.
-  void WillActivateAsFallback() { activating_as_fallback_ = true; }
-
-  // Overridden from views::WidgetObserver:
-  virtual void OnWidgetActivationChanged(
-      views::Widget* widget, bool active) OVERRIDE;
+  void SetViewBounds(gfx::Rect bounds);

[Mr4D (OOO till 08-26), 2013/02/27 19:42:51]

Could you please add a comment for the new functions? Also - "SetViewBounds": In
most cases we have a SetBounds and fro animations something like
SetTargetBounds, why do we need another one? 

[Harry McCleave, 2013/03/01 04:31:19]

Will add comment, but would like some input on correct name first, these
functions provide access to get/set the bounds of the launcher_view view, where
it is not publicly exposed by the launcher (except via a function required by
test framework marked todo:remove), they might be possible to remove when the
status area becomes a view, as they are used right now to size/position the
major axis of the launcher so it is the correct size/side of the status area
widget.

[Mr4D (OOO till 08-26), 2013/03/02 02:02:12]

So as a general comment: If a function is only used for tests it should have a
name like "<bla>_for_test(..)" or similar. It is is also used elsewhere I would
name it "SetBounds()".
So if only a unit test: SetViewBoundsForTest(..), otherwise only SetBounds. In
the end Sky might have a different opinion there. (but: Eclipse - Refactor /
rename, done!)

+  gfx::Rect view_bounds() const;
 
  private:
-  class DelegateView;
-  class DimmerView;
-
-  // Widget hosting the view.
-  scoped_ptr<views::Widget> widget_;
-  scoped_ptr<views::Widget> dimmer_;
-
-  aura::Window* window_container_;
-
-  // Contents view of the widget. Houses the LauncherView.
-  DelegateView* delegate_view_;
-
   // LauncherView used to display icons.
   internal::LauncherView* launcher_view_;
 
@@ -153,14 +107,7 @@ class ASH_EXPORT Launcher: public views::WidgetObserver  {
 
   LauncherDelegate* delegate_;
 
-  // Size reserved for the status area.
-  gfx::Size status_size_;
-
-  // Used to animate the background.
-  internal::BackgroundAnimator background_animator_;
-
-  // Used then activation is forced from the activation delegate.
-  bool activating_as_fallback_;
+  ShelfWidget* shelf_widget_;
 
   DISALLOW_COPY_AND_ASSIGN(Launcher);
 };