Use the wrench menu as a reference view for overflowed extension actions' popups

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

Index: chrome/browser/ui/views/extensions/extension_action_view_controller.cc
diff --git a/chrome/browser/ui/views/extensions/extension_action_view_controller.cc b/chrome/browser/ui/views/extensions/extension_action_view_controller.cc
index 9256bd0b36e9279b8869b19bab76eacc64f30f97..23305920cb86ca636f18f9f6d3b464f8eaa14703 100644
--- a/chrome/browser/ui/views/extensions/extension_action_view_controller.cc
+++ b/chrome/browser/ui/views/extensions/extension_action_view_controller.cc
@@ -244,6 +244,11 @@ void ExtensionActionViewController::DoShowContextMenu(
 
 bool ExtensionActionViewController::ShowPopupWithUrl(
     ExtensionPopup::ShowAction show_action, const GURL& popup_url) {
+  ExtensionActionViewController* preferred_controller =
+      delegate_->GetPreferredPopupViewController();
+  if (preferred_controller != this)

[Peter Kasting, 2014/09/11 20:23:12]

Can we just make callers call this method on the correct controller instead of
allowing them to call it on the wrong one and then fixing that up here?

[Devlin, 2014/09/11 20:53:42]

I think that's actually more complicated overall.  This is called from
ExecuteAction, which is, in turn, called from several different call sites
(including mouse clicks, api calls, keyboard commands, etc).  For most of those,
there's no reason to know to direct the call to a "non-overflowed" vs.
"overflowed" extension action, and for some (like mouse clicks), it's extremely
painful (we'd have to forward left-clicks to one controller, right clicks stay
in here, etc).  I'd prefer to only forward the call when we need to - which is
in this case.

[Peter Kasting, 2014/09/11 20:57:37]

I'm only suggesting hoisting this to ExecuteAction().  Basically, that function
would call "delegate_->GetPreferredPopupViewController()->ShowPopupWithUrl()"
instead of "ShowPopupWithUrl()".  Isn't that all you need?

[Devlin, 2014/09/11 21:04:07]

Ah, misunderstood.  Yes, that's fine - done.  (Only had it this way to keep all
the popup-related logic confined to one method.)

+    return preferred_controller->ShowPopupWithUrl(show_action, popup_url);
+
   // If we're already showing the popup for this browser action, just hide it
   // and return.
   bool already_showing = popup_ != NULL;