Make the chevron menu button responsible for legacy overflow menu logic

chrome/browser/ui/views/toolbar/browser_actions_container.cc

Index: chrome/browser/ui/views/toolbar/browser_actions_container.cc
diff --git a/chrome/browser/ui/views/toolbar/browser_actions_container.cc b/chrome/browser/ui/views/toolbar/browser_actions_container.cc
index e96c18da5637c1158769fe7d7ee6f38f133993a7..bfaaee25e66c95eec0d9f6bfe5be5f48a5ae4684 100644
--- a/chrome/browser/ui/views/toolbar/browser_actions_container.cc
+++ b/chrome/browser/ui/views/toolbar/browser_actions_container.cc
@@ -52,35 +52,6 @@ namespace {
 // Horizontal spacing before the chevron (if visible).
 const int kChevronSpacing = ToolbarView::kStandardSpacing - 2;
 
-// A version of MenuButton with almost empty insets to fit properly on the
-// toolbar.
-class ChevronMenuButton : public views::MenuButton {
- public:
-  ChevronMenuButton(views::ButtonListener* listener,
-                    const base::string16& text,
-                    views::MenuButtonListener* menu_button_listener,
-                    bool show_menu_marker)
-      : views::MenuButton(listener,
-                          text,
-                          menu_button_listener,
-                          show_menu_marker) {
-  }
-
-  virtual ~ChevronMenuButton() {}
-
-  virtual scoped_ptr<views::LabelButtonBorder> CreateDefaultBorder() const
-      OVERRIDE {
-    // 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();
-  }
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(ChevronMenuButton);
-};
-
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -126,12 +97,9 @@ BrowserActionsContainer::BrowserActionsContainer(
       model_(NULL),
       container_width_(0),
       resize_area_(NULL),
-      chevron_(NULL),
-      overflow_menu_(NULL),
       suppress_chevron_(false),
       resize_amount_(0),
-      animation_target_size_(0),
-      show_menu_task_factory_(this) {
+      animation_target_size_(0) {
   set_id(VIEW_ID_BROWSER_ACTION_TOOLBAR);
 
   model_ = extensions::ExtensionToolbarModel::Get(browser->profile());
@@ -156,12 +124,15 @@ BrowserActionsContainer::BrowserActionsContainer(
     // 'Main' mode doesn't need a chevron overflow when overflow is shown inside
     // the Chrome menu.
     if (!overflow_experiment) {
-      chevron_ = new ChevronMenuButton(NULL, base::string16(), this, false);
+      // Since the ChevronMenuButton holds a raw pointer to us, we need to
+      // ensure it doesn't outlive us.  Having it owned by the view hierarchy as
+      // a child will suffice.
+      chevron_.reset(new ChevronMenuButton(this));

[Peter Kasting, 2014/09/25 20:42:41]

This needs to not be a scoped_ptr now, since otherwise you have
double-ownership.

[Devlin, 2014/09/25 20:47:12]

Done.

       chevron_->EnableCanvasFlippingForRTLUI(true);
       chevron_->SetAccessibleName(
           l10n_util::GetStringUTF16(IDS_ACCNAME_EXTENSIONS_CHEVRON));
       chevron_->SetVisible(false);
-      AddChildView(chevron_);
+      AddChildView(chevron_.get());
     }
   }
 }
@@ -171,11 +142,8 @@ BrowserActionsContainer::~BrowserActionsContainer() {
                     observers_,
                     OnBrowserActionsContainerDestroyed());
 
-  if (overflow_menu_)
-    overflow_menu_->set_observer(NULL);
   if (model_)
     model_->RemoveObserver(this);
-  StopShowFolderDropMenuTimer();
   HideActivePopup();
   DeleteBrowserActionViews();
 }
@@ -233,8 +201,6 @@ void BrowserActionsContainer::CreateBrowserActionViews() {
 
 void BrowserActionsContainer::DeleteBrowserActionViews() {
   HideActivePopup();
-  if (overflow_menu_)
-    overflow_menu_->NotifyBrowserActionViewsDeleting();
   STLDeleteElements(&browser_action_views_);
 }
 
@@ -295,7 +261,7 @@ views::MenuButton* BrowserActionsContainer::GetOverflowReferenceView() {
   // With traditional overflow, the reference is the chevron. With the
   // redesign, we use the wrench menu instead.
   return chevron_ ?
-      chevron_ :
+      chevron_.get() :
       BrowserView::GetBrowserViewForBrowser(browser_)->toolbar()->app_menu();
 }
 
@@ -382,7 +348,7 @@ void BrowserActionsContainer::Layout() {
 
   // If the icons don't all fit, show the chevron (unless suppressed).
   int max_x = GetPreferredSize().width();
-  if ((IconCountToWidth(-1, false) > max_x) && !suppress_chevron_ && chevron_) {
+  if (IconCountToWidth(-1, false) > max_x && !suppress_chevron_ && chevron_) {
     chevron_->SetVisible(true);
     gfx::Size chevron_size(chevron_->GetPreferredSize());
     max_x -=
@@ -447,14 +413,6 @@ bool BrowserActionsContainer::CanDrop(const OSExchangeData& data) {
 
 int BrowserActionsContainer::OnDragUpdated(
     const ui::DropTargetEvent& event) {
-  // First check if we are above the chevron (overflow) menu.
-  if (chevron_ && GetEventHandlerForPoint(event.location()) == chevron_) {
-    if (!show_menu_task_factory_.HasWeakPtrs() && !overflow_menu_)
-      StartShowFolderDropMenuTimer();
-    return ui::DragDropTypes::DRAG_MOVE;
-  }
-  StopShowFolderDropMenuTimer();
-
   size_t row_index = 0;
   size_t before_icon_in_row = 0;
   // If there are no visible browser actions (such as when dragging an icon to
@@ -534,7 +492,6 @@ int BrowserActionsContainer::OnDragUpdated(
 }
 
 void BrowserActionsContainer::OnDragExited() {
-  StopShowFolderDropMenuTimer();
   drop_position_.reset();
   SchedulePaint();
 }
@@ -596,21 +553,6 @@ void BrowserActionsContainer::GetAccessibleState(
   state->name = l10n_util::GetStringUTF16(IDS_ACCNAME_EXTENSIONS);
 }
 
-void BrowserActionsContainer::OnMenuButtonClicked(views::View* source,
-                                                  const gfx::Point& point) {
-  if (source == chevron_) {
-    overflow_menu_ =
-        new BrowserActionOverflowMenuController(this,
-                                                browser_,
-                                                chevron_,
-                                                browser_action_views_,
-                                                VisibleBrowserActions(),
-                                                false);
-    overflow_menu_->set_observer(this);
-    overflow_menu_->RunMenu(GetWidget());
-  }
-}
-
 void BrowserActionsContainer::WriteDragDataForView(View* sender,
                                                    const gfx::Point& press_pt,
                                                    OSExchangeData* data) {
@@ -692,12 +634,6 @@ void BrowserActionsContainer::AnimationEnded(const gfx::Animation* animation) {
                     OnBrowserActionsContainerAnimationEnded());
 }
 
-void BrowserActionsContainer::NotifyMenuDeleted(
-    BrowserActionOverflowMenuController* controller) {
-  DCHECK_EQ(overflow_menu_, controller);
-  overflow_menu_ = NULL;
-}
-
 content::WebContents* BrowserActionsContainer::GetCurrentWebContents() {
   return browser_->tab_strip_model()->GetActiveWebContents();
 }
@@ -819,7 +755,8 @@ void BrowserActionsContainer::ToolbarExtensionAdded(const Extension* extension,
            "exists.";
   }
 #endif
-  CloseOverflowMenu();
+  if (chevron_)
+    chevron_->CloseMenu();
 
   if (!ShouldDisplayBrowserAction(extension))
     return;
@@ -866,7 +803,8 @@ void BrowserActionsContainer::ToolbarExtensionAdded(const Extension* extension,
 
 void BrowserActionsContainer::ToolbarExtensionRemoved(
     const Extension* extension) {
-  CloseOverflowMenu();
+  if (chevron_)
+    chevron_->CloseMenu();
 
   size_t visible_actions = VisibleBrowserActionsAfterAnimation();
   for (BrowserActionViews::iterator i(browser_action_views_.begin());
@@ -1008,36 +946,6 @@ void BrowserActionsContainer::SetChevronVisibility() {
   }
 }
 
-void BrowserActionsContainer::CloseOverflowMenu() {
-  if (overflow_menu_)
-    overflow_menu_->CancelMenu();
-}
-
-void BrowserActionsContainer::StopShowFolderDropMenuTimer() {
-  show_menu_task_factory_.InvalidateWeakPtrs();
-}
-
-void BrowserActionsContainer::StartShowFolderDropMenuTimer() {
-  base::MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&BrowserActionsContainer::ShowDropFolder,
-                 show_menu_task_factory_.GetWeakPtr()),
-      base::TimeDelta::FromMilliseconds(views::GetMenuShowDelay()));
-}
-
-void BrowserActionsContainer::ShowDropFolder() {
-  DCHECK(!overflow_menu_);
-  overflow_menu_ =
-      new BrowserActionOverflowMenuController(this,
-                                              browser_,
-                                              chevron_,
-                                              browser_action_views_,
-                                              VisibleBrowserActions(),
-                                              true);
-  overflow_menu_->set_observer(this);
-  overflow_menu_->RunMenu(GetWidget());
-}
-
 int BrowserActionsContainer::IconCountToWidth(int icons,
                                               bool display_chevron) const {
   if (icons < 0)