MacViews: Remove GetMenu() from MenuRunner.

MacViews: Remove GetMenu() from MenuRunner.

With https://codereview.chromium.org/331993009/, GetMenu() may return
NULL if running a native Cocoa menu.
This patch deletes GetMenu and updates call sites to no longer call it.

BUG=388455
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=283640

[sky, 2014-07-15 23:53:00]

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
File
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc
(right):

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc:63:
menu_view_ = menu_adapter_->CreateMenu();
I thought RenderViewContextMenuView was one of the places we wanted to use
system menus?

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
File
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h
(right):

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h:66:
scoped_ptr<views::MenuModelAdapter> menu_adapter_;
I think this should be before menu_runner_.

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h:69:
views::MenuItemView* menu_view_;
Initialize to NULL.

https://codereview.chromium.org/393943006/diff/100001/ui/views/controls/combo...
File ui/views/controls/combobox/combobox.h (right):

https://codereview.chromium.org/393943006/diff/100001/ui/views/controls/combo...
ui/views/controls/combobox/combobox.h:193: MenuItemView* menu_;
Initialize to NULL.

[Andre, 2014-07-16 00:25:17]

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
File
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc
(right):

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc:63:
menu_view_ = menu_adapter_->CreateMenu();
On 2014/07/15 23:53:00, sky wrote:
> I thought RenderViewContextMenuView was one of the places we wanted to use
> system menus?

This is true, but I'm not sure how to convert
RenderViewContextMenuViews::UpdateMenuItem
to equivalent MenuModel based code.
Additionally, RenderViewContextMenuMac has additional items we will want such as
dictionary
lookup and text-to-speech.
It seems like what we want is to keep using RenderViewContextMenuMac even when
toolkit_views=1.

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
File
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h
(right):

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h:66:
scoped_ptr<views::MenuModelAdapter> menu_adapter_;
On 2014/07/15 23:53:00, sky wrote:
> I think this should be before menu_runner_.

Done.

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.h:69:
views::MenuItemView* menu_view_;
On 2014/07/15 23:53:00, sky wrote:
> Initialize to NULL.

Done.

https://codereview.chromium.org/393943006/diff/100001/ui/views/controls/combo...
File ui/views/controls/combobox/combobox.h (right):

https://codereview.chromium.org/393943006/diff/100001/ui/views/controls/combo...
ui/views/controls/combobox/combobox.h:193: MenuItemView* menu_;
On 2014/07/15 23:53:00, sky wrote:
> Initialize to NULL.

Done.

[sky, 2014-07-16 14:44:37]

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
File
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc
(right):

https://codereview.chromium.org/393943006/diff/100001/chrome/browser/ui/views...
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc:63:
menu_view_ = menu_adapter_->CreateMenu();
On 2014/07/16 00:25:16, Andre wrote:
> On 2014/07/15 23:53:00, sky wrote:
> > I thought RenderViewContextMenuView was one of the places we wanted to use
> > system menus?
> 
> This is true, but I'm not sure how to convert
> RenderViewContextMenuViews::UpdateMenuItem
> to equivalent MenuModel based code.
> Additionally, RenderViewContextMenuMac has additional items we will want such
as
> dictionary
> lookup and text-to-speech.
> It seems like what we want is to keep using RenderViewContextMenuMac even when
> toolkit_views=1.

This means we're done to the omnibox and textfield needing special handling. I'm
starting to think we should keep the ifdefs in those two places. I say that is
this is a large change that complicates things and doesn't address some other
places that are getting the menu (such as bookmark bar and making some tests).
WDYT?

[sky, 2014-07-16 22:57:55]

LGTM - I think you're going to have to merge the two patchsets, else things will
break.

[Andre, 2014-07-16 23:18:29]

On 2014/07/16 22:57:55, sky wrote:
> LGTM - I think you're going to have to merge the two patchsets, else things
will
> break.

I think the other patch depends on this one, but not the other way around.
Unless I'm missing something?

[Andre, 2014-07-17 00:50:11]

The CQ bit was checked by andresantoso@chromium.org

[commit-bot: I haz the power, 2014-07-17 00:54:08]

CQ is trying da patch. Follow status at

https://chromium-status.appspot.com/cq/andresantoso@chromium.org/393943006/18...

[commit-bot: I haz the power, 2014-07-17 04:22:04]

Change committed as 283640