Cleanup ash shelf accessor functions.

Remove RootWindowController::ForShelf, use ForWindow instead. Remove ShelfLayoutManager::ForShelf, use Shelf::For*, etc. Avoid direct RootWindowController use with Shelf::ForWindow. Add Shelf::shelf_layout_manager() accessor via ShelfWidget. (avoid Shelf::ForWindow in shell.cc for ShelfDelegate init path) (No Shelf exists for ShelfWidget::CreateShelf's delegate init) Make ShelfView take a Shelf* pointer; not ShelfLayoutManager. Add Shelf alignment helpers to avoid ShelfLayoutManager use. Miscellaneous refactoring of shelf object access callsites. TODO: Remove Shell::[G|S]etShelfAutoHideBehavior TODO: Remove Shell::[G|S]etShelfAlignment BUG=557406 TEST=No regressions or behavior changes. [email protected] Review URL: https://codereview.chromium.org/1743493002 Cr-Commit-Position: refs/heads/master@{#378392}
huningxin · Mar 1, 2016 · a7e8a5f · a7e8a5f
1 parent 3b893b5
commit a7e8a5f
Show file tree

Hide file tree

Showing 33 changed files with 275 additions and 372 deletions.
diff --git a/ash/focus_cycler_unittest.cc b/ash/focus_cycler_unittest.cc
@@ -6,7 +6,6 @@
 
 #include "ash/root_window_controller.h"
 #include "ash/shelf/shelf.h"
-#include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
 #include "ash/shell_factory.h"
@@ -94,9 +93,7 @@ class FocusCyclerTest : public AshTestBase {
         Shell::GetPrimaryRootWindowController()->GetContainer(
             ash::kShellWindowId_StatusContainer);
 
-    StatusAreaWidget* widget = new StatusAreaWidget(
-        parent,
-        ShelfLayoutManager::ForShelf(parent->GetRootWindow())->shelf_widget());
+    StatusAreaWidget* widget = new StatusAreaWidget(parent, shelf_widget());
     widget->CreateTrayViews();
     widget->Show();
     tray_.reset(widget->system_tray());

diff --git a/ash/metrics/user_metrics_recorder.cc b/ash/metrics/user_metrics_recorder.cc
@@ -6,12 +6,11 @@
 
 #include "ash/metrics/desktop_task_switch_metric_recorder.h"
 #include "ash/session/session_state_delegate.h"
+#include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_delegate.h"
 #include "ash/shelf/shelf_item_types.h"
-#include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shelf/shelf_model.h"
 #include "ash/shelf/shelf_view.h"
-#include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #include "ash/system/tray/system_tray_delegate.h"
@@ -622,18 +621,16 @@ void UserMetricsRecorder::OnShellShuttingDown() {
 }
 
 void UserMetricsRecorder::RecordPeriodicMetrics() {
-  ShelfLayoutManager* manager =
-      ShelfLayoutManager::ForShelf(Shell::GetPrimaryRootWindow());
+  Shelf* shelf = Shelf::ForPrimaryDisplay();
   // TODO(bruthig): Investigating whether the check for |manager| is necessary
   // and add tests if it is.
-  if (manager) {
+  if (shelf) {
     // TODO(bruthig): Consider tracking the time spent in each alignment.
     UMA_HISTOGRAM_ENUMERATION("Ash.ShelfAlignmentOverTime",
-                              manager->SelectValueForShelfAlignment(
+                              shelf->SelectValueForShelfAlignment(
                                   SHELF_ALIGNMENT_UMA_ENUM_VALUE_BOTTOM,
                                   SHELF_ALIGNMENT_UMA_ENUM_VALUE_LEFT,
-                                  SHELF_ALIGNMENT_UMA_ENUM_VALUE_RIGHT,
-                                  -1),
+                                  SHELF_ALIGNMENT_UMA_ENUM_VALUE_RIGHT, -1),
                               SHELF_ALIGNMENT_UMA_ENUM_VALUE_COUNT);
   }
 

diff --git a/ash/root_window_controller.cc b/ash/root_window_controller.cc
@@ -273,13 +273,6 @@ void RootWindowController::CreateForSecondaryDisplay(AshWindowTreeHost* host) {
   controller->Init(RootWindowController::SECONDARY, false /* first run */);
 }
 
-// static
-RootWindowController* RootWindowController::ForShelf(
-    const aura::Window* window) {
-  CHECK(Shell::HasInstance());
-  return GetRootWindowController(window->GetRootWindow());
-}
-
 // static
 RootWindowController* RootWindowController::ForWindow(
     const aura::Window* window) {

diff --git a/ash/root_window_controller.h b/ash/root_window_controller.h
@@ -85,12 +85,6 @@ class ASH_EXPORT RootWindowController : public ShellObserver {
   // Creates and Initialize the RootWindowController for secondary displays.
   static void CreateForSecondaryDisplay(AshWindowTreeHost* host);
 
-  // Returns a RootWindowController that has a shelf for given
-  // |window|. This returns the RootWindowController for the |window|'s
-  // root window when multiple shelf mode is enabled, or the primary
-  // RootWindowController otherwise.
-  static RootWindowController* ForShelf(const aura::Window* window);
-
   // Returns a RootWindowController of the window's root window.
   static RootWindowController* ForWindow(const aura::Window* window);
 

diff --git a/ash/shelf/overflow_bubble.cc b/ash/shelf/overflow_bubble.cc
@@ -6,7 +6,7 @@
 
 #include "ash/root_window_controller.h"
 #include "ash/shelf/overflow_bubble_view.h"
-#include "ash/shelf/shelf_layout_manager.h"
+#include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_view.h"
 #include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
@@ -94,7 +94,7 @@ void OverflowBubble::OnWidgetDestroying(views::Widget* widget) {
   DCHECK(widget == bubble_->GetWidget());
   bubble_ = NULL;
   anchor_ = NULL;
-  shelf_view_->shelf_layout_manager()->shelf_widget()->shelf()->SchedulePaint();
+  shelf_view_->shelf()->SchedulePaint();
   shelf_view_ = NULL;
 }
 

diff --git a/ash/shelf/overflow_bubble_view.cc b/ash/shelf/overflow_bubble_view.cc
@@ -7,8 +7,8 @@
 #include <algorithm>
 
 #include "ash/root_window_controller.h"
+#include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_constants.h"
-#include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shelf/shelf_view.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
@@ -32,12 +32,9 @@ const int kShelfViewLeadingInset = 8;
 
 }  // namespace
 
-OverflowBubbleView::OverflowBubbleView()
-    : shelf_view_(NULL) {
-}
+OverflowBubbleView::OverflowBubbleView() : shelf_view_(NULL) {}
 
-OverflowBubbleView::~OverflowBubbleView() {
-}
+OverflowBubbleView::~OverflowBubbleView() {}
 
 void OverflowBubbleView::InitOverflowBubble(views::View* anchor,
                                             ShelfView* shelf_view) {
@@ -65,9 +62,7 @@ void OverflowBubbleView::InitOverflowBubble(views::View* anchor,
 }
 
 bool OverflowBubbleView::IsHorizontalAlignment() const {
-  ShelfLayoutManager* shelf_layout_manager = GetShelfLayoutManager();
-  return shelf_layout_manager ? shelf_layout_manager->IsHorizontalAlignment()
-                              : false;
+  return shelf_view_ ? shelf_view_->shelf()->IsHorizontalAlignment() : false;
 }
 
 const gfx::Size OverflowBubbleView::GetContentsSize() const {
@@ -76,14 +71,11 @@ const gfx::Size OverflowBubbleView::GetContentsSize() const {
 
 // Gets arrow location based on shelf alignment.
 views::BubbleBorder::Arrow OverflowBubbleView::GetBubbleArrow() const {
-  ShelfLayoutManager* shelf_layout_manager = GetShelfLayoutManager();
-  return shelf_layout_manager ?
-      shelf_layout_manager->SelectValueForShelfAlignment(
-          views::BubbleBorder::BOTTOM_LEFT,
-          views::BubbleBorder::LEFT_TOP,
-          views::BubbleBorder::RIGHT_TOP,
-          views::BubbleBorder::TOP_LEFT) :
-      views::BubbleBorder::NONE;
+  if (!shelf_view_)
+    return views::BubbleBorder::NONE;
+  return shelf_view_->shelf()->SelectValueForShelfAlignment(
+      views::BubbleBorder::BOTTOM_LEFT, views::BubbleBorder::LEFT_TOP,
+      views::BubbleBorder::RIGHT_TOP, views::BubbleBorder::TOP_LEFT);
 }
 
 void OverflowBubbleView::ScrollByXOffset(int x_offset) {
@@ -106,10 +98,6 @@ void OverflowBubbleView::ScrollByYOffset(int y_offset) {
   scroll_offset_.set_y(y);
 }
 
-ShelfLayoutManager* OverflowBubbleView::GetShelfLayoutManager() const {
-  return shelf_view_ ? shelf_view_->shelf_layout_manager() : nullptr;
-}
-
 gfx::Size OverflowBubbleView::GetPreferredSize() const {
   gfx::Size preferred_size = GetContentsSize();
 

diff --git a/ash/shelf/overflow_bubble_view.h b/ash/shelf/overflow_bubble_view.h
@@ -11,7 +11,6 @@
 #include "ui/views/bubble/bubble_delegate.h"
 
 namespace ash {
-class ShelfLayoutManager;
 class ShelfView;
 
 namespace test {
@@ -43,8 +42,6 @@ class ASH_EXPORT OverflowBubbleView : public views::BubbleDelegateView {
   void ScrollByXOffset(int x_offset);
   void ScrollByYOffset(int y_offset);
 
-  ShelfLayoutManager* GetShelfLayoutManager() const;
-
   // views::View overrides:
   gfx::Size GetPreferredSize() const override;
   void Layout() override;

diff --git a/ash/shelf/overflow_button.cc b/ash/shelf/overflow_button.cc
@@ -5,6 +5,7 @@
 #include "ash/shelf/overflow_button.h"
 
 #include "ash/ash_switches.h"
+#include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shelf/shelf_widget.h"
 #include "grit/ash_resources.h"
@@ -34,8 +35,7 @@ const int kBackgroundOffset = (48 - kButtonHoverSize) / 2;
 
 }  // namesapce
 
-OverflowButton::OverflowButton(views::ButtonListener* listener,
-                               ShelfLayoutManager* shelf)
+OverflowButton::OverflowButton(views::ButtonListener* listener, Shelf* shelf)
     : CustomButton(listener), bottom_image_(nullptr), shelf_(shelf) {
   ui::ResourceBundle* rb = &ui::ResourceBundle::GetSharedInstance();
   bottom_image_ = rb->GetImageNamed(IDR_ASH_SHELF_OVERFLOW).ToImageSkia();
@@ -80,12 +80,10 @@ void OverflowButton::PaintBackground(gfx::Canvas* canvas, int alpha) {
 }
 
 void OverflowButton::OnPaint(gfx::Canvas* canvas) {
-  ShelfAlignment alignment = shelf_->GetAlignment();
-
   gfx::Rect bounds(GetContentsBounds());
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   int background_image_id = 0;
-  if (shelf_->shelf_widget()->shelf()->IsShowingOverflowBubble())
+  if (shelf_->IsShowingOverflowBubble())
     background_image_id = IDR_AURA_NOTIFICATION_BACKGROUND_PRESSED;
   else if (shelf_->shelf_widget()->GetDimsShelf())
     background_image_id = IDR_AURA_NOTIFICATION_BACKGROUND_ON_BLACK;
@@ -94,6 +92,7 @@ void OverflowButton::OnPaint(gfx::Canvas* canvas) {
 
   const gfx::ImageSkia* background =
       rb.GetImageNamed(background_image_id).ToImageSkia();
+  ShelfAlignment alignment = shelf_->alignment();
   if (alignment == SHELF_ALIGNMENT_LEFT) {
     bounds = gfx::Rect(
         bounds.right() - background->width() -

diff --git a/ash/shelf/overflow_button.h b/ash/shelf/overflow_button.h
@@ -13,12 +13,12 @@
 
 namespace ash {
 
-class ShelfLayoutManager;
+class Shelf;
 
 // Shelf overflow chevron button.
 class OverflowButton : public views::CustomButton {
  public:
-  OverflowButton(views::ButtonListener* listener, ShelfLayoutManager* shelf);
+  OverflowButton(views::ButtonListener* listener, Shelf* shelf);
   ~OverflowButton() override;
 
   void OnShelfAlignmentChanged();
@@ -35,7 +35,7 @@ class OverflowButton : public views::CustomButton {
   gfx::ImageSkia right_image_;
   // Bottom image is owned by the resource bundle.
   const gfx::ImageSkia* bottom_image_;
-  ShelfLayoutManager* shelf_;
+  Shelf* shelf_;
 
   DISALLOW_COPY_AND_ASSIGN(OverflowButton);
 };

diff --git a/ash/shelf/shelf.cc b/ash/shelf/shelf.cc
@@ -18,7 +18,6 @@
 #include "ash/shelf/shelf_navigator.h"
 #include "ash/shelf/shelf_util.h"
 #include "ash/shelf/shelf_view.h"
-#include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
 #include "ash/shell_delegate.h"
 #include "ash/shell_window_ids.h"
@@ -47,8 +46,7 @@ Shelf::Shelf(ShelfModel* shelf_model,
       alignment_(shelf_widget->GetAlignment()),
       delegate_(shelf_delegate),
       shelf_widget_(shelf_widget) {
-  shelf_view_ = new ShelfView(
-      shelf_model, delegate_, shelf_widget_->shelf_layout_manager());
+  shelf_view_ = new ShelfView(shelf_model, delegate_, this);
   shelf_view_->Init();
   shelf_widget_->GetContentsView()->AddChildView(shelf_view_);
   shelf_widget_->GetNativeView()->SetName(kNativeViewName);
@@ -61,14 +59,12 @@ Shelf::~Shelf() {
 
 // static
 Shelf* Shelf::ForPrimaryDisplay() {
-  ShelfWidget* shelf_widget =
-      RootWindowController::ForShelf(Shell::GetPrimaryRootWindow())->shelf();
-  return shelf_widget ? shelf_widget->shelf() : NULL;
+  return Shelf::ForWindow(Shell::GetPrimaryRootWindow());
 }
 
 // static
-Shelf* Shelf::ForWindow(aura::Window* window) {
-  ShelfWidget* shelf_widget = RootWindowController::ForShelf(window)->shelf();
+Shelf* Shelf::ForWindow(const aura::Window* window) {
+  ShelfWidget* shelf_widget = RootWindowController::ForWindow(window)->shelf();
   return shelf_widget ? shelf_widget->shelf() : NULL;
 }
 
@@ -78,6 +74,11 @@ void Shelf::SetAlignment(ShelfAlignment alignment) {
   // ShelfLayoutManager will resize the shelf.
 }
 
+bool Shelf::IsHorizontalAlignment() const {
+  return alignment_ == SHELF_ALIGNMENT_BOTTOM ||
+         alignment_ == SHELF_ALIGNMENT_TOP;
+}
+
 gfx::Rect Shelf::GetScreenBoundsOfItemIconForWindow(
     const aura::Window* window) {
   ShelfID id = GetShelfIDForWindow(window);

diff --git a/ash/shelf/shelf.h b/ash/shelf/shelf.h
@@ -8,6 +8,7 @@
 #include "ash/ash_export.h"
 #include "ash/shelf/shelf_constants.h"
 #include "ash/shelf/shelf_types.h"
+#include "ash/shelf/shelf_widget.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
 #include "ui/gfx/geometry/size.h"
@@ -35,13 +36,11 @@ class ShelfDelegate;
 class ShelfIconObserver;
 class ShelfModel;
 class ShelfView;
-class ShelfWidget;
 
 namespace test {
 class ShelfTestAPI;
 }
 
-
 class ASH_EXPORT Shelf {
  public:
   static const char kNativeViewName[];
@@ -53,12 +52,36 @@ class ASH_EXPORT Shelf {
   static Shelf* ForPrimaryDisplay();
 
   // Return the shelf for the display that |window| is currently on, or a shelf
-  // on primary display if the shelf per display feature  is disabled. NULL if
-  // no user is logged in yet.
-  static Shelf* ForWindow(aura::Window* window);
+  // on primary display if the shelf per display feature is disabled. NULL if no
+  // user is logged in yet.
+  static Shelf* ForWindow(const aura::Window* window);
 
   void SetAlignment(ShelfAlignment alignment);
   ShelfAlignment alignment() const { return alignment_; }
+  bool IsHorizontalAlignment() const;
+
+  // A helper functions that chooses values specific to a shelf alignment.
+  template <typename T>
+  T SelectValueForShelfAlignment(T bottom, T left, T right, T top) const {
+    switch (alignment_) {
+      case SHELF_ALIGNMENT_BOTTOM:
+        return bottom;
+      case SHELF_ALIGNMENT_LEFT:
+        return left;
+      case SHELF_ALIGNMENT_RIGHT:
+        return right;
+      case SHELF_ALIGNMENT_TOP:
+        return top;
+    }
+    NOTREACHED();
+    return right;
+  }
+
+  // A helper functions that chooses values specific to a shelf alignment type.
+  template <typename T>
+  T PrimaryAxisValue(T horizontal, T vertical) const {
+    return IsHorizontalAlignment() ? horizontal : vertical;
+  }
 
   // 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.
@@ -96,6 +119,11 @@ class ASH_EXPORT Shelf {
 
   ShelfWidget* shelf_widget() { return shelf_widget_; }
 
+  // TODO(msw): ShelfLayoutManager should not be accessed externally.
+  ShelfLayoutManager* shelf_layout_manager() {
+    return shelf_widget_->shelf_layout_manager();
+  }
+
   // Set the bounds of the shelf view.
   void SetShelfViewBounds(gfx::Rect bounds);
   gfx::Rect GetShelfViewBounds() const;

diff --git a/ash/shelf/shelf_alignment_menu.cc b/ash/shelf/shelf_alignment_menu.cc
@@ -5,7 +5,7 @@
 #include "ash/shelf/shelf_alignment_menu.h"
 
 #include "ash/metrics/user_metrics_recorder.h"
-#include "ash/shelf/shelf_layout_manager.h"
+#include "ash/shelf/shelf.h"
 #include "ash/shelf/shelf_types.h"
 #include "ash/shell.h"
 #include "grit/ash_strings.h"
@@ -33,11 +33,10 @@ ShelfAlignmentMenu::ShelfAlignmentMenu(aura::Window* root)
 ShelfAlignmentMenu::~ShelfAlignmentMenu() {}
 
 bool ShelfAlignmentMenu::IsCommandIdChecked(int command_id) const {
-  return ShelfLayoutManager::ForShelf(root_window_)
+  return Shelf::ForWindow(root_window_)
       ->SelectValueForShelfAlignment(MENU_ALIGN_BOTTOM == command_id,
                                      MENU_ALIGN_LEFT == command_id,
-                                     MENU_ALIGN_RIGHT == command_id,
-                                     false);
+                                     MENU_ALIGN_RIGHT == command_id, false);
 }
 
 bool ShelfAlignmentMenu::IsCommandIdEnabled(int command_id) const {