Make the chevron menu button responsible for legacy overflow menu logic

chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc

Index: chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
diff --git a/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc b/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
index 86009f91d94d136157c33f46d53d95439387ff3a..3bbb00fac99f17618b0961f039e83eeeb4e75423 100644
--- a/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
+++ b/chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.cc
@@ -1,9 +1,10 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2014 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.
 
 #include "chrome/browser/ui/views/extensions/browser_action_overflow_menu_controller.h"
 
+#include "base/memory/scoped_vector.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/extensions/extension_action.h"
@@ -11,15 +12,20 @@
 #include "chrome/browser/extensions/extension_toolbar_model.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/views/extensions/browser_action_drag_data.h"
 #include "chrome/browser/ui/views/toolbar/browser_action_view.h"
 #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
+#include "ui/views/border.h"
+#include "ui/views/controls/button/label_button_border.h"
+#include "ui/views/controls/menu/menu_delegate.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_runner.h"
+#include "ui/views/metrics.h"
+
+namespace {
 
 // In the browser actions container's chevron menu, a menu item view's icon
 // comes from BrowserActionView::GetIconWithBadge() when the menu item view is
@@ -40,7 +46,7 @@ class IconUpdater : public BrowserActionView::IconObserver {
     view_->set_icon_observer(NULL);
   }
 
-  // Overridden from BrowserActionView::IconObserver:
+  // BrowserActionView::IconObserver:
   virtual void OnIconUpdated(const gfx::ImageSkia& icon) OVERRIDE {
     menu_item_view_->SetIcon(icon);
   }
@@ -56,20 +62,93 @@ class IconUpdater : public BrowserActionView::IconObserver {
   DISALLOW_COPY_AND_ASSIGN(IconUpdater);
 };
 
-BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
-    BrowserActionsContainer* owner,
-    Browser* browser,
-    views::MenuButton* menu_button,
-    const std::vector<BrowserActionView*>& views,
-    int start_index,
+}  // namespace
+
+// This class handles the overflow menu for browser actions (showing the menu,
+// drag and drop, etc). This class manages its own lifetime.
+class ChevronMenuButton::MenuController : public views::MenuDelegate {
+ public:
+  MenuController(ChevronMenuButton* owner,
+                 BrowserActionsContainer* browser_actions_container,
+                 bool for_drop);
+  virtual ~MenuController();
+
+  // Shows the overflow menu.
+  void RunMenu(views::Widget* widget);
+
+  // Closes the overflow menu (and its context menu if open as well).
+  void CloseMenu();
+
+ private:
+  // Overridden from views::MenuDelegate:

[Peter Kasting, 2014/09/25 01:06:43]

Nit: "Overridden from" not necessary

[Devlin, 2014/09/25 15:56:49]

Dang, caught it on line 49 but not here.  Removed.

+  virtual bool IsCommandEnabled(int id) const OVERRIDE;
+  virtual void ExecuteCommand(int id) OVERRIDE;
+  virtual bool ShowContextMenu(views::MenuItemView* source,
+                               int id,
+                               const gfx::Point& p,
+                               ui::MenuSourceType source_type) OVERRIDE;
+  virtual void DropMenuClosed(views::MenuItemView* menu) OVERRIDE;
+  // These drag functions offer support for dragging icons into the overflow
+  // menu.

[Peter Kasting, 2014/09/25 01:06:43]

So, MenuButton and MenuDelegate both have these drag-and-drop methods?  Is there
a way to just use one set?  It looks below as if we basically implement all
these in both classes by forwarding to BrowserActionDragData, it would be nice
to save on some boilerplate.

[Devlin, 2014/09/25 15:56:49]

Unfortunately, there really isn't a good way.  The methods on MenuButton are
inherited from View, whereas those on MenuDelegate aren't inherited, but are
called from MenuController when the menu is run.  And we really do need to
inherit both separately, since both the chevron button and the drop down menu
need to respond to drag/drop events.  We could potentially have a third class
(e.g. "BrowserActionDropController") that each of these classes own and call
into, but that only saves us on the implementation of these methods, not the
declaration.  Since the methods are already basically just forwarded to
BrowserActionDragData, that doesn't really help.

+  virtual bool GetDropFormats(
+      views::MenuItemView* menu,
+      int* formats,
+      std::set<ui::OSExchangeData::CustomFormat>* custom_formats) OVERRIDE;
+  virtual bool AreDropTypesRequired(views::MenuItemView* menu) OVERRIDE;
+  virtual bool CanDrop(views::MenuItemView* menu,
+                       const ui::OSExchangeData& data) OVERRIDE;
+  virtual int GetDropOperation(views::MenuItemView* item,
+                               const ui::DropTargetEvent& event,
+                               DropPosition* position) OVERRIDE;
+  virtual int OnPerformDrop(views::MenuItemView* menu,
+                            DropPosition position,
+                            const ui::DropTargetEvent& event) OVERRIDE;
+  // These three drag functions offer support for dragging icons out of the
+  // overflow menu.
+  virtual bool CanDrag(views::MenuItemView* menu) OVERRIDE;
+  virtual void WriteDragData(views::MenuItemView* sender,
+                             ui::OSExchangeData* data) OVERRIDE;
+  virtual int GetDragOperations(views::MenuItemView* sender) OVERRIDE;
+
+  // Returns the offset into |views_| for the given |id|.
+  size_t IndexForId(int id) const;
+
+  // The owning ChevronMenuButton.
+  ChevronMenuButton* owner_;
+
+  // A pointer to the browser action container.
+  BrowserActionsContainer* browser_actions_container_;
+
+  // The overflow menu for the menu button. Owned by |menu_runner_|.
+  views::MenuItemView* menu_;
+
+  // Resposible for running the menu.
+  scoped_ptr<views::MenuRunner> menu_runner_;
+
+  // The index into the BrowserActionView vector, indicating where to start
+  // picking browser actions to draw.
+  int start_index_;
+
+  // Whether this controller is being used for drop.
+  bool for_drop_;
+
+  // The vector keeps all icon updaters associated with menu item views in the
+  // controller. The icon updater will update the menu item view's icon when
+  // the browser action view's icon has been updated.
+  ScopedVector<IconUpdater> icon_updaters_;
+
+  DISALLOW_COPY_AND_ASSIGN(MenuController);
+};
+
+ChevronMenuButton::MenuController::MenuController(
+    ChevronMenuButton* owner,
+    BrowserActionsContainer* browser_actions_container,
     bool for_drop)
     : owner_(owner),
