Consolidate code to close overflow menu from ui/ layer.

Currently, the extension action, media router action and the toolbar action bar,
all call into their delegates to close the overflow menu. Consolidate this code
so that the extension action and media router action use
ToolbarActionBar::CloseOverflowMenuIfOpen.

BUG=None

Change-Id: I4c6933901a3e3e2ba52ad0c4b0cb48efb6cd5172
Reviewed-on: https://chromium-review.googlesource.com/1222685Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591131}

chrome/browser/ui/cocoa/extensions/browser_actions_controller.mm

@@ -178,7 +178,7 @@ class ToolbarActionsBarBridge : public ToolbarActionsBarDelegate {
   void StopAnimating() override;
   void ShowToolbarActionBubble(
       std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) override;
-  void CloseOverflowMenuIfOpen() override;
+  bool CloseOverflowMenuIfOpen() override;
 
   // The owning BrowserActionsController; weak.
   BrowserActionsController* controller_;
@@ -245,11 +245,14 @@ void ToolbarActionsBarBridge::ShowToolbarActionBubble(
   [controller_ createMessageBubble:std::move(bubble)];
 }
 
-void ToolbarActionsBarBridge::CloseOverflowMenuIfOpen() {
+bool ToolbarActionsBarBridge::CloseOverflowMenuIfOpen() {
   AppMenuController* appMenuController =
       [[controller_ toolbarController] appMenuController];
-  if ([appMenuController isMenuOpen])
-    [appMenuController cancel];
+  if (![appMenuController isMenuOpen])
+    return false;
+
+  [appMenuController cancel];
+  return true;
 }
 
 }  // namespace

chrome/browser/ui/cocoa/extensions/extension_action_platform_delegate_cocoa.h

@@ -29,7 +29,6 @@ class ExtensionActionPlatformDelegateCocoa
       std::unique_ptr<extensions::ExtensionViewHost> host,
       bool grant_tab_permissions,
       ExtensionActionViewController::PopupShowAction show_action) override;
-  void CloseOverflowMenu() override;
   void ShowContextMenu() override;
 
   // content::NotificationObserver:

chrome/browser/ui/cocoa/extensions/extension_action_platform_delegate_cocoa.mm

@@ -13,7 +13,6 @@
 #include "chrome/browser/extensions/extension_view_host.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
-#import "chrome/browser/ui/cocoa/app_menu/app_menu_controller.h"
 #include "chrome/browser/ui/cocoa/browser_dialogs_views_mac.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/extensions/browser_action_button.h"
@@ -101,18 +100,6 @@ void ExtensionActionPlatformDelegateCocoa::ShowPopup(
                                     viewsScreenPoint, popupShowAction);
 }
 
