net: Make TestCompletionCallback use RunLoops.

This is more robust against re-entrantly run MessageLoops.

BUG=none

Review URL: https://codereview.chromium.org/1142843002

Cr-Commit-Position: refs/heads/master@{#330825}

net/base/test_completion_callback.cc

@@ -7,7 +7,7 @@
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
-#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
 #include "net/base/io_buffer.h"
 
 namespace net {
@@ -16,23 +16,27 @@ namespace internal {
 
 void TestCompletionCallbackBaseInternal::DidSetResult() {
   have_result_ = true;
-  if (waiting_for_result_)
-    base::MessageLoop::current()->Quit();
+  if (run_loop_)
+    run_loop_->Quit();
 }
 
 void TestCompletionCallbackBaseInternal::WaitForResult() {
-  DCHECK(!waiting_for_result_);
-  while (!have_result_) {
-    waiting_for_result_ = true;
-    base::MessageLoop::current()->Run();
-    waiting_for_result_ = false;
+  DCHECK(!run_loop_);
+  if (!have_result_) {
+    run_loop_.reset(new base::RunLoop());
+    run_loop_->Run();
+    run_loop_.reset();
+    DCHECK(have_result_);
+    // A huge number of tests depend on this class running events after the
+    // result is set.
+    // TODO(mmenke):  We really should fix this.
+    base::RunLoop().RunUntilIdle();
   }
   have_result_ = false;  // Auto-reset for next callback.
 }
 
 TestCompletionCallbackBaseInternal::TestCompletionCallbackBaseInternal()
-    : have_result_(false),
-      waiting_for_result_(false) {
+    : have_result_(false) {
 }
 
 TestCompletionCallbackBaseInternal::~TestCompletionCallbackBaseInternal() {

net/base/test_completion_callback.h

@@ -7,6 +7,7 @@
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
 #include "net/base/completion_callback.h"
 #include "net/base/net_errors.h"
 
@@ -15,12 +16,17 @@
 
 // A helper class for completion callbacks, designed to make it easy to run
 // tests involving asynchronous operations.  Just call WaitForResult to wait
-// for the asynchronous operation to complete.
+// for the asynchronous operation to complete.  Uses a RunLoop to spin the
+// current MessageLoop while waiting.  The callback must be invoked on the same
+// thread WaitForResult is called on.
 //
 // NOTE: Since this runs a message loop to wait for the completion callback,
 // there could be other side-effects resulting from WaitForResult.  For this
 // reason, this class is probably not ideal for a general application.
 //
+namespace base {
+class RunLoop;
+}
 
 namespace net {
 
@@ -39,10 +45,12 @@ class TestCompletionCallbackBaseInternal {
   void DidSetResult();
   void WaitForResult();
 
+ private:
+  // RunLoop.  Only non-NULL during the call to WaitForResult, so the class is
+  // reusable.
+  scoped_ptr<base::RunLoop> run_loop_;
   bool have_result_;
-  bool waiting_for_result_;
 
- private:
   DISALLOW_COPY_AND_ASSIGN(TestCompletionCallbackBaseInternal);
 };
 
@@ -64,23 +72,23 @@ class TestCompletionCallbackTemplate
   }
 
  protected:
+  TestCompletionCallbackTemplate() : result_(R()) {}
+
   // Override this method to gain control as the callback is running.
   virtual void SetResult(R result) {
     result_ = result;
     DidSetResult();
   }
 
-  TestCompletionCallbackTemplate() : result_(R()) {}
+ private:
   R result_;
 
- private:
   DISALLOW_COPY_AND_ASSIGN(TestCompletionCallbackTemplate);
 };
 
 }  // namespace internal
 
-class TestClosure
-    : public internal::TestCompletionCallbackBaseInternal {
+class TestClosure : public internal::TestCompletionCallbackBaseInternal {
  public:
   using internal::TestCompletionCallbackBaseInternal::WaitForResult;