Refactor the interface for the CrxDownloader to take a callback.

chrome/browser/component_updater/test/crx_downloader_unittest.cc

 // Copyright 2013 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/bind.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
@@ skipping 36 matching lines @@
 
   void Quit();
   void RunThreads();
   void RunThreadsUntilIdle();
 
   void DownloadComplete(int crx_context, const CrxDownloader::Result& result);
 
  protected:
   scoped_ptr<CrxDownloader> crx_downloader_;
 
+  CrxDownloader::DownloadCallback callback_;
+
   int crx_context_;
   int error_;
   base::FilePath response_;
 
   int num_calls_;
 
   // A magic value for the context to be used in the tests.
   static const int kExpectedContext = 0xaabb;
 
  private:
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
   scoped_refptr<net::TestURLRequestContextGetter> context_;
   content::TestBrowserThreadBundle thread_bundle_;
   base::Closure quit_closure_;
 };
 
 const int CrxDownloaderTest::kExpectedContext;
 
 CrxDownloaderTest::CrxDownloaderTest()
-    : crx_context_(0),
+    : callback_(base::Bind(&CrxDownloaderTest::DownloadComplete,
+                           base::Unretained(this),
+                           kExpectedContext)),
+      crx_context_(0),
       error_(0),
       num_calls_(0),
       blocking_task_runner_(BrowserThread::GetBlockingPool()->
           GetSequencedTaskRunnerWithShutdownBehavior(
               BrowserThread::GetBlockingPool()->GetSequenceToken(),
               base::SequencedWorkerPool::SKIP_ON_SHUTDOWN)),
       context_(new net::TestURLRequestContextGetter(
           BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO))),
       thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP) {
 }
 
 CrxDownloaderTest::~CrxDownloaderTest() {
   context_ = NULL;
 }
 
 void CrxDownloaderTest::SetUp() {
   num_calls_ = 0;
   crx_downloader_.reset(CrxDownloader::Create(
       false,    // Do not use the background downloader in these tests.
       context_.get(),
-      blocking_task_runner_,
-      base::Bind(&CrxDownloaderTest::DownloadComplete,
-                 base::Unretained(this),
-                 kExpectedContext)));
+      blocking_task_runner_));
 }
 
 void CrxDownloaderTest::TearDown() {
 }
 
 void CrxDownloaderTest::Quit() {
   if (!quit_closure_.is_null())
     quit_closure_.Run();
 }
 
