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

components/component_updater/test/update_checker_unittest.cc

 // Copyright 2014 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 "base/basictypes.h"
 #include "base/compiler_specific.h"
 #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"
 #include "components/component_updater/crx_update_item.h"
 #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 {
 
 base::FilePath test_file(const char* file) {
   base::FilePath path;
   PathService::Get(base::DIR_SOURCE_ROOT, &path);
   return path.AppendASCII("components").AppendASCII("test").AppendASCII("data")
       .AppendASCII("component_updater").AppendASCII(file);
 }
@@ skipping 25 matching lines @@
   scoped_ptr<UpdateChecker> update_checker_;
 
   scoped_ptr<InterceptorFactory> interceptor_factory_;
   URLRequestPostInterceptor* post_interceptor_;  // Owned by the factory.
 
   int error_;
   std::string error_message_;
   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);
 }
 
 UpdateCheckerTest::~UpdateCheckerTest() {
   net::URLFetcher::SetEnableInterceptionForTests(false);
 }
 
 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_);
 
   update_checker_.reset();
 
   error_ = 0;
   error_message_.clear();
   results_ = UpdateResponse::Results();
 }
 
 void UpdateCheckerTest::TearDown() {
   update_checker_.reset();
 
   post_interceptor_ = NULL;
   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() {
   base::RunLoop runloop;
   quit_closure_ = runloop.QuitClosure();
   runloop.Run();
 
   // Since some tests need to drain currently enqueued tasks such as network
   // intercepts on the IO thread, run the threads until they are
   // idle. The component updater service won't loop again until the loop count
@@ skipping 106 matching lines @@
       << post_interceptor_->GetRequestsAsString();
   EXPECT_EQ(1, post_interceptor_->GetCount())
       << post_interceptor_->GetRequestsAsString();
 
   EXPECT_NE(0, error_);
   EXPECT_STREQ("network error", error_message_.c_str());
   EXPECT_EQ(0ul, results_.list.size());
 }
 
 }  // namespace component_updater