Add base::RunLoop and update ui_test_utils to use it to reduce flakiness

base/message_loop.cc

 // Copyright (c) 2012 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.
 
 #include "base/message_loop.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 285 matching lines @@
 }
 
 void MessageLoop::PostNonNestableDelayedTask(
     const tracked_objects::Location& from_here,
     const base::Closure& task,
     base::TimeDelta delay) {
   PostNonNestableDelayedTask(from_here, task, delay.InMillisecondsRoundedUp());
 }
 
 void MessageLoop::Run() {
-  AutoRunState save_state(this);
+  AutoRunState save_state(this, NULL);
+  RunHandler();
+}
+
+void MessageLoop::RunWithID(const RunID& run_id) {
+  DCHECK(run_id.get() && !run_id->used_);
+  AutoRunState save_state(this, run_id);
+  if (run_id->quit_now_received_)

[jar (doing other things), 2012/06/18 19:43:21]

If you want to make the DCHECK in line 311 more valuable, you should add to this
clause:
run_id->used_ = true;

[jbates, 2012/06/18 23:14:03]

It's done by the AutoRunState constructor. I moved the DCHECK there to be more
clear.

+    return;
   RunHandler();
 }
 
 void MessageLoop::RunAllPending() {
-  AutoRunState save_state(this);
+  AutoRunState save_state(this, NULL);
   state_->quit_received = true;  // Means run until we would otherwise block.
   RunHandler();
 }
 
