[Extension Toolbar] Slide out overflowed actions for popups

[Extension Toolbar] Slide out overflowed actions for popups

Instead of using the wrench menu as an anchor for an overflowed
action's popup, slide the action out onto the main toolbar
for the duration of the popup, and then slide it back in
when the popup closes.
On Mac, do the same for context menus (since we can't open a
menu-in-menu on mac).

Known issue:
- On Mac, if you right click on an action that has slid out
to show its popup, it will slide back in (but the context
menu still appears). This is fixable, but requires a bit
of a refactor.

BUG=479387
Committed: https://crrev.com/fb6edb44b47dfcb661608f4e33d776480c34bdac
Cr-Commit-Position: refs/heads/master@{#327330}

[Devlin, 2015-04-24 23:46:52]

Patchset #1 (id:1) has been deleted

[Devlin, 2015-04-24 23:46:58]

Patchset #1 (id:20001) has been deleted

[Devlin, 2015-04-24 23:47:06]

Patchset #1 (id:40001) has been deleted

[Devlin, 2015-04-24 23:47:12]

Patchset #1 (id:60001) has been deleted

[Devlin, 2015-04-24 23:47:18]

Patchset #1 (id:80001) has been deleted

[Devlin, 2015-04-24 23:53:20]

Patchset #1 (id:100001) has been deleted

[Devlin, 2015-04-25 00:00:31]

Patchset #1 (id:120001) has been deleted

[Devlin, 2015-04-25 00:04:12]

rdevlin.cronin@chromium.org changed reviewers:
+ finnur@chromium.org

[Devlin, 2015-04-25 00:04:12]

And, here's the fun one. :)

There are a few rough edges (but mostly just in Cocoa), but would appreciate a
quick review to make sure I'm not doing anything terribly silly.

[Finnur, 2015-04-27 14:39:00]

> would appreciate a quick review to make sure I'm 
> not doing anything terribly silly.

I don't see anything that immediately raises a red flag on the non-Mac side (the
Mac code I mostly glanced at).

Just a few nits, and apart from that: LGTM.

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/toolbar_actions_bar.h (right):

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/toolbar_actions_bar.h:139: void PopInAction();
nit: In context (next to PopOut) this naming makes sense. But on its own, it
feels a bit weird. The PopOut concept is easy to explain -- I think of a little
popcorn warming up on a pan and once it reaches enough temperature to burst, it
pops and jumps out. I don't know how something *pops back into* the pan. But I
don't know if this is worth changing. And maybe this is common terminology, just
unfamiliar to me. It did get me wondering: is UndoPopOut a viable alternative?

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/toolbar_actions_bar.h:276: base::Closure
popped_out_closure_;
nit: Document.

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
File
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc
(right):

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc:173:
LOG(WARNING) << "Show context menu for view";
Accidentally included?

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc:192:
LOG(WARNING) << "Do Show";
Accidentally included?

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc:232:
LOG(WARNING) << "Ran";
Accidentally included? ... or just checking to see if I was paying attention? :)

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/toolbar/browser_actions_container.cc (left):

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/toolbar/browser_actions_container.cc:210:
view_controller->GetActionName();
Heh... :)

[Devlin, 2015-04-27 23:58:29]

Patchset #2 (id:160001) has been deleted

[Devlin, 2015-04-27 23:58:35]

Patchset #2 (id:180001) has been deleted

[Devlin, 2015-04-27 23:58:41]

Patchset #2 (id:200001) has been deleted

[Devlin, 2015-04-27 23:58:46]

Patchset #2 (id:220001) has been deleted

[Devlin, 2015-04-28 00:35:57]

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/tool...
File chrome/browser/ui/toolbar/toolbar_actions_bar.h (right):

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/toolbar_actions_bar.h:139: void PopInAction();
On 2015/04/27 14:39:00, Finnur wrote:
> nit: In context (next to PopOut) this naming makes sense. But on its own, it
> feels a bit weird. The PopOut concept is easy to explain -- I think of a
little
> popcorn warming up on a pan and once it reaches enough temperature to burst,
it
> pops and jumps out. I don't know how something *pops back into* the pan. But I
> don't know if this is worth changing. And maybe this is common terminology,
just
> unfamiliar to me. It did get me wondering: is UndoPopOut a viable alternative?

I like UndoPopOut, done.

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/tool...
chrome/browser/ui/toolbar/toolbar_actions_bar.h:276: base::Closure
popped_out_closure_;
On 2015/04/27 14:39:00, Finnur wrote:
> nit: Document.

Done.

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
File
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc
(right):

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc:173:
LOG(WARNING) << "Show context menu for view";
On 2015/04/27 14:39:00, Finnur wrote:
> Accidentally included?

Done.

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc:192:
LOG(WARNING) << "Do Show";
On 2015/04/27 14:39:00, Finnur wrote:
> Accidentally included?

Done.

https://codereview.chromium.org/1105713002/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.cc:232:
LOG(WARNING) << "Ran";
On 2015/04/27 14:39:00, Finnur wrote:
> Accidentally included? ... or just checking to see if I was paying attention?
:)

Nope, just those "rough edges" I mentioned. ;)

[Devlin, 2015-04-28 00:36:09]

rdevlin.cronin@chromium.org changed reviewers:
+ avi@chromium.org

[Devlin, 2015-04-28 00:36:09]

+Avi for the cocoa.

[Avi (use Gerrit), 2015-04-28 15:47:16]

cocoa lgtm

[Devlin, 2015-04-28 16:08:17]

rdevlin.cronin@chromium.org changed reviewers:
+ sky@chromium.org

[Devlin, 2015-04-28 16:08:17]

+Scott for toolbar_view (one new method).

[sky, 2015-04-28 16:56:07]

LGTM

[Devlin, 2015-04-28 17:45:14]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2015-04-28 17:45:15]

The patchset sent to the CQ was uploaded after l-g-t-m from finnur@chromium.org,
avi@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1105713002/#ps260001
(title: "Latest master")

[commit-bot: I haz the power, 2015-04-28 17:46:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1105713002/260001

[commit-bot: I haz the power, 2015-04-28 18:23:26]

Committed patchset #3 (id:260001)

[commit-bot: I haz the power, 2015-04-28 18:24:23]

Patchset 3 (id:??) landed as
https://crrev.com/fb6edb44b47dfcb661608f4e33d776480c34bdac
Cr-Commit-Position: refs/heads/master@{#327330}