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

base/message_loop.h

Index: base/message_loop.h
diff --git a/base/message_loop.h b/base/message_loop.h
index d78b58addc2019ad46b61e79adf8c918f432a9dd..9989be082af9292be06187746b42ec563cf104bd 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -224,23 +224,61 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
   // Return as soon as all items that can be run are taken care of.
   void RunAllPending();
 
-  // Signals the Run method to return after it is done processing all pending
-  // messages.  This method may only be called on the same thread that called
-  // Run, and Run must still be on the call stack.
+  // TODO(jbates) remove this. crbug.com/131220. See QuitWhenIdle().
+  void Quit() { QuitWhenIdle(); }
+
+  // Signals the Run method to return when it becomes idle. It will continue to
+  // process pending messages and future messages as long as they are enqueued.
+  // Warning: if the MessageLoop remains busy, it may never quit. Only use this
+  // Quit method when looping procedures (such as web pages) have been shut
+  // down.
   //
-  // Use QuitClosure if you need to Quit another thread's MessageLoop, but note
-  // that doing so is fairly dangerous if the target thread makes nested calls
-  // to MessageLoop::Run.  The problem being that you won't know which nested
-  // run loop you are quitting, so be careful!
-  void Quit();
+  // This method may only be called on the same thread that called Run, and Run
+  // must still be on the call stack.
+  //
+  // Use Quit*Closure variants if you need to Quit another thread's MessageLoop,
+  // but note that doing so is fairly dangerous if the target thread makes
+  // nested calls to MessageLoop::Run.  The problem being that you won't know
+  // which nested run loop you are quitting, so be careful!
+  void QuitWhenIdle();
 
   // This method is a variant of Quit, that does not wait for pending messages
   // to be processed before returning from Run.
   void QuitNow();
 
-  // Invokes Quit on the current MessageLoop when run. Useful to schedule an
-  // arbitrary MessageLoop to Quit.
-  static base::Closure QuitClosure();
+  // This method is a variant of QuitNow that applies to a specific nested call
+  // to MessageLoop::Run(). |run_id| is returned by an earlier call to
+  // next_run_id(). If the corresponding MessageLoop::Run call has already
+  // completed, nothing happens. If there are one or more nested calls to
+  // MessageLoop::Run inside the target Run call, the QuitNow is deferred until
+  // all nested Run calls have completed.
+  void QuitNowByID(int run_id);

[jar (doing other things), 2012/06/14 04:50:29]

This is an interesting extension.
My general feeling is that I want to always massively discourage nested message
loops, and although this helps some scenarios, it is still murderously hard to
use these without messing up (use only with fully re-entrant tasks!!!).

From an API perspective, I'd rather not expose next_run_id(), which gives a
caller the ability to mess-up, and get the wrong id (some how).  I'd rather have
a 

Run(int* generated_run_id)  // arg is output value only.

This would put all the onus of manipulating these ids as a private affair in the
message loop, but allow a thread that called Run() to get a copy of the id
pushed to an accessible place, for use in a Quit(int
previously_generated_run_id).

[jbates, 2012/06/15 02:20:38]

Done as discussed over chat.

+
+  // TODO(jbates) remove this. crbug.com/131220. See QuitWhenIdleClosure().
+  static base::Closure QuitClosure() { return QuitWhenIdleClosure(); }
+
+  // Construct a Closure that will call QuitWhenIdle(). Useful to schedule an
+  // arbitrary MessageLoop to QuitWhenIdle.
+  static base::Closure QuitWhenIdleClosure();
+
+  // Construct a Closure that will call QuitNow(). Useful to schedule an
+  // arbitrary MessageLoop to QuitNow.
+  static base::Closure QuitNowClosure();
+
+  // Construct a Closure that will call QuitNowByID(run_id). Useful to schedule
+  // an arbitrary MessageLoop to QuitNowByID.
+  static base::Closure QuitNowByIDClosure(int run_id);
+
+  // Get the run id for the next call to MessageLoop::Run. Use this
+  // value to quit a specific nested call to MessageLoop::Run. For example:
+  //
+  //  // Record next_run_id before running the MessageLoop:
+  //  run_id_ = MessageLoop::current()->next_run_id();
+  //  MessageLoop::current()->Run();

[jar (doing other things), 2012/06/14 04:50:29]

With the proposed API (preventing others from manipulating run_ids).  This could
would read:
int run_id = 0;
MessageLoop::current()->Run(&run_id);

IMO, if you failed to pass in a run_id pointer, or passed in NULL, then *no*
run_id would even be generated (and hence no need for a global lock, etc.).

+  //
+  //  // Then, during a task or callback during the above call to Run:
+  //  MessageLoop::current()->QuitNowByID(run_id_);
+  int next_run_id() const { return current_run_state_id_ + 1; }
 
   // Returns the type passed to the constructor.
   Type type() const { return type_; }
@@ -366,13 +404,20 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
     // Used to count how many Run() invocations are on the stack.
     int run_depth;
 
+    // Used to distinguish this RunState from other nested RunStates.
+    int run_state_id;
+
     // Used to record that Quit() was called, or that we should quit the pump
     // once it becomes idle.
     bool quit_received;
 
+    // Used to record that QuitNow() was called at this run_depth.
+    bool quit_now_received;
+
 #if !defined(OS_MACOSX) && !defined(OS_ANDROID)
     Dispatcher* dispatcher;
 #endif
+    RunState* previous_state;
   };
 
 #if defined(OS_ANDROID)
@@ -386,7 +431,6 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
     ~AutoRunState();
    private:
     MessageLoop* loop_;
-    RunState* previous_state_;
   };
 #if defined(OS_ANDROID)
  protected:
@@ -522,6 +566,10 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
   scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
   scoped_ptr<base::ThreadTaskRunnerHandle> thread_task_runner_handle_;
 
+  // Unique identifier (within the scope of this MessageLoop) for AutoRunState
+  // instances (AutoRunState is associated with calls to Run).

[jar (doing other things), 2012/06/14 04:50:29]

It might be nice to expand on this statement.  Is the ID globally unique for the
process? Is it generated automagically for the message loop (thread?), or is it
the responsibility of the caller of Run() to select one?

[jbates, 2012/06/15 02:20:38]

Removed.

+  int current_run_state_id_;
+
  private:
   template <class T, class R> friend class base::subtle::DeleteHelperInternal;
   template <class T, class R> friend class base::subtle::ReleaseHelperInternal;