-      browser_(browser),
-      observer_(NULL),
-      menu_button_(menu_button),
+      browser_actions_container_(browser_actions_container),
       menu_(NULL),
-      views_(views),
-      start_index_(start_index),
+      start_index_(
+          browser_actions_container_->VisibleBrowserActionsAfterAnimation()),
       for_drop_(for_drop) {
   menu_ = new views::MenuItemView(this);
   menu_runner_.reset(new views::MenuRunner(
@@ -77,8 +156,10 @@ BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
   menu_->set_has_icons(true);
 
   size_t command_id = 1;  // Menu id 0 is reserved, start with 1.
-  for (size_t i = start_index; i < views_.size(); ++i) {
-    BrowserActionView* view = views_[i];
+  for (size_t i = start_index_;
+       i < browser_actions_container_->num_browser_actions(); ++i) {
+    BrowserActionView* view =
+        browser_actions_container_->GetBrowserActionViewAt(i);
     views::MenuItemView* menu_item = menu_->AppendMenuItemWithIcon(
         command_id,
         base::UTF8ToUTF16(view->extension()->name()),
@@ -96,71 +177,77 @@ BrowserActionOverflowMenuController::BrowserActionOverflowMenuController(
   }
 }
 
-BrowserActionOverflowMenuController::~BrowserActionOverflowMenuController() {
-  if (observer_)
-    observer_->NotifyMenuDeleted(this);
+ChevronMenuButton::MenuController::~MenuController() {
 }
 
-bool BrowserActionOverflowMenuController::RunMenu(views::Widget* window) {
-  gfx::Rect bounds = menu_button_->bounds();
+void ChevronMenuButton::MenuController::RunMenu(views::Widget* window) {
+  gfx::Rect bounds = owner_->bounds();
   gfx::Point screen_loc;
-  views::View::ConvertPointToScreen(menu_button_, &screen_loc);
+  views::View::ConvertPointToScreen(owner_, &screen_loc);
   bounds.set_x(screen_loc.x());
   bounds.set_y(screen_loc.y());
 
-  views::MenuAnchorPosition anchor = views::MENU_ANCHOR_TOPRIGHT;
-  // As we maintain our own lifetime we can safely ignore the result.
-  ignore_result(menu_runner_->RunMenuAt(
-      window, menu_button_, bounds, anchor, ui::MENU_SOURCE_NONE));
+  if (menu_runner_->RunMenuAt(window,
+                              owner_,
+                              bounds,
+                              views::MENU_ANCHOR_TOPRIGHT,
+                              ui::MENU_SOURCE_NONE) ==
+          views::MenuRunner::MENU_DELETED)
+    return;
+
   if (!for_drop_) {
     // Give the context menu (if any) a chance to execute the user-selected
     // command.
-    base::MessageLoop::current()->DeleteSoon(FROM_HERE, this);
+    base::MessageLoop::current()->PostTask(
+        FROM_HERE,
+        base::Bind(&ChevronMenuButton::MenuDone,
+                   owner_->weak_factory_.GetWeakPtr()));
   }
-  return true;
 }
 
-void BrowserActionOverflowMenuController::CancelMenu() {
+void ChevronMenuButton::MenuController::CloseMenu() {
   menu_->Cancel();
 }
 
-void BrowserActionOverflowMenuController::NotifyBrowserActionViewsDeleting() {
-  icon_updaters_.clear();
-}
-
-bool BrowserActionOverflowMenuController::IsCommandEnabled(int id) const {
-  BrowserActionView* view = views_[start_index_ + id - 1];
+bool ChevronMenuButton::MenuController::IsCommandEnabled(int id) const {
+  BrowserActionView* view =
+      browser_actions_container_->GetBrowserActionViewAt(start_index_ + id - 1);
   return view->IsEnabled(view->view_controller()->GetCurrentTabId());
 }
 
-void BrowserActionOverflowMenuController::ExecuteCommand(int id) {
-  views_[start_index_ + id - 1]->view_controller()->ExecuteActionByUser();
+void ChevronMenuButton::MenuController::ExecuteCommand(int id) {
+  browser_actions_container_->GetBrowserActionViewAt(start_index_ + id - 1)->
+      view_controller()->ExecuteActionByUser();
 }
 
-bool BrowserActionOverflowMenuController::ShowContextMenu(
+bool ChevronMenuButton::MenuController::ShowContextMenu(
     views::MenuItemView* source,
     int id,
     const gfx::Point& p,
     ui::MenuSourceType source_type) {
-  BrowserActionView* view = views_[start_index_ + id - 1];
+  BrowserActionView* view = browser_actions_container_->GetBrowserActionViewAt(
+      start_index_ + id - 1);
   if (!view->extension()->ShowConfigureContextMenus())
     return false;
 
   scoped_refptr<ExtensionContextMenuModel> context_menu_contents =
-      new ExtensionContextMenuModel(
-          view->extension(), browser_, view->view_controller());
+      new ExtensionContextMenuModel(view->extension(),
+                                    view->view_controller()->browser(),
+                                    view->view_controller());
   views::MenuRunner context_menu_runner(context_menu_contents.get(),
                                         views::MenuRunner::HAS_MNEMONICS |
                                             views::MenuRunner::IS_NESTED |
                                             views::MenuRunner::CONTEXT_MENU);
 
   // We can ignore the result as we delete ourself.
-  // This blocks until the user choses something or dismisses the menu.
-  ignore_result(context_menu_runner.RunMenuAt(menu_button_->GetWidget(),
-                                              NULL,
-                                              gfx::Rect(p, gfx::Size()),
-                                              views::MENU_ANCHOR_TOPLEFT,
-                                              source_type));
+  // This blocks until the user chooses something or dismisses the menu.
+  if (context_menu_runner.RunMenuAt(owner_->GetWidget(),
+                                    NULL,
+                                    gfx::Rect(p, gfx::Size()),
+                                    views::MENU_ANCHOR_TOPLEFT,
+                                    source_type) ==
+          views::MenuRunner::MENU_DELETED)
+    return true;
 
   // The user is done with the context menu, so we can close the underlying
   // menu.
@@ -169,29 +256,30 @@ bool BrowserActionOverflowMenuController::ShowContextMenu(
   return true;
 }
 
-void BrowserActionOverflowMenuController::DropMenuClosed(
+void ChevronMenuButton::MenuController::DropMenuClosed(
     views::MenuItemView* menu) {
-  delete this;
+  owner_->MenuDone();
 }
 
-bool BrowserActionOverflowMenuController::GetDropFormats(
+bool ChevronMenuButton::MenuController::GetDropFormats(
     views::MenuItemView* menu,
     int* formats,
     std::set<OSExchangeData::CustomFormat>* custom_formats) {
   return BrowserActionDragData::GetDropFormats(custom_formats);
 }
 
-bool BrowserActionOverflowMenuController::AreDropTypesRequired(
+bool ChevronMenuButton::MenuController::AreDropTypesRequired(
     views::MenuItemView* menu) {
   return BrowserActionDragData::AreDropTypesRequired();
 }
 
-bool BrowserActionOverflowMenuController::CanDrop(
+bool ChevronMenuButton::MenuController::CanDrop(
     views::MenuItemView* menu, const OSExchangeData& data) {
-  return BrowserActionDragData::CanDrop(data, owner_->profile());
+  return BrowserActionDragData::CanDrop(data,
+                                        browser_actions_container_->profile());
 }
 
-int BrowserActionOverflowMenuController::GetDropOperation(
+int ChevronMenuButton::MenuController::GetDropOperation(
     views::MenuItemView* item,
     const ui::DropTargetEvent& event,
     DropPosition* position) {
@@ -204,14 +292,14 @@ int BrowserActionOverflowMenuController::GetDropOperation(
     if (!drop_data.Read(event.data()))
       return ui::DragDropTypes::DRAG_NONE;
 
-    if (drop_data.index() < owner_->VisibleBrowserActions())
+    if (drop_data.index() < browser_actions_container_->VisibleBrowserActions())
       return ui::DragDropTypes::DRAG_NONE;
   }
 
   return ui::DragDropTypes::DRAG_MOVE;
 }
 
-int BrowserActionOverflowMenuController::OnPerformDrop(
+int ChevronMenuButton::MenuController::OnPerformDrop(
     views::MenuItemView* menu,
     DropPosition position,
     const ui::DropTargetEvent& event) {
@@ -224,49 +312,130 @@ int BrowserActionOverflowMenuController::OnPerformDrop(
   // When not dragging within the overflow menu (dragging an icon into the menu)
   // subtract one to get the right index.
   if (position == DROP_BEFORE &&
-      drop_data.index() < owner_->VisibleBrowserActions())
+      drop_data.index() < browser_actions_container_->VisibleBrowserActions())
     --drop_index;
 
+  Profile* profile = browser_actions_container_->profile();
   // Move the extension in the model.
   const extensions::Extension* extension =
-      extensions::ExtensionRegistry::Get(browser_->profile())->
+      extensions::ExtensionRegistry::Get(profile)->
           enabled_extensions().GetByID(drop_data.id());
   extensions::ExtensionToolbarModel* toolbar_model =
-      extensions::ExtensionToolbarModel::Get(browser_->profile());
-  if (browser_->profile()->IsOffTheRecord())
+      extensions::ExtensionToolbarModel::Get(profile);
+  if (profile->IsOffTheRecord())
     drop_index = toolbar_model->IncognitoIndexToOriginal(drop_index);
   toolbar_model->MoveExtensionIcon(extension, drop_index);
 
   // If the extension was moved to the overflow menu from the main bar, notify
   // the owner.
-  if (drop_data.index() < owner_->VisibleBrowserActions())
-    owner_->NotifyActionMovedToOverflow();
+  if (drop_data.index() < browser_actions_container_->VisibleBrowserActions())
+    browser_actions_container_->NotifyActionMovedToOverflow();
 
   if (for_drop_)
-    delete this;
+    owner_->MenuDone();
   return ui::DragDropTypes::DRAG_MOVE;
 }
 
-bool BrowserActionOverflowMenuController::CanDrag(views::MenuItemView* menu) {
+bool ChevronMenuButton::MenuController::CanDrag(views::MenuItemView* menu) {
   return true;
 }
 
-void BrowserActionOverflowMenuController::WriteDragData(
+void ChevronMenuButton::MenuController::WriteDragData(
     views::MenuItemView* sender, OSExchangeData* data) {
   size_t drag_index = IndexForId(sender->GetCommand());
-  const extensions::Extension* extension = views_[drag_index]->extension();
+  const extensions::Extension* extension =
+      browser_actions_container_->GetBrowserActionViewAt(drag_index)->
+          extension();
   BrowserActionDragData drag_data(extension->id(), drag_index);
-  drag_data.Write(owner_->profile(), data);
+  drag_data.Write(browser_actions_container_->profile(), data);
 }
 
-int BrowserActionOverflowMenuController::GetDragOperations(
+int ChevronMenuButton::MenuController::GetDragOperations(
     views::MenuItemView* sender) {
   return ui::DragDropTypes::DRAG_MOVE;
 }
 
-size_t BrowserActionOverflowMenuController::IndexForId(int id) const {
+size_t ChevronMenuButton::MenuController::IndexForId(int id) const {
   // The index of the view being dragged (GetCommand gives a 1-based index into
   // the overflow menu).
-  DCHECK_GT(owner_->VisibleBrowserActions() + id, 0u);
-  return owner_->VisibleBrowserActions() + id - 1;
+  DCHECK_GT(browser_actions_container_->VisibleBrowserActions() + id, 0u);
+  return browser_actions_container_->VisibleBrowserActions() + id - 1;
+}
+
+ChevronMenuButton::ChevronMenuButton(
+    BrowserActionsContainer* browser_actions_container)
+    : views::MenuButton(NULL, base::string16(), this, false),
+      browser_actions_container_(browser_actions_container),
+      weak_factory_(this) {
+}
+
+ChevronMenuButton::~ChevronMenuButton() {
+}
+
+void ChevronMenuButton::CloseMenu() {
+  if (menu_controller_.get())
+    menu_controller_->CloseMenu();
+}
+
+scoped_ptr<views::LabelButtonBorder> ChevronMenuButton::CreateDefaultBorder()
+    const {
+  // The chevron resource was designed to not have any insets.
+  scoped_ptr<views::LabelButtonBorder> border =
+      views::MenuButton::CreateDefaultBorder();
+  border->set_insets(gfx::Insets());
+  return border.Pass();
+}
+
+bool ChevronMenuButton::GetDropFormats(
+    int* formats,
+    std::set<OSExchangeData::CustomFormat>* custom_formats) {
+  return BrowserActionDragData::GetDropFormats(custom_formats);
+}
+
+bool ChevronMenuButton::AreDropTypesRequired() {
+  return BrowserActionDragData::AreDropTypesRequired();
+}
+
+bool ChevronMenuButton::CanDrop(const OSExchangeData& data) {
+  return BrowserActionDragData::CanDrop(
+      data, browser_actions_container_->profile());
+}
+
+void ChevronMenuButton::OnDragEntered(const ui::DropTargetEvent& event) {
+  DCHECK(!weak_factory_.HasWeakPtrs());
+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&ChevronMenuButton::ShowOverflowMenu,
+                 weak_factory_.GetWeakPtr(),
+                 true),
+      base::TimeDelta::FromMilliseconds(views::GetMenuShowDelay()));
+}
+
+int ChevronMenuButton::OnDragUpdated(const ui::DropTargetEvent& event) {
+  return ui::DragDropTypes::DRAG_MOVE;
+}
+
+void ChevronMenuButton::OnDragExited() {
+  weak_factory_.InvalidateWeakPtrs();
+}
+
+int ChevronMenuButton::OnPerformDrop(const ui::DropTargetEvent& event) {
+  return ui::DragDropTypes::DRAG_MOVE;
+}
+
+void ChevronMenuButton::OnMenuButtonClicked(views::View* source,
+                                            const gfx::Point& point) {
+  DCHECK_EQ(this, source);
+  ShowOverflowMenu(false);
+}
+
+void ChevronMenuButton::ShowOverflowMenu(bool for_drop) {
+  DCHECK(!menu_controller_.get());
+  menu_controller_.reset(new MenuController(
+      this, browser_actions_container_, for_drop));
+  menu_controller_->RunMenu(GetWidget());
+}
+
+void ChevronMenuButton::MenuDone() {
+  menu_controller_.reset();
 }