Componentize component_updater: Break content/ dependency for rest of component_updater tests.

components/component_updater/test/update_checker_unittest.cc

Index: components/component_updater/test/update_checker_unittest.cc
diff --git a/components/component_updater/test/update_checker_unittest.cc b/components/component_updater/test/update_checker_unittest.cc
index 79cf15c4e7dc255f52a35323a8cea21a5a2c0989..352c36e3a53cd6cb5d9c717d0e2dd48fe2fbcec7 100644
--- a/components/component_updater/test/update_checker_unittest.cc
+++ b/components/component_updater/test/update_checker_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/version.h"
@@ -14,14 +15,10 @@
 #include "components/component_updater/test/test_configurator.h"
 #include "components/component_updater/test/url_request_post_interceptor.h"
 #include "components/component_updater/update_checker.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/test/test_browser_thread_bundle.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-using content::BrowserThread;
-
 namespace component_updater {
 
 namespace {
@@ -67,16 +64,15 @@ class UpdateCheckerTest : public testing::Test {
   UpdateResponse::Results results_;
 
  private:
-  content::TestBrowserThreadBundle thread_bundle_;
+  base::MessageLoopForIO loop_;
   base::Closure quit_closure_;
 
   DISALLOW_COPY_AND_ASSIGN(UpdateCheckerTest);
 };
 
 UpdateCheckerTest::UpdateCheckerTest()
-    : config_(new TestConfigurator),
-      error_(0),
-      thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {
+    : post_interceptor_(NULL),
+      error_(0) {
   net::URLFetcher::SetEnableInterceptionForTests(true);
 }
 
@@ -85,7 +81,10 @@ UpdateCheckerTest::~UpdateCheckerTest() {
 }
 
 void UpdateCheckerTest::SetUp() {
-  interceptor_factory_.reset(new InterceptorFactory);
+  config_.reset(new TestConfigurator(base::MessageLoopProxy::current(),
+                                     base::MessageLoopProxy::current()));
+  interceptor_factory_.reset(
+      new InterceptorFactory(base::MessageLoopProxy::current()));
   post_interceptor_ = interceptor_factory_->CreateInterceptor();
   EXPECT_TRUE(post_interceptor_);
 
@@ -103,6 +102,10 @@ void UpdateCheckerTest::TearDown() {
   interceptor_factory_.reset();
 
   config_.reset();
+
+  // The PostInterceptor requires the message loop to run to destruct correctly.
+  // TODO: This is fragile and should be fixed.
+  RunThreadsUntilIdle();

[blundell, 2014/08/27 12:44:56]

Why didn't this need to be done before?

[tommycli, 2014/08/27 15:36:30]

In the destructor of content::TestBrowserThreadBundle it runs every message loop
until idle.

The real issue, IMO, is that PostInterceptor (and TestUrlRequestInterceptor has
the same problem) requires the IO message loops to be run to clean itself up
correctly.

I think it should either be reworked to not require this, or run the loop
itself, or hang the constructor until the IO thread cleanup task is run.

The way it is currently, you have to do this, or you'll get a failure on the
next test that uses the interceptor.

[Ryan Sleevi, 2014/09/03 00:12:17]

This is more of a question for Paul. content:TBTB is "Doing the right thing" and
matching the guarantees of the //content API.

I don't see the issue you described as a "real issue", but perhaps I'm missing
something.

 }
 
 void UpdateCheckerTest::RunThreads() {