-void MessageLoop::Quit() {
+void MessageLoop::QuitWhenIdle() {
   DCHECK_EQ(this, current());
   if (state_) {
     state_->quit_received = true;
   } else {
     NOTREACHED() << "Must be inside Run to call Quit";
   }
 }
 
 void MessageLoop::QuitNow() {
   DCHECK_EQ(this, current());
   if (state_) {
     pump_->Quit();
   } else {
     NOTREACHED() << "Must be inside Run to call Quit";
   }
 }
 
-static void QuitCurrent() {
-  MessageLoop::current()->Quit();
+void MessageLoop::QuitNowWithID(const RunID& run_id) {
+  DCHECK_EQ(this, current());
+  if (!state_)
+    return;
+  // If the state_ pointer matches that of the RunIDObject, we're at the same
+  // Run scope, so quit now.
+  if (state_ == run_id->run_state_)
+    QuitNow();
+
+  // Set quit_now_received_ so that the corresponding Run scope is quit as soon
+  // as it is returned to. It's also possible that the corresponding Run has not
+  // begun; in that case, this will cause the Run to immediately return.

[jar (doing other things), 2012/06/18 19:43:21]

Is the case of calling quit before the RunWithId() useful?  IF so, this
justifies the separate bool.

Does this let us run exactly one task in a message loop? Is there another
scenario for this?

[jbates, 2012/06/18 23:14:03]

Explained in response to your other comment in message_loop.h. This feature
should allow some simplification of test code that is currently responsible for
tracking run/quit states. I added some new tests to verify behavior when
QuitWithID is used before, during and after RunWithID.

+  run_id->quit_now_received_ = true;
+}
+
+static void QuitCurrentWhenIdle() {
+  MessageLoop::current()->QuitWhenIdle();
+}
+
+static void QuitCurrentNow() {
+  MessageLoop::current()->QuitNow();
+}
+
+static void QuitCurrentNowWithID(const MessageLoop::RunID& run_id) {
+  MessageLoop::current()->QuitNowWithID(run_id);
 }
 
 // static
-base::Closure MessageLoop::QuitClosure() {
-  return base::Bind(&QuitCurrent);
+base::Closure MessageLoop::QuitWhenIdleClosure() {
+  return base::Bind(&QuitCurrentWhenIdle);
+}
+
+// static
+base::Closure MessageLoop::QuitNowClosure() {
+  return base::Bind(&QuitCurrentNow);
+}
+
+// static
+base::Closure MessageLoop::QuitNowWithIDClosure(
+    const RunID& run_id) {
+  return base::Bind(&QuitCurrentNowWithID, run_id);
+}
+
+MessageLoop::RunID MessageLoop::GetRunID() {
+  return new RunIDObject();
 }
 
 void MessageLoop::SetNestableTasksAllowed(bool allowed) {
   if (nestable_tasks_allowed_ != allowed) {
     nestable_tasks_allowed_ = allowed;
     if (!nestable_tasks_allowed_)
       return;
     // Start the native pump if we are not already pumping.
     pump_->ScheduleWork();
   }
@@ skipping 358 matching lines @@
 }
 
 void MessageLoop::ReleaseSoonInternal(
     const tracked_objects::Location& from_here,
     void(*releaser)(const void*),
     const void* object) {
   PostNonNestableTask(from_here, base::Bind(releaser, object));
 }
 
 //------------------------------------------------------------------------------
+// MessageLoop::RunState
+
+MessageLoop::RunState::RunState() {
+}
+
+MessageLoop::RunState::~RunState() {
+}
+
+//------------------------------------------------------------------------------
 // MessageLoop::AutoRunState
 
-MessageLoop::AutoRunState::AutoRunState(MessageLoop* loop) : loop_(loop) {
+MessageLoop::AutoRunState::AutoRunState(MessageLoop* loop, const RunID& run_id)
+    : loop_(loop) {
   // Make the loop reference us.
   previous_state_ = loop_->state_;
   if (previous_state_) {
     run_depth = previous_state_->run_depth + 1;
   } else {
     run_depth = 1;
   }
   loop_->state_ = this;
 
+  if (run_id.get()) {
+    run_state_id = run_id;
+    run_state_id->used_ = true;
+    run_state_id->run_state_ = this;
+  }
+
   // Initialize the other fields:
   quit_received = false;
 #if !defined(OS_MACOSX) && !defined(OS_ANDROID)
   dispatcher = NULL;
 #endif
 }
 
 MessageLoop::AutoRunState::~AutoRunState() {
   loop_->state_ = previous_state_;
+  if (run_state_id.get()) {
+    // run_state_id->run_state_ points to 'this', so clear it.
+    run_state_id->run_state_ = NULL;
+  }
+  if (previous_state_ && previous_state_->run_state_id.get()
+      && previous_state_->run_state_id->quit_now_received_)
+    MessageLoop::current()->QuitNow();
 }
 
 //------------------------------------------------------------------------------
 // MessageLoopForUI
 
 #if defined(OS_WIN)
 void MessageLoopForUI::DidProcessMessage(const MSG& message) {
   pump_win()->DidProcessMessage(message);
 }
 #endif  // defined(OS_WIN)
 
 #if defined(OS_ANDROID)
 void MessageLoopForUI::Start() {
   // No Histogram support for UI message loop as it is managed by Java side
   static_cast<base::MessagePumpForUI*>(pump_.get())->Start(this);
 }
 #endif
 
 #if !defined(OS_MACOSX) && !defined(OS_NACL) && !defined(OS_ANDROID)
 void MessageLoopForUI::AddObserver(Observer* observer) {
   pump_ui()->AddObserver(observer);
 }
 
 void MessageLoopForUI::RemoveObserver(Observer* observer) {
   pump_ui()->RemoveObserver(observer);
 }
 
 void MessageLoopForUI::RunWithDispatcher(Dispatcher* dispatcher) {
-  AutoRunState save_state(this);
+  AutoRunState save_state(this, NULL);
+  state_->dispatcher = dispatcher;
+  RunHandler();
+}
+
+void MessageLoopForUI::RunWithDispatcher(Dispatcher* dispatcher,
+                                         const RunID& run_id) {
+  AutoRunState save_state(this, run_id);
   state_->dispatcher = dispatcher;
   RunHandler();
 }
 
 void MessageLoopForUI::RunAllPendingWithDispatcher(Dispatcher* dispatcher) {
-  AutoRunState save_state(this);
+  AutoRunState save_state(this, NULL);
   state_->dispatcher = dispatcher;
   state_->quit_received = true;  // Means run until we would otherwise block.
   RunHandler();
 }
 
 #endif  //  !defined(OS_MACOSX) && !defined(OS_NACL) && !defined(OS_ANDROID)
 
 //------------------------------------------------------------------------------
 // MessageLoopForIO
 
@@ skipping 16 matching lines @@
                                            Watcher *delegate) {
   return pump_libevent()->WatchFileDescriptor(
       fd,
       persistent,
       static_cast<base::MessagePumpLibevent::Mode>(mode),
       controller,
       delegate);
 }
 
 #endif