[UI] Resolve method conflict between MenuModel, SimpleMenuModel::Delegate

ui::MenuModel (which ui::SimpleMenuModel inherits from) declares a
virtual method, `void MenuWillShow()`. SimpleMenuModel::Delegate
declares a similar method, `void MenuWillShow(SimpleMenuModel*)`. This
results in a conflict for any class that inherits from both
SimpleMenuModel and SimpleMenuModel::Delegate (which is not uncommon).

Resolve this by renaming the SimpleMenuModel::Delegate version to
`void SimpleMenuWillShow(SimpleMenuModel*)`.

Bug: 885198
Change-Id: I0581991fa7a12368007f6c082c461acb4833750c
Reviewed-on: https://chromium-review.googlesource.com/1230581Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592694}

chrome/browser/renderer_context_menu/mock_render_view_context_menu.cc

@@ -47,10 +47,6 @@ void MockRenderViewContextMenu::ExecuteCommand(int command_id,
   observer_->ExecuteCommand(command_id);
 }
 
-void MockRenderViewContextMenu::MenuWillShow(ui::SimpleMenuModel* source) {}
-
-void MockRenderViewContextMenu::MenuClosed(ui::SimpleMenuModel* source) {}
-
 void MockRenderViewContextMenu::AddMenuItem(int command_id,
                                             const base::string16& title) {
   MockMenuItem item;

chrome/browser/renderer_context_menu/mock_render_view_context_menu.h

@@ -49,8 +49,6 @@ class MockRenderViewContextMenu : public ui::SimpleMenuModel::Delegate,
   bool IsCommandIdChecked(int command_id) const override;
   bool IsCommandIdEnabled(int command_id) const override;
   void ExecuteCommand(int command_id, int event_flags) override;
-  void MenuWillShow(ui::SimpleMenuModel* source) override;
-  void MenuClosed(ui::SimpleMenuModel* source) override;
 
   // RenderViewContextMenuProxy implementation.
   void AddMenuItem(int command_id, const base::string16& title) override;

chrome/browser/ui/toolbar/media_router_contextual_menu.cc

@@ -204,7 +204,7 @@ void MediaRouterContextualMenu::ExecuteCommand(int command_id,
   }
 }
 