@@ skipping 19 matching lines @@
   RunThreadsUntilIdle();
 }
 
 void CrxDownloaderTest::RunThreadsUntilIdle() {
   base::RunLoop().RunUntilIdle();
 }
 
 // Tests that starting a download without a url results in an error.
 TEST_F(CrxDownloaderTest, NoUrl) {
   std::vector<GURL> urls;
-  crx_downloader_->StartDownload(urls);
+  crx_downloader_->StartDownload(urls, callback_);
 
   RunThreadsUntilIdle();
   EXPECT_EQ(-1, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 // Tests that downloading from one url is successful.
 TEST_F(CrxDownloaderTest, OneUrl) {
   const GURL expected_crx_url =
     GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx");
 
   GetInterceptor interceptor;
   interceptor.SetResponse(
        expected_crx_url,
        test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
-  crx_downloader_->StartDownloadFromUrl(expected_crx_url);
+  crx_downloader_->StartDownloadFromUrl(expected_crx_url, callback_);
 
   RunThreads();
   EXPECT_EQ(1, interceptor.GetHitCount());
   EXPECT_EQ(0, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 // Tests that specifying from two urls has no side effects. Expect a successful
@@ skipping 10 matching lines @@
 
   GetInterceptor interceptor;
   interceptor.SetResponse(
        expected_crx_url,
        test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
   std::vector<GURL> urls;
   urls.push_back(expected_crx_url);
   urls.push_back(expected_crx_url);
 
-  crx_downloader_->StartDownload(urls);
+  crx_downloader_->StartDownload(urls, callback_);
 
   RunThreads();
   EXPECT_EQ(1, interceptor.GetHitCount());
   EXPECT_EQ(0, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 // Tests that an invalid host results in a download error.
 TEST_F(CrxDownloaderTest, OneUrl_InvalidHost) {
   const GURL expected_crx_url =
     GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx");
 
   GetInterceptor interceptor;
   interceptor.SetResponse(
        expected_crx_url,
        test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
   crx_downloader_->StartDownloadFromUrl(
       GURL("http://no.such.host"
-           "/download/jebgalgnebhfojomionfpkfelancnnkf.crx"));
+           "/download/jebgalgnebhfojomionfpkfelancnnkf.crx"),
+      callback_);
 
   RunThreads();
   EXPECT_EQ(0, interceptor.GetHitCount());
   EXPECT_NE(0, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 // Tests that an invalid path results in a download error.
 TEST_F(CrxDownloaderTest, OneUrl_InvalidPath) {
   const GURL expected_crx_url =
     GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx");
 
   GetInterceptor interceptor;
   interceptor.SetResponse(
        expected_crx_url,
        test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
-  crx_downloader_->StartDownloadFromUrl(GURL("http://localhost/no/such/file"));
+  crx_downloader_->StartDownloadFromUrl(
+      GURL("http://localhost/no/such/file"),
+      callback_);
 
   RunThreads();
   EXPECT_EQ(0, interceptor.GetHitCount());
   EXPECT_NE(0, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 // Tests that the fallback to a valid url is successful.
 // This test is flaky on Android. crbug.com/329883
 #if defined(OS_ANDROID)
 #define MAYBE_TwoUrls_FirstInvalid DISABLED_TwoUrls_FirstInvalid
 #else
 #define MAYBE_TwoUrls_FirstInvalid TwoUrls_FirstInvalid
 #endif
 TEST_F(CrxDownloaderTest, MAYBE_TwoUrls_FirstInvalid) {
   const GURL expected_crx_url =
     GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx");
 
   GetInterceptor interceptor;
   interceptor.SetResponse(
        expected_crx_url,
        test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
   std::vector<GURL> urls;
   urls.push_back(GURL("http://localhost/no/such/file"));
   urls.push_back(expected_crx_url);
 
-  crx_downloader_->StartDownload(urls);
+  crx_downloader_->StartDownload(urls, callback_);
 
   RunThreads();
   EXPECT_EQ(1, interceptor.GetHitCount());
   EXPECT_EQ(0, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 // Tests that the download succeeds if the first url is correct and the
 // second bad url does not have a side-effect.
 TEST_F(CrxDownloaderTest, TwoUrls_SecondInvalid) {
   const GURL expected_crx_url =
     GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx");
 
   GetInterceptor interceptor;
   interceptor.SetResponse(
        expected_crx_url,
        test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
   std::vector<GURL> urls;
   urls.push_back(expected_crx_url);
   urls.push_back(GURL("http://localhost/no/such/file"));
 
-  crx_downloader_->StartDownload(urls);
+  crx_downloader_->StartDownload(urls, callback_);
 
   RunThreads();
   EXPECT_EQ(1, interceptor.GetHitCount());
   EXPECT_EQ(0, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 // Tests that the download fails if both urls are bad.
 TEST_F(CrxDownloaderTest, TwoUrls_BothInvalid) {
   const GURL expected_crx_url =
     GURL("http://localhost/download/jebgalgnebhfojomionfpkfelancnnkf.crx");
 
   GetInterceptor interceptor;
   interceptor.SetResponse(
        expected_crx_url,
        test_file("jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
   std::vector<GURL> urls;
   urls.push_back(GURL("http://localhost/no/such/file"));
   urls.push_back(GURL("http://no.such.host/"
                       "/download/jebgalgnebhfojomionfpkfelancnnkf.crx"));
 
-  crx_downloader_->StartDownload(urls);
+  crx_downloader_->StartDownload(urls, callback_);
 
   RunThreads();
   EXPECT_EQ(0, interceptor.GetHitCount());
   EXPECT_NE(0, error_);
   EXPECT_EQ(kExpectedContext, crx_context_);
 
   EXPECT_EQ(1, num_calls_);
 }
 
 }  // namespace component_updater
-

[waffles, 2014/04/24 23:27:53]

I'm glad you got rid of this. Now if only we could get rid of it in all the
other files...