base: Don't keep running tasks after nested run loop quits on iOS

The CoreFoundation message pump (MessagePumpCFRunLoop) scheduled an extra call
to Delegate::DoWork in the following scenario:

  1. Start a run loop and schedule DoWork.
  2. In DoWork, start a second run nested run loop which quits immediately.
  3. After the nested run loop quits, schedule another DoWork which quits the
     original run loop.

After step #3, the message pump would call DoWork again because the nested
run loop triggered the execution of the nesting deferred work source.

This patch fixes the issue by checking whether the further work is allowed
before calling DoWork. Note that this check can't be done just for the nested
deferred work source, because in the flow above that source is what triggers
the call to DoWork in step #3, i.e., the run loop hasn't quit yet at that point.

The patch also includes a fix to
WebContentsViewMacInteractiveTest.SelectMenuLifetime; the test had the
following sequence in it (while a select menu is open):

  1. Quit an outer run loop.
  2. Post a task onto an inner run loop to continue the test.

The assumption here was that a select menu causes an inner run loop to
be active while the menu is open. This wasn't strictly correct: the
inner run loop is a native (CoreFoundation) run loop as opposed to a
base::RunLoop, which can't be explicitly exited with RunLoop::Quit().
This means that the Quit() in step #1 affects the same run loop as the
PostTask in step #2, i.e., the task is prevented from running.

This patch changes the test to only quit the test run loop after the
posted task (which closes the select menu) has run.

Bug: 891670
Change-Id: I6fe21289205664c4e1b1469547495667c753f56d
Reviewed-on: https://chromium-review.googlesource.com/c/1373754Reviewed-by: Mark Mentovai <mark@chromium.org>
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615967}

base/message_loop/message_pump_mac.mm

@@ -472,6 +472,8 @@ bool MessagePumpCFRunLoopBase::RunWork() {
     delegateless_work_ = true;
     return false;
   }
+  if (!keep_running())
+    return false;
 
   // The NSApplication-based run loop only drains the autorelease pool at each
   // UI event (NSEvent).  The autorelease pool is not drained for each

base/message_loop/message_pump_unittest.cc

@@ -8,7 +8,9 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
+using ::testing::AnyNumber;
 using ::testing::Invoke;
+using ::testing::Return;
 
 namespace base {
 namespace {
@@ -50,6 +52,39 @@ TEST_P(MessagePumpTest, QuitStopsWork) {
   message_pump_->Run(&delegate);
 }
 
+TEST_P(MessagePumpTest, QuitStopsWorkWithNestedRunLoop) {
+  testing::InSequence sequence;
+  testing::StrictMock<MockMessagePumpDelegate> delegate;
+  testing::StrictMock<MockMessagePumpDelegate> nested_delegate;
+
+  // We first schedule a call to DoWork, which runs a nested run loop. After the
+  // nested loop exits, we schedule another DoWork which quits the outer
+  // (original) run loop. The test verifies that there are no extra calls to
+  // DoWork after the outer loop quits.
+  EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([&] {
+    message_pump_->ScheduleWork();
+    message_pump_->Run(&nested_delegate);
+    message_pump_->ScheduleWork();
+    return false;
+  }));
+  EXPECT_CALL(nested_delegate, DoWork).WillOnce(Invoke([&] {
+    // Quit the nested run loop.
+    message_pump_->Quit();
+    return false;
+  }));
+  EXPECT_CALL(delegate, DoDelayedWork(_)).WillOnce(Return(false));
+  // The outer pump may or may not trigger idle work at this point.
+  EXPECT_CALL(delegate, DoIdleWork()).Times(AnyNumber());
+  EXPECT_CALL(delegate, DoWork).WillOnce(Invoke([&] {
+    // Quit the original run loop.
+    message_pump_->Quit();
+    return false;
+  }));
+
+  message_pump_->ScheduleWork();
+  message_pump_->Run(&delegate);
+}
+
 INSTANTIATE_TEST_CASE_P(,
                         MessagePumpTest,
                         ::testing::Values(MessageLoop::TYPE_DEFAULT,

chrome/browser/ui/cocoa/tab_contents/web_contents_view_mac_interactive_uitest.mm

@@ -6,6 +6,7 @@
 
 #import "base/mac/scoped_nsobject.h"
 #include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -41,20 +42,17 @@ IN_PROC_BROWSER_TEST_F(WebContentsViewMacInteractiveTest, SelectMenuLifetime) {
               usingBlock:^(NSNotification* notification) {
                 first_item.reset(
                     [[[[notification object] itemAtIndex:0] title] copy]);
-                // The nested run loop runs next. Ensure the outer run loop
-                // exits once the inner run loop quits.
-                outer_run_loop_for_block->Quit();
-
                 // We can't cancel tracking until after
                 // NSMenuDidBeginTrackingNotification is processed (i.e. after
                 // this block returns). So post a task to run on the inner run
                 // loop which will close the tab (and cancel tracking in
-                // ~PopupMenuHelper()).
+                // ~PopupMenuHelper()) and quit the outer run loop to continue
+                // the test.
                 base::ThreadTaskRunnerHandle::Get()->PostTask(
-                    FROM_HERE,
-                    base::Bind(
-                        base::IgnoreResult(&TabStripModel::CloseWebContentsAt),
-                        base::Unretained(browser()->tab_strip_model()), 1, 0));
+                    FROM_HERE, base::BindLambdaForTesting([&] {
+                      browser()->tab_strip_model()->CloseWebContentsAt(1, 0);
+                      outer_run_loop_for_block->Quit();
+                    }));
               }];
 
   // Send a space key to open the <select>.