-void MediaRouterContextualMenu::MenuWillShow(ui::SimpleMenuModel* source) {
+void MediaRouterContextualMenu::OnMenuWillShow(ui::SimpleMenuModel* source) {
   observer_->OnContextMenuShown();
 }
 

chrome/browser/ui/toolbar/media_router_contextual_menu.h

@@ -67,7 +67,7 @@ class MediaRouterContextualMenu : public ui::SimpleMenuModel::Delegate {
   bool IsCommandIdEnabled(int command_id) const override;
   bool IsCommandIdVisible(int command_id) const override;
   void ExecuteCommand(int command_id, int event_flags) override;
-  void MenuWillShow(ui::SimpleMenuModel* source) override;
+  void OnMenuWillShow(ui::SimpleMenuModel* source) override;
   void MenuClosed(ui::SimpleMenuModel* source) override;
 
   // Toggles the enabled/disabled state of cloud services. This may show a

chrome/browser/ui/toolbar/media_router_contextual_menu_unittest.cc

@@ -313,7 +313,7 @@ TEST_F(MediaRouterContextualMenuUnitTest, NotifyActionController) {
   EXPECT_CALL(observer_, OnContextMenuShown());
   auto menu = std::make_unique<MediaRouterContextualMenu>(
       browser(), kInToolbar, kShownByUser, &observer_);
-  menu->MenuWillShow(menu->menu_model());
+  menu->OnMenuWillShow(menu->menu_model());
 
   EXPECT_CALL(observer_, OnContextMenuHidden());
   menu->MenuClosed(menu->menu_model());

components/renderer_context_menu/render_view_context_menu_base.cc

@@ -358,7 +358,7 @@ void RenderViewContextMenuBase::ExecuteCommand(int id, int event_flags) {
   command_executed_ = false;
 }
 
-void RenderViewContextMenuBase::MenuWillShow(ui::SimpleMenuModel* source) {
+void RenderViewContextMenuBase::OnMenuWillShow(ui::SimpleMenuModel* source) {
   for (int i = 0; i < source->GetItemCount(); ++i) {
     if (source->IsVisibleAt(i) &&
         source->GetTypeAt(i) != ui::MenuModel::TYPE_SEPARATOR &&

components/renderer_context_menu/render_view_context_menu_base.h

@@ -90,7 +90,7 @@ class RenderViewContextMenuBase : public ui::SimpleMenuModel::Delegate,
   // SimpleMenuModel::Delegate implementation.
   bool IsCommandIdChecked(int command_id) const override;
   void ExecuteCommand(int command_id, int event_flags) override;
-  void MenuWillShow(ui::SimpleMenuModel* source) override;
+  void OnMenuWillShow(ui::SimpleMenuModel* source) override;
   void MenuClosed(ui::SimpleMenuModel* source) override;
 
   // RenderViewContextMenuProxy implementation.

ui/base/cocoa/menu_controller_unittest.mm

@@ -135,7 +135,7 @@ class Delegate : public SimpleMenuModel::Delegate {
     ++execute_count_;
   }
 
-  void MenuWillShow(SimpleMenuModel* /*source*/) override {
+  void OnMenuWillShow(SimpleMenuModel* /*source*/) override {
     EXPECT_FALSE(did_show_);
     EXPECT_FALSE(did_close_);
     did_show_ = true;
@@ -159,7 +159,7 @@ class Delegate : public SimpleMenuModel::Delegate {
   int execute_count_ = 0;
   mutable int enable_count_ = 0;
   // The menu on which to call |-cancelTracking| after a short delay in
-  // MenuWillShow.
+  // OnMenuWillShow.
   NSMenu* menu_to_close_ = nil;
   bool did_show_ = false;
   bool did_close_ = false;
@@ -604,7 +604,7 @@ TEST_F(MenuControllerTest, OpenClose) {
   EXPECT_FALSE([menu isMenuOpen]);
 
   // When control returns back to here, the menu will have finished running its
-  // loop and will have closed itself (see Delegate::MenuWillShow).
+  // loop and will have closed itself (see Delegate::OnMenuWillShow).
   EXPECT_TRUE(delegate.did_show_);
 
   // When the menu tells the Model it closed, the Model posts a task to notify

ui/base/models/simple_menu_model.cc

@@ -53,8 +53,7 @@ bool SimpleMenuModel::Delegate::GetIconForCommandId(
 void SimpleMenuModel::Delegate::CommandIdHighlighted(int command_id) {
 }
 
-void SimpleMenuModel::Delegate::MenuWillShow(SimpleMenuModel* /*source*/) {
-}
+void SimpleMenuModel::Delegate::OnMenuWillShow(SimpleMenuModel* /*source*/) {}
 
 void SimpleMenuModel::Delegate::MenuClosed(SimpleMenuModel* /*source*/) {
 }
@@ -438,7 +437,7 @@ MenuModel* SimpleMenuModel::GetSubmenuModelAt(int index) const {
 
 void SimpleMenuModel::MenuWillShow() {
   if (delegate_)
-    delegate_->MenuWillShow(this);
+    delegate_->OnMenuWillShow(this);
 }
 
 void SimpleMenuModel::MenuWillClose() {

ui/base/models/simple_menu_model.h

@@ -56,7 +56,10 @@ class UI_BASE_EXPORT SimpleMenuModel : public MenuModel {
     virtual void ExecuteCommand(int command_id, int event_flags) = 0;
 
     // Notifies the delegate that the menu is about to show.
-    virtual void MenuWillShow(SimpleMenuModel* source);
+    // Slight hack: Prefix with "On" to make sure this doesn't conflict with
+    // MenuModel::MenuWillShow(), since many classes derive from both
+    // SimpleMenuModel and SimpleMenuModel::Delegate.
+    virtual void OnMenuWillShow(SimpleMenuModel* source);
 
     // Notifies the delegate that the menu has closed.
     virtual void MenuClosed(SimpleMenuModel* source);

ui/views/controls/menu/menu_runner_cocoa_unittest.mm

@@ -45,7 +45,7 @@ class TestModel : public ui::SimpleMenuModel {
     bool IsCommandIdEnabled(int command_id) const override { return true; }
     void ExecuteCommand(int command_id, int event_flags) override {}
 
-    void MenuWillShow(SimpleMenuModel* source) override {
+    void OnMenuWillShow(SimpleMenuModel* source) override {
       if (!model_->menu_open_callback_.is_null())
         std::move(model_->menu_open_callback_).Run();
     }