-void ExtensionActionPlatformDelegateCocoa::CloseOverflowMenu() {
-  // If this was triggered by an action overflowed to the app menu, then the app
-  // menu will be open. Close it before opening the popup.
-  AppMenuController* appMenuController =
-      [[[BrowserWindowController
-          browserWindowControllerForWindow:
-              controller_->browser()->window()->GetNativeWindow()]
-          toolbarController] appMenuController];
-  if ([appMenuController isMenuOpen])
-    [appMenuController cancel];
-}
-
 void ExtensionActionPlatformDelegateCocoa::ShowContextMenu() {
   // We should only use this code path for extensions shown in the toolbar.
   BrowserWindowController* windowController = [BrowserWindowController

chrome/browser/ui/cocoa/toolbar/media_router_action_platform_delegate_cocoa.h

@@ -15,9 +15,6 @@ class MediaRouterActionPlatformDelegateCocoa :
   explicit MediaRouterActionPlatformDelegateCocoa(Browser* browser);
   ~MediaRouterActionPlatformDelegateCocoa() override;
 
-  // MediaRouterActionPlatformDelegate:
-  bool CloseOverflowMenuIfOpen() override;
-
  private:
   // The corresponding browser.
   Browser* browser_;

chrome/browser/ui/cocoa/toolbar/media_router_action_platform_delegate_cocoa.mm

@@ -6,11 +6,6 @@
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_window.h"
-#import "chrome/browser/ui/cocoa/app_menu/app_menu_controller.h"
-#import "chrome/browser/ui/cocoa/browser_window_controller.h"
-#import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
 #include "ui/base/ui_features.h"
 
 // static
@@ -36,18 +31,3 @@ MediaRouterActionPlatformDelegateCocoa::MediaRouterActionPlatformDelegateCocoa(
 MediaRouterActionPlatformDelegateCocoa::
     ~MediaRouterActionPlatformDelegateCocoa() {
 }
-
-bool MediaRouterActionPlatformDelegateCocoa::CloseOverflowMenuIfOpen() {
-  // TODO(apacible): This should be factored to share code with extension
-  // actions.
-  AppMenuController* appMenuController =
-      [[[BrowserWindowController
-          browserWindowControllerForWindow:
-              browser_->window()->GetNativeWindow()]
-          toolbarController] appMenuController];
-  if (![appMenuController isMenuOpen])
-    return false;
-
-  [appMenuController cancel];
-  return true;
-}

chrome/browser/ui/extensions/extension_action_platform_delegate.h

@@ -42,9 +42,6 @@ class ExtensionActionPlatformDelegate {
       bool grant_tab_permissions,
       ExtensionActionViewController::PopupShowAction show_action) = 0;
 
-  // Closes the overflow menu, if it was open.
-  virtual void CloseOverflowMenu() = 0;
-
   // Shows the context menu for the extension.
   virtual void ShowContextMenu() = 0;
 };

chrome/browser/ui/extensions/extension_action_view_controller.cc

@@ -393,7 +393,7 @@ bool ExtensionActionViewController::TriggerPopupWithUrl(
 
   if (toolbar_actions_bar_ &&
       !toolbar_actions_bar_->IsActionVisibleOnMainBar(this)) {
-    platform_delegate_->CloseOverflowMenu();
+    toolbar_actions_bar_->CloseOverflowMenuIfOpen();
     toolbar_actions_bar_->PopOutAction(
         this,
         show_action == SHOW_POPUP_AND_INSPECT,

chrome/browser/ui/toolbar/media_router_action.cc

@@ -180,10 +180,13 @@ bool MediaRouterAction::ExecuteAction(bool by_user) {
 
   GetMediaRouterDialogController()->ShowMediaRouterDialog();
   if (GetPlatformDelegate()) {
+    // TODO(karandeepb): Instead of checking the return value of
+    // CloseOverflowMenuIfOpen, just check
+    // ToolbarActionsBar::IsActionVisibleOnMainBar.
     media_router::MediaRouterMetrics::RecordMediaRouterDialogOrigin(
-        GetPlatformDelegate()->CloseOverflowMenuIfOpen() ?
-        media_router::MediaRouterDialogOpenOrigin::OVERFLOW_MENU :
-        media_router::MediaRouterDialogOpenOrigin::TOOLBAR);
+        toolbar_actions_bar_->CloseOverflowMenuIfOpen()
+            ? media_router::MediaRouterDialogOpenOrigin::OVERFLOW_MENU
+            : media_router::MediaRouterDialogOpenOrigin::TOOLBAR);
   }
   return true;
 }

chrome/browser/ui/toolbar/media_router_action_platform_delegate.h

@@ -11,6 +11,7 @@
 
 class Browser;
 
+// TODO(karandeepb): Delete this class.
 class MediaRouterActionPlatformDelegate {
  public:
   MediaRouterActionPlatformDelegate() {}
@@ -26,10 +27,6 @@ class MediaRouterActionPlatformDelegate {
   static std::unique_ptr<MediaRouterActionPlatformDelegate> CreateCocoa(
       Browser* browser);
 #endif
-
-  // Closes the overflow menu, if it was open. Returns whether or not the
-  // overflow menu was closed.
-  virtual bool CloseOverflowMenuIfOpen() = 0;
 };
 
 #endif  // CHROME_BROWSER_UI_TOOLBAR_MEDIA_ROUTER_ACTION_PLATFORM_DELEGATE_H_

chrome/browser/ui/toolbar/toolbar_actions_bar.cc

@@ -568,6 +568,10 @@ void ToolbarActionsBar::ShowToolbarActionBubbleAsync(
                      weak_ptr_factory_.GetWeakPtr(), std::move(bubble)));
 }
 
+bool ToolbarActionsBar::CloseOverflowMenuIfOpen() {
+  return delegate_->CloseOverflowMenuIfOpen();
+}
+
 void ToolbarActionsBar::MaybeShowExtensionBubble() {
   std::unique_ptr<extensions::ExtensionMessageBubbleController> controller =
       model_->GetExtensionMessageBubbleController(browser_);

chrome/browser/ui/toolbar/toolbar_actions_bar.h

@@ -224,6 +224,10 @@ class ToolbarActionsBar : public ToolbarActionsModel::Observer,
   void ShowToolbarActionBubbleAsync(
       std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble);
 
+  // Closes the overflow menu, if it was open. Returns whether or not the
+  // overflow menu was closed.
+  bool CloseOverflowMenuIfOpen();
+
   // Returns the underlying toolbar actions, but does not order them. Primarily
   // for use in testing.
   const ToolbarActions& toolbar_actions_unordered() const {

chrome/browser/ui/toolbar/toolbar_actions_bar_delegate.h

@@ -59,8 +59,9 @@ class ToolbarActionsBarDelegate {
   virtual void ShowToolbarActionBubble(
       std::unique_ptr<ToolbarActionsBarBubbleDelegate> bubble) = 0;
 
-  // Closes the overflow menu if it's open.
-  virtual void CloseOverflowMenuIfOpen() = 0;
+  // Closes the overflow menu, if it was open. Returns whether or not the
+  // overflow menu was closed.
+  virtual bool CloseOverflowMenuIfOpen() = 0;
 };
 
 #endif  // CHROME_BROWSER_UI_TOOLBAR_TOOLBAR_ACTIONS_BAR_DELEGATE_H_

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

@@ -16,7 +16,6 @@
 #include "chrome/browser/ui/extensions/accelerator_priority.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/browser_app_menu_button.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_action_view_delegate_views.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/ui/views_mode_controller.h"
@@ -95,17 +94,6 @@ void ExtensionActionPlatformDelegateViews::ShowPopup(
                             popup_show_action);
 }
 
-void ExtensionActionPlatformDelegateViews::CloseOverflowMenu() {
-  // TODO(mgiuca): Use button_provider() instead of toolbar(), so this also
-  // works for hosted app windows.
-  AppMenuButton* app_menu_button =
-      BrowserView::GetBrowserViewForBrowser(controller_->browser())
-          ->toolbar()
-          ->app_menu_button();
-  if (app_menu_button && app_menu_button->IsMenuShowing())
-    app_menu_button->CloseMenu();
-}
-
 void ExtensionActionPlatformDelegateViews::ShowContextMenu() {
   views::View* view = GetDelegateViews()->GetAsView();
   view->context_menu_controller()->ShowContextMenuForView(

chrome/browser/ui/views/extensions/extension_action_platform_delegate_views.h

@@ -37,7 +37,6 @@ class ExtensionActionPlatformDelegateViews
       std::unique_ptr<extensions::ExtensionViewHost> host,
       bool grant_tab_permissions,
       ExtensionActionViewController::PopupShowAction show_action) override;
-  void CloseOverflowMenu() override;
   void ShowContextMenu() override;
 
   // content::NotificationObserver:

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

@@ -307,15 +307,18 @@ void BrowserActionsContainer::ShowToolbarActionBubble(
   bubble->Show();
 }
 
-void BrowserActionsContainer::CloseOverflowMenuIfOpen() {
+bool BrowserActionsContainer::CloseOverflowMenuIfOpen() {
   // TODO(mgiuca): Use toolbar_button_provider() instead of toolbar(), so this
   // also works for hosted app windows.
   BrowserAppMenuButton* app_menu_button =
       BrowserView::GetBrowserViewForBrowser(browser_)
           ->toolbar()
           ->app_menu_button();
-  if (app_menu_button && app_menu_button->IsMenuShowing())
-    app_menu_button->CloseMenu();
+  if (!app_menu_button || !app_menu_button->IsMenuShowing())
+    return false;
+
+  app_menu_button->CloseMenu();
+  return true;
 }
 
 void BrowserActionsContainer::OnWidgetClosing(views::Widget* widget) {

chrome/browser/ui/views/toolbar/browser_actions_container.h

@@ -240,7 +240,7 @@ class BrowserActionsContainer : public views::View,
   void StopAnimating() override;
   void ShowToolbarActionBubble(
       std::unique_ptr<ToolbarActionsBarBubbleDelegate> controller) override;
-  void CloseOverflowMenuIfOpen() override;
+  bool CloseOverflowMenuIfOpen() override;
 
   // views::WidgetObserver:
   void OnWidgetClosing(views::Widget* widget) override;

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

@@ -4,11 +4,9 @@
 
 #include "chrome/browser/ui/views/toolbar/media_router_action_platform_delegate_views.h"
 
+#include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "build/build_config.h"
-#include "chrome/browser/ui/views/frame/browser_view.h"
-#include "chrome/browser/ui/views/toolbar/browser_app_menu_button.h"
-#include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/browser/ui/views_mode_controller.h"
 
 // static
@@ -31,17 +29,3 @@ MediaRouterActionPlatformDelegateViews::MediaRouterActionPlatformDelegateViews(
 MediaRouterActionPlatformDelegateViews::
     ~MediaRouterActionPlatformDelegateViews() {
 }
-
-bool MediaRouterActionPlatformDelegateViews::CloseOverflowMenuIfOpen() {
-  // TODO(mgiuca): Use button_provider() instead of toolbar(), so this also
-  // works for hosted app windows.
-  AppMenuButton* app_menu_button =
-      BrowserView::GetBrowserViewForBrowser(browser_)
-          ->toolbar()
-          ->app_menu_button();
-  if (!app_menu_button || !app_menu_button->IsMenuShowing())
-    return false;
-
-  app_menu_button->CloseMenu();
-  return true;
-}

chrome/browser/ui/views/toolbar/media_router_action_platform_delegate_views.h

@@ -15,9 +15,6 @@ class MediaRouterActionPlatformDelegateViews :
   explicit MediaRouterActionPlatformDelegateViews(Browser* browser);
   ~MediaRouterActionPlatformDelegateViews() override;
 
-  // MediaRouterActionPlatformDelegate:
-  bool CloseOverflowMenuIfOpen() override;
-
  private:
   // The corresponding browser.
   Browser* const browser_;