Mac[Views]: Make native menus more responsive by pumping private runloop modes.

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/extensions/browser_action_button.h"
 
 #import <Cocoa/Cocoa.h>
 
 #include <memory>
 
@@ skipping 19 matching lines @@
 #include "chrome/browser/ui/global_error/global_error_service.h"
 #include "chrome/browser/ui/global_error/global_error_service_factory.h"
 #include "chrome/browser/ui/toolbar/component_toolbar_actions_factory.h"
 #include "chrome/browser/ui/toolbar/media_router_action.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_bar.h"
 #include "chrome/browser/ui/toolbar/toolbar_actions_model.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "ui/base/cocoa/cocoa_base_utils.h"
 #import "ui/events/test/cocoa_test_event_utils.h"
 
+// A helper class to wait for a menu to open and close.
+@interface MenuWatcher : NSObject
+- (id)initWithController:(MenuController*)controller;
+@property(nonatomic, assign) base::Closure openClosure;
+@property(nonatomic, assign) base::Closure closeClosure;
+@end
+
 namespace {
 
 const int kMenuPadding = 26;
 
 // A simple error class that has a menu item.
 class MenuError : public GlobalError {
  public:
   MenuError() {}
   ~MenuError() override {}
 
@@ skipping 36 matching lines @@
 
 // Moves the mouse (synchronously) to the center of the given |view|.
 void MoveMouseToCenter(NSView* view) {
   NSPoint centerPoint = GetCenterPoint(view);
   base::RunLoop runLoop;
   ui_controls::SendMouseMoveNotifyWhenDone(
       centerPoint.x, centerPoint.y, runLoop.QuitClosure());
   runLoop.Run();
 }
 
-// Simulates a click on the action button in the overflow menu, and runs
-// |closure| upon completion.
-void ClickOnOverflowedAction(ToolbarController* toolbarController,
-                             const base::Closure& closure) {
-  AppMenuController* appMenuController = [toolbarController appMenuController];
+// Simulates a right-click on the action button in the overflow menu.
+void DispatchClickOnOverflowedAction(BrowserActionButton* action_button) {
+  NSEvent* event = nil;  // The event doesn't matter when sending directly.
+  [action_button rightMouseDown:event];
+}
+
+// ui_controls:: methods don't play nice when there is an open menu (like the
+// app menu). Instead, ClickOnOverflowedAction() simulates a right click by
+// feeding the event directly to the button. In regular interaction, this is
+// dispatched after menuDidClose:. To simulate that, start the menu closing
+// (with no action), but invoke the action on the button directly when the close
+// is observed.
+void ClickOnOverflowedAction(
+    base::scoped_nsobject<MenuWatcher> app_menu_watcher,
+    AppMenuController* app_menu_controller) {
   // The app menu should start as open (since that's where the overflowed
   // actions are).
-  EXPECT_TRUE([appMenuController isMenuOpen]);
-  BrowserActionsController* overflowController =
-      [appMenuController browserActionsController];
+  EXPECT_TRUE([app_menu_controller isMenuOpen]);
 
-  ASSERT_TRUE(overflowController);
-  BrowserActionButton* actionButton = [overflowController buttonWithIndex:0];
+  BrowserActionsController* overflow_controller =
+      [app_menu_controller browserActionsController];
+
+  ASSERT_TRUE(overflow_controller);
+  BrowserActionButton* action_button = [overflow_controller buttonWithIndex:0];
+
   // The action should be attached to a superview.
-  EXPECT_TRUE([actionButton superview]);
+  EXPECT_TRUE([action_button superview]);
 
-  // ui_controls:: methods don't play nice when there is an open menu (like the
-  // app menu). Instead, simulate a right click by feeding the event directly to
-  // the button.
-  NSPoint centerPoint = GetCenterPoint(actionButton);
-  NSEvent* mouseEvent = cocoa_test_event_utils::RightMouseDownAtPointInWindow(
-      centerPoint, [actionButton window]);
-  [actionButton rightMouseDown:mouseEvent];
+  base::Closure invoke_action = base::Bind(&DispatchClickOnOverflowedAction,
+                                           base::Unretained(action_button));
+  [app_menu_watcher setCloseClosure:invoke_action];
 
-  // This should close the app menu.
-  EXPECT_FALSE([appMenuController isMenuOpen]);
-  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, closure);
+  // Close the app menu.
+  [app_menu_controller cancel];
 }
 
 }  // namespace
 
 // A simple helper menu delegate that will keep track of if a menu is opened,
 // and closes them immediately (which is useful because message loops with
 // menus open in Cocoa don't play nicely with testing).
 @interface MenuHelper : NSObject<NSMenuDelegate> {
   // Whether or not a menu has been opened. This can be reset so the helper can
   // be used multiple times.
   BOOL menuOpened_;
 
+  // The closure to run when the menu opens, if any.
+  base::Closure openClosure_;
+
   // A function to be called to verify state while the menu is open.
   base::Closure verify_;
 }
 
-// Bare-bones init.
-- (id)init;
-
 @property(nonatomic, assign) BOOL menuOpened;
+@property(nonatomic, assign) base::Closure openClosure;
 @property(nonatomic, assign) base::Closure verify;
 
 @end
 
 @implementation MenuHelper
 
 - (void)menuWillOpen:(NSMenu*)menu {
   menuOpened_ = YES;
+  if (!openClosure_.is_null())
+    openClosure_.Run();
   if (!verify_.is_null())
     verify_.Run();
   [menu cancelTracking];
 }
 
-- (id)init {
-  self = [super init];
-  return self;
-}
-
 @synthesize menuOpened = menuOpened_;
+@synthesize openClosure = openClosure_;
 @synthesize verify = verify_;
 
 @end
 
-// A helper class to wait for a menu to open and close.
-@interface MenuWatcher : NSObject {
+@implementation MenuWatcher {
   // The MenuController for the menu this object is watching.
   MenuController* menuController_;
 
   // The closure to run when the menu opens, if any.
   base::Closure openClosure_;
 
   // The closure to run when the menu closes, if any.
   base::Closure closeClosure_;
 }
 
-- (id)initWithController:(MenuController*)controller;
-
-// Notifications from the MenuController.
-- (void)menuDidClose:(NSNotification*)notification;
-- (void)menuDidOpen:(NSNotification*)notification;
-
-@property(nonatomic, assign) base::Closure openClosure;
-@property(nonatomic, assign) base::Closure closeClosure;
-
-@end
-
-@implementation MenuWatcher
-
 @synthesize openClosure = openClosure_;
 @synthesize closeClosure = closeClosure_;
 
 - (id)initWithController:(MenuController*)controller {
   if (self = [super init]) {
     menuController_ = controller;
     [[NSNotificationCenter defaultCenter]
         addObserver:self
          selector:@selector(menuDidOpen:)
        name:kMenuControllerMenuWillOpenNotification
         object:menuController_];
     [[NSNotificationCenter defaultCenter]
         addObserver:self
          selector:@selector(menuDidClose:)
        name:kMenuControllerMenuDidCloseNotification
         object:menuController_];
   }
   return self;
 }
 
 - (void)dealloc {
   [[NSNotificationCenter defaultCenter] removeObserver:self];
   [super dealloc];
 }
 
 - (void)menuDidClose:(NSNotification*)notification {
   if (!closeClosure_.is_null()) {
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::ResetAndReturn(&closeClosure_));
+    // Run |closeClosure_| synchronously since it may depend on objects that are
+    // torn down once the menu is closed and execution has returned to the main
+    // run loop.
+    closeClosure_.Run();
+    closeClosure_.Reset();
   }
 }
 
 - (void)menuDidOpen:(NSNotification*)notification {
   if (!openClosure_.is_null()) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::ResetAndReturn(&openClosure_));
   }
 }
 
