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 a92ff7e2660f8afff037bd291c2c1a6de2d5ddf8..92d7d3b0be09b9e03020529b714cefa8738b1228 100644
--- a/base/message_loop.h
+++ b/base/message_loop.h
@@ -14,6 +14,7 @@
 #include "base/callback_forward.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop_proxy.h"
 #include "base/message_pump.h"
 #include "base/observer_list.h"
@@ -43,6 +44,9 @@
 namespace base {
 class Histogram;
 class ThreadTaskRunnerHandle;
+#if defined(OS_ANDROID)
+class MessagePumpForUI;
+#endif
 }  // namespace base
 
 // A MessageLoop is used to process events for a particular thread.  There is
@@ -209,6 +213,90 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
         this, from_here, object);
   }
 
+  // Helper class to Run a nested MessageLoop. Please do not use nested
+  // MessageLoops in production code! If you must, use this class instead of
+  // calling MessageLoop::Run/Quit directly. RunLoop::Run can only be called
+  // once per RunLoop lifetime. Create a RunLoop on the stack and call Run/Stop
+  // to run a nested MessageLoop.
+  class RunLoop : public base::SupportsWeakPtr<RunLoop> {

[darin (slow to review), 2012/06/22 04:42:23]

this can be a top-level type in the base:: namespace.

[jbates, 2012/06/22 23:21:01]

Done.

+   public:
+    RunLoop();
+#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
+    explicit RunLoop(Dispatcher* dispatcher);

[darin (slow to review), 2012/06/22 04:42:23]

cool, i had the same idea to just make the dispatcher a property of the RunLoop
to avoid the need for RunWithDispatcher :)

+#endif
+    ~RunLoop();
+
+#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
+    void set_dispatcher(Dispatcher* dispatcher) { dispatcher_ = dispatcher; }
+#endif
+
+    // Run the current MessageLoop. This blocks until Stop is called. Before
+    // calling Run, be sure to grab an AsWeakPtr or the StopClosure in order to
+    // stop the MessageLoop asynchronously. MessageLoop::Quit and QuitNow will
+    // also trigger a return from Run, but those are deprecated.
+    void Run();
+
+    // Run the current MessageLoop until it doesn't find any tasks or messages
+    // in the queue (it goes idle). WARNING: This may never return! Only use
+    // this when repeating tasks such as animated web pages have been shut down.
+    void RunUntilIdle();
+
+    // Stop an earlier call to this->Run(). Nested RunLoops (and deprecated
+    // MessageLoop::Run*) are unaffected and continue to run until they are
+    // stopped by other means. This call stops processing tasks immediately
+    // (when there are no nested RunLoops running), leaving any pending tasks

[darin (slow to review), 2012/06/22 04:42:23]

i think you can just define this in terms of quitting the RunLoop.  there can be
other RunLoops servicing the same task queue (MessageLoop), and quitting one
RunLoop has no bearing on the others.

[jbates, 2012/06/22 23:21:01]

Done.

+    // unexecuted. Stop can be called before, during or after Run. If called
+    // before Run, Run will return immediately when called.
+    //
+    // WARNING: You must NEVER assume that a call to Stop will terminate the
+    // targetted message loop. If a nested message loop continues running, the
+    // target may NEVER terminate. It is very easy to livelock (run forever) in
+    // such a case.
+    void Stop();

[darin (slow to review), 2012/06/22 04:42:23]

probably should go back to calling this Quit.  but we can define it to mean an
immediate quit.

[jbates, 2012/06/22 23:21:01]

Done.

