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

content/test/test_navigation_observer.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 "content/public/test/test_navigation_observer.h"
 
 #include "base/bind.h"
 #include "base/message_loop.h"
+#include "base/run_loop.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_view_host_observer.h"
 #include "content/public/test/js_injection_ready_observer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
+namespace {
+
+// Allow some pending tasks up to |num_deferrals| generations to complete.
+void DeferredQuitRunLoop(const base::Closure& quit_task,

[Paweł Hajdan Jr., 2012/06/27 07:33:28]

I've seen this routine in this CL before. Please de-duplicate and move it to
ui_test_utils.

[jbates, 2012/06/28 20:46:11]

Yeah it should be moved to content/test/test_utils.h (or similar). Added TODO.

+                         int num_quit_deferrals) {
+  if (num_quit_deferrals <= 0) {
+    quit_task.Run();
+  } else {
+    MessageLoop::current()->PostTask(FROM_HERE,
+        base::Bind(&DeferredQuitRunLoop, quit_task, num_quit_deferrals - 1));
+  }
+}
+
+}  // namespace
+
 // This class observes |render_view_host| and calls OnJsInjectionReady() of
 // |js_injection_ready_observer| when the time is right to inject JavaScript
 // into the page.
 class TestNavigationObserver::RVHOSendJS : public RenderViewHostObserver {
  public:
   RVHOSendJS(RenderViewHost* render_view_host,
              JsInjectionReadyObserver* js_injection_ready_observer)
       : RenderViewHostObserver(render_view_host),
         js_injection_ready_observer_(js_injection_ready_observer) {
   }
@@ skipping 50 matching lines @@
     return;
 
   EXPECT_FALSE(running_);
   running_ = true;
   done_callback_ = done_callback;
   wait_loop_callback.Run();
   EXPECT_TRUE(done_);
 }
 
 void TestNavigationObserver::Wait() {
+  base::RunLoop run_loop;
+  // Number of times to repost Quit task to allow pending tasks to complete. See
+  // kNumQuitDeferrals in ui_test_utils.cc for explanation.

[Paweł Hajdan Jr., 2012/06/27 07:33:28]

Why not make ui_test_utils expose its kNumQuitDeferrals constant instead of
duplicating it here?

Ah right, this is content/ vs. chrome/ thing... Feel free to create some test
utils files in content/test then.

And because this CL is already complex, feel free to do that in a separate CL
(just stick a TODO here so that things are not forgotten).

[jbates, 2012/06/28 20:46:11]

Sounds good, added a TODO.

+  const int num_quit_deferrals = 10;
   WaitForObservation(
-      base::Bind(&MessageLoop::Run,
-                 base::Unretained(MessageLoop::current())),
-      base::Bind(&MessageLoop::Quit,
-                 base::Unretained(MessageLoop::current())));
+      base::Bind(&base::RunLoop::Run, base::Unretained(&run_loop)),
+      base::Bind(&DeferredQuitRunLoop, run_loop.QuitClosure(),
+                 num_quit_deferrals));
 }
 
 TestNavigationObserver::TestNavigationObserver(
     JsInjectionReadyObserver* js_injection_ready_observer,
     int number_of_navigations)
     : navigation_started_(false),
       navigations_completed_(0),
       number_of_navigations_(number_of_navigations),
       js_injection_ready_observer_(js_injection_ready_observer),
       done_(false),
@@ skipping 22 matching lines @@
   switch (type) {
     case NOTIFICATION_NAV_ENTRY_COMMITTED:
     case NOTIFICATION_LOAD_START:
       navigation_started_ = true;
       break;
     case NOTIFICATION_LOAD_STOP:
       if (navigation_started_ &&
           ++navigations_completed_ == number_of_navigations_) {
         navigation_started_ = false;
         done_ = true;
-        if (running_)
+        if (running_) {
+          running_ = false;
           done_callback_.Run();
+        }
       }
       break;
     case NOTIFICATION_RENDER_VIEW_HOST_CREATED:
       rvho_send_js_.reset(new RVHOSendJS(
           Source<RenderViewHost>(source).ptr(),
           js_injection_ready_observer_));
       break;
     default:
       NOTREACHED();
   }
 }
 
 }  // namespace content