Extension Toolbar redesign, part 1 (overflow)

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

Index: chrome/browser/ui/views/toolbar/browser_action_view.cc
diff --git a/chrome/browser/ui/views/toolbar/browser_action_view.cc b/chrome/browser/ui/views/toolbar/browser_action_view.cc
index e277c3cd82b50b2bc4c6a84069a3181e0448c063..bc159145fc314eae2526a0ec42d82570f219dfa2 100644
--- a/chrome/browser/ui/views/toolbar/browser_action_view.cc
+++ b/chrome/browser/ui/views/toolbar/browser_action_view.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/themes/theme_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/view_ids.h"
+#include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/toolbar/browser_actions_container.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "extensions/common/extension.h"
@@ -61,7 +62,7 @@ gfx::ImageSkia BrowserActionView::GetIconWithBadge() {
 }
 
 void BrowserActionView::Layout() {
-  button_->SetBounds(0, y(), width(), height());
+  button_->SetBounds(0, 0, width(), height());
 }
 
 void BrowserActionView::GetAccessibleState(ui::AXViewState* state) {
@@ -175,20 +176,35 @@ void BrowserActionButton::ShowContextMenuForView(
   SetButtonPushed();
 
   // Reconstructs the menu every time because the menu's contents are dynamic.
-  scoped_refptr<ExtensionContextMenuModel> context_menu_contents_(
+  scoped_refptr<ExtensionContextMenuModel> context_menu_contents(
       new ExtensionContextMenuModel(extension(), browser_, delegate_));
-  menu_runner_.reset(new views::MenuRunner(context_menu_contents_.get()));
+  menu_runner_.reset(new views::MenuRunner(context_menu_contents.get()));
 
   context_menu_ = menu_runner_->GetMenu();
   gfx::Point screen_loc;
   views::View::ConvertPointToScreen(this, &screen_loc);
-  if (menu_runner_->RunMenuAt(
-          GetWidget(),
-          NULL,
-          gfx::Rect(screen_loc, size()),
-          views::MENU_ANCHOR_TOPLEFT,
-          source_type,
-          views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::CONTEXT_MENU) ==
+
+  bool nested = delegate_->ShownInsideMenu();
+  views::Widget* parent;

[Devlin, 2014/06/23 17:55:28]

nit: initialize to NULL.

+  int run_types = views::MenuRunner::HAS_MNEMONICS |
+                  views::MenuRunner::CONTEXT_MENU;
+  if (nested) {

[Devlin, 2014/06/23 17:55:28]

nit: since |nested| is only used here, and is short to define, just inline it in
the if clause.

+    run_types |= views::MenuRunner::IS_NESTED;
+    // RunMenuAt expects a nested menu to be parented by the same widget as the
+    // already visible menu, in this case the wrench menu (hamburger).

[Devlin, 2014/06/23 17:55:28]

I don't know if I've ever heard the wrench menu be called the hamburger menu
before (I know I've heard "wrench", "options", "stack", "application", "hotdog",
etc). Is hamburger a common name, too?  (I'm not really too worried, I think
everyone knows that the food-related menu is the options menu.)

[Finnur, 2014/06/24 17:00:49]

I've heard it called both and it looks more to me like hamburger than a hotdog
(although I see the resemblance either way). :)

+    parent = BrowserView::GetBrowserViewForBrowser(browser_)->toolbar()
+                                                            ->app_menu()
+                                                            ->GetWidget();
+  } else {
+    parent = GetWidget();
+  }
+
+  if (menu_runner_->RunMenuAt(parent,
+                              NULL,
+                              gfx::Rect(screen_loc, size()),
+                              views::MENU_ANCHOR_TOPLEFT,
+                              source_type,
+                              run_types) ==
       views::MenuRunner::MENU_DELETED) {
     return;
   }