+
+    // Hand this closure off to code that asynchronously notifies of completion.
+    // The returned Closure is safe to run after the RunLoop has been
+    // destructed. Example:
+    //   RunLoop run_loop;
+    //   kick_off_some_procedure(run_loop.StopClosure());
+    //   run_loop.Run();
+    base::Closure StopClosure();
+
+   private:
+    friend class MessageLoop;
+#if defined(OS_ANDROID)
+    // Android doesn't support the blocking MessageLoop::Run, so it calls
+    // BeforeRun and AfterRun directly.
+    friend class base::MessagePumpForUI;
+#endif
+
+    // Return false to abort the Run.
+    bool BeforeRun();
+    void AfterRun();
+
+    MessageLoop* loop_;
+
+    base::WeakPtr<RunLoop> previous_run_loop_;
+
+    bool run_called_;
+    bool stop_called_;
+    bool running_;
+
+    // Used to count how many nested Run() invocations are on the stack.
+    int run_depth_;
+
+    // Used to record that Quit() was called on the MessageLoop, meaning that we
+    // should quit Run once it becomes idle.
+    bool quit_when_idle_received_;
+
+#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
+    Dispatcher* dispatcher_;
+#endif
+
+    DISALLOW_COPY_AND_ASSIGN(RunLoop);
+  };
+
+  // Deprecated: use RunLoop instead.
   // Run the message loop.
   void Run();
 
@@ -216,23 +304,39 @@ 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(); }
+
+  // Deprecated: use RunLoop or RunAllPending instead.
   //
-  // 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();
+  // 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.
+  //
+  // This method may only be called on the same thread that called Run, and Run
+  // must still be on the call stack.
+  //
+  // Use QuitClosure 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();

[darin (slow to review), 2012/06/22 04:42:23]

hmm... thinking about names again.  RunUntilIdle() and QuitWhenIdle() fit nicely
together.  RunAllPending() and QuitWhenIdle() are not so obviously related.  I'm
not sure how to rename QuitWhenIdle() to say "Pending" instead of "Idle".  Maybe
that means the names with "Idle" are better.  QuitWhenNonePending :-P :-P

[jbates, 2012/06/22 23:21:01]

I'm not too partial. I guess I prefer "Idle" because it might make people more
inclined to RTFM compared to Pending which seems more likely inferred as
all-currently-pending. Switching everything to Idle too see how it looks...

 
+  // Deprecated: use RunLoop instead.
+  //
   // 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();
+  // TODO(jbates) remove this. crbug.com/131220. See QuitWhenIdleClosure().
+  static base::Closure QuitClosure() { return QuitWhenIdleClosure(); }
+
+  // Deprecated: use RunLoop instead.
+  // Construct a Closure that will call QuitWhenIdle(). Useful to schedule an
+  // arbitrary MessageLoop to QuitWhenIdle.
+  static base::Closure QuitWhenIdleClosure();
 
   // Returns the type passed to the constructor.
   Type type() const { return type_; }
@@ -354,35 +458,7 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
 
   //----------------------------------------------------------------------------
  protected:
-  struct RunState {
-    // Used to count how many Run() invocations are on the stack.
-    int run_depth;
-
-    // Used to record that Quit() was called, or that we should quit the pump
-    // once it becomes idle.
-    bool quit_received;
-
-#if !defined(OS_MACOSX) && !defined(OS_ANDROID)
-    Dispatcher* dispatcher;
-#endif
-  };
-
-#if defined(OS_ANDROID)
-  // Android Java process manages the UI thread message loop. So its
-  // MessagePumpForUI needs to keep the RunState.
- public:
-#endif
-  class BASE_EXPORT AutoRunState : RunState {
-   public:
-    explicit AutoRunState(MessageLoop* loop);
-    ~AutoRunState();
-   private:
-    MessageLoop* loop_;
-    RunState* previous_state_;
-  };
-#if defined(OS_ANDROID)
- protected:
-#endif
+  friend class RunLoop;
 
 #if defined(OS_WIN)
   base::MessagePumpWin* pump_win() {
@@ -496,7 +572,7 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
   // Protect access to incoming_queue_.
   mutable base::Lock incoming_queue_lock_;
 
-  RunState* state_;
+  base::WeakPtr<RunLoop> run_loop_;
 
 #if defined(OS_WIN)
   base::TimeTicks high_resolution_timer_expiration_;
@@ -525,7 +601,6 @@ class BASE_EXPORT MessageLoop : public base::MessagePump::Delegate {
                            void(*releaser)(const void*),
                            const void* object);
 
-
   DISALLOW_COPY_AND_ASSIGN(MessageLoop);
 };