MacViews: Fix flaky MenuRunnerCocoaTests.

ui/views/controls/menu/menu_runner_cocoa_unittest.mm

Index: ui/views/controls/menu/menu_runner_cocoa_unittest.mm
diff --git a/ui/views/controls/menu/menu_runner_cocoa_unittest.mm b/ui/views/controls/menu/menu_runner_cocoa_unittest.mm
index d25a6628b47db5294dba95b81c3816386746ed48..da18109b99ffec5b32e2e9ca946ea7b6939b263e 100644
--- a/ui/views/controls/menu/menu_runner_cocoa_unittest.mm
+++ b/ui/views/controls/menu/menu_runner_cocoa_unittest.mm
@@ -17,12 +17,41 @@ namespace views {
 namespace test {
 namespace {
 
+void MenuCancelCallback(internal::MenuRunnerImplCocoa* runner) {

[tapted, 2016/04/06 06:38:37]

I think these all would be better as member functions on MenuRunnerCocoaTest.
you just need to base base::Unretained(this) into base::Bind

[karandeepb, 2016/04/06 10:25:16]

Done.

+  runner->Cancel();
+  EXPECT_FALSE(runner->IsRunning());
+}
+
+void MenuDeleteCallback(internal::MenuRunnerImplCocoa** runner) {
+  (*runner)->Release();
+  *runner = nullptr;
+}
+
+void RunMenuWrapperCallback(const base::Closure& callback,
+                            internal::MenuRunnerImplCocoa* runner) {
+  EXPECT_TRUE(runner->IsRunning());
+  callback.Run();
+}
+
+void RunMenuAtCallback(views::Widget* parent,
+                       internal::MenuRunnerImplCocoa* runner,
+                       NSRect* last_anchor_frame) {
+  NSArray* subviews = [parent->GetNativeView() subviews];
+  EXPECT_EQ(2u, [subviews count]);
+  *last_anchor_frame = [[subviews objectAtIndex:1] frame];
+  runner->Cancel();
+}
+
 class TestModel : public ui::SimpleMenuModel {
  public:
   TestModel() : ui::SimpleMenuModel(&delegate_), delegate_(this) {}
 
   void set_checked_command(int command) { checked_command_ = command; }
 
+  void set_menu_open_callback(const base::Closure& callback) {
+    menu_open_callback_ = callback;
+  }
+
  private:
   class Delegate : public ui::SimpleMenuModel::Delegate {
    public:
@@ -37,6 +66,12 @@ class TestModel : public ui::SimpleMenuModel {
     }
     void ExecuteCommand(int command_id, int event_flags) override {}
 
+    void MenuWillShow(SimpleMenuModel* source) override {
+      if (model_->menu_open_callback_.is_null())

[tapted, 2016/04/06 06:38:37]

nit:
    if (!model_->menu_open_callback_.is_null())
      model_->menu_open_callback_.Run();

but also can we skip `if`? (is there a code path where we expect it to be null?)

[karandeepb, 2016/04/06 10:25:15]

Done.

+        return;
+      model_->menu_open_callback_.Run();
+    }
+
    private:
     TestModel* model_;
 
@@ -46,6 +81,7 @@ class TestModel : public ui::SimpleMenuModel {
  private:
   int checked_command_ = -1;
   Delegate delegate_;
+  base::Closure menu_open_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(TestModel);
 };
@@ -89,14 +125,12 @@ class MenuRunnerCocoaTest : public ViewsTestBase {
     ViewsTestBase::TearDown();
   }
 
-  // Runs the menu after scheduling |block| on the run loop.
-  MenuRunner::RunResult RunMenu(dispatch_block_t block) {
-    CFRunLoopPerformBlock(CFRunLoopGetCurrent(), kCFRunLoopCommonModes, ^{
-        EXPECT_TRUE(runner_->IsRunning());
-        block();
-    });
-    return runner_->RunMenuAt(parent_, NULL, gfx::Rect(), MENU_ANCHOR_TOPLEFT,
-                              MenuRunner::CONTEXT_MENU);
+  // Runs the menu after registering |callback| as the menu open callback.
+  MenuRunner::RunResult RunMenu(const base::Closure& callback) {
+    menu_->set_menu_open_callback(
+        base::Bind(&RunMenuWrapperCallback, callback, runner_));
+    return runner_->RunMenuAt(parent_, nullptr, gfx::Rect(),
+                              MENU_ANCHOR_TOPLEFT, MenuRunner::CONTEXT_MENU);
   }
 
   // Runs then cancels a combobox menu and captures the frame of the anchoring
@@ -107,12 +141,10 @@ class MenuRunnerCocoaTest : public ViewsTestBase {
     // Should be one child (the compositor layer) before showing, and it should
     // go up by one (the anchor view) while the menu is shown.
     EXPECT_EQ(1u, [[parent_->GetNativeView() subviews] count]);
-    CFRunLoopPerformBlock(CFRunLoopGetCurrent(), kCFRunLoopCommonModes, ^{
-      NSArray* subviews = [parent_->GetNativeView() subviews];
-      EXPECT_EQ(2u, [subviews count]);
-      last_anchor_frame_ = [[subviews objectAtIndex:1] frame];
-      runner_->Cancel();
-    });
+
+    menu_->set_menu_open_callback(
+        base::Bind(&RunMenuAtCallback, parent_, runner_, &last_anchor_frame_));
+
     MenuRunner::RunResult result = runner_->RunMenuAt(
         parent_, nullptr, anchor, MENU_ANCHOR_TOPLEFT, MenuRunner::COMBOBOX);
 
@@ -134,10 +166,8 @@ class MenuRunnerCocoaTest : public ViewsTestBase {
 TEST_F(MenuRunnerCocoaTest, RunMenuAndCancel) {
   base::TimeDelta min_time = ui::EventTimeForNow();
 
-  MenuRunner::RunResult result = RunMenu(^{
-      runner_->Cancel();
-      EXPECT_FALSE(runner_->IsRunning());
-  });
+  MenuRunner::RunResult result =
+      RunMenu(base::Bind(&MenuCancelCallback, runner_));
 
   EXPECT_EQ(MenuRunner::NORMAL_EXIT, result);
   EXPECT_FALSE(runner_->IsRunning());
@@ -151,19 +181,15 @@ TEST_F(MenuRunnerCocoaTest, RunMenuAndCancel) {
 }
 
 TEST_F(MenuRunnerCocoaTest, RunMenuAndDelete) {
-  MenuRunner::RunResult result = RunMenu(^{
-      runner_->Release();
-      runner_ = NULL;
-  });
-
+  MenuRunner::RunResult result =
+      RunMenu(base::Bind(&MenuDeleteCallback, &runner_));
   EXPECT_EQ(MenuRunner::MENU_DELETED, result);
 }
 
 TEST_F(MenuRunnerCocoaTest, RunMenuTwice) {
   for (int i = 0; i < 2; ++i) {
-    MenuRunner::RunResult result = RunMenu(^{
-        runner_->Cancel();
-    });
+    MenuRunner::RunResult result =
+        RunMenu(base::Bind(&MenuCancelCallback, runner_));
     EXPECT_EQ(MenuRunner::NORMAL_EXIT, result);
     EXPECT_FALSE(runner_->IsRunning());
   }