@@ skipping 20 matching lines @@
     ExtensionBrowserTest::SetUpCommandLine(command_line);
     ToolbarActionsBar::disable_animations_for_testing_ = true;
   }
 
   void TearDownOnMainThread() override {
     ToolbarActionsBar::disable_animations_for_testing_ = false;
     ExtensionBrowserTest::TearDownOnMainThread();
   }
 
   // Opens the app menu and the context menu of the overflowed action, and
-  // checks that the menus get opened/closed properly. |menuHelper| must be set
-  // as the delegate for the context menu.
-  void OpenAppMenuAndActionContextMenu(MenuHelper* menuHelper) {
+  // checks that the menus get opened/closed properly.
+  void OpenAppMenuAndActionContextMenu(MenuHelper* context_menu_helper) {
     // Move the mouse over the app menu button.
     MoveMouseToCenter(appMenuButton());
 
     // No menu yet (on the browser action).
-    EXPECT_FALSE([menuHelper menuOpened]);
-    base::RunLoop runLoop;
-    // Click on the app menu, and pass in a callback to continue the test in
-    // ClickOnOverflowedAction. Due to the blocking nature of Cocoa menus, we
-    // can't test the menu synchronously here by quitting the run loop when it
-    // opens, and instead need to test through a callback.
-    base::scoped_nsobject<MenuWatcher> menuWatcher(
+    EXPECT_FALSE([context_menu_helper menuOpened]);
+    base::RunLoop run_loop;
+    base::scoped_nsobject<MenuWatcher> app_menu_watcher(
         [[MenuWatcher alloc] initWithController:appMenuController()]);
-    [menuWatcher
-        setOpenClosure:base::Bind(&ClickOnOverflowedAction,
-                                  base::Unretained(toolbarController()),
-                                  runLoop.QuitClosure())];
+
+    base::Closure close_with_action =
+        base::Bind(&ClickOnOverflowedAction, app_menu_watcher,
+                   base::Unretained(appMenuController()));
+    [app_menu_watcher setOpenClosure:close_with_action];
+
+    // Quit the RunLoop below when the context menu opens. This dictates that
+    // the MenuHelper's verify action has also run.
+    [context_menu_helper setOpenClosure:run_loop.QuitClosure()];
+
+    // Show the app menu.
     ui_controls::SendMouseEvents(ui_controls::LEFT,
                                  ui_controls::DOWN | ui_controls::UP);
-    runLoop.Run();
+    run_loop.Run();
 
     // The menu opened on the main bar's action button rather than the
     // overflow's since Cocoa does not support running a menu within a menu.
-    EXPECT_TRUE([menuHelper menuOpened]);
+    EXPECT_TRUE([context_menu_helper menuOpened]);
   }
 
   ToolbarController* toolbarController() { return toolbarController_; }
   AppMenuController* appMenuController() { return appMenuController_; }
   ToolbarActionsModel* model() { return model_; }
   NSView* appMenuButton() { return [toolbarController_ appMenuButton]; }
 
  private:
   ToolbarController* toolbarController_ = nil;
   AppMenuController* appMenuController_ = nil;
@@ skipping 375 matching lines @@
   openAndCloseAppMenu(appMenuController());
 
   // Move the extension back to the main bar, so an overflow bar is no longer
   // needed. Then open and close the app menu a couple times.
   // This tests that the menu properly cleans up after itself when an overflow
   // was present, and is no longer (fix for crbug.com/603241).
   model()->SetVisibleIconCount(1);
   openAndCloseAppMenu(appMenuController());
   openAndCloseAppMenu(appMenuController());
 }