Implement support for fallback update check urls in the component updater

This allows for specifying more than one urls for update checks and pings.
The urls are then tried in the order they are specified.

BUG=413879

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

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

chrome/browser/component_updater/chrome_component_updater_configurator.cc

@@ -47,13 +47,11 @@ const char kSwitchUrlSource[] = "url-source";
 #define COMPONENT_UPDATER_SERVICE_ENDPOINT \
   "//clients2.google.com/service/update2"
 
-// The default url for the v3 protocol service endpoint. Can be
-// overridden with --component-updater=url-source=someurl.
+// The default url for the v3 protocol service endpoint.
+// The value of |kDefaultUrlSource| can be overridden with
+// --component-updater=url-source=someurl.
 const char kDefaultUrlSource[] = "https:" COMPONENT_UPDATER_SERVICE_ENDPOINT;
 
-// The url to send the pings to.
-const char kPingUrl[] = "https:" COMPONENT_UPDATER_SERVICE_ENDPOINT;
-
 // Disables differential updates.
 const char kSwitchDisableDeltaUpdates[] = "disable-delta-updates";
 
@@ -103,8 +101,8 @@ class ChromeConfigurator : public Configurator {
   virtual int StepDelayMedium() OVERRIDE;
   virtual int MinimumReCheckWait() const OVERRIDE;
   virtual int OnDemandDelay() const OVERRIDE;
-  virtual GURL UpdateUrl() const OVERRIDE;
-  virtual GURL PingUrl() const OVERRIDE;
+  virtual std::vector<GURL> UpdateUrl() const OVERRIDE;
+  virtual std::vector<GURL> PingUrl() const OVERRIDE;
   virtual base::Version GetBrowserVersion() const OVERRIDE;
   virtual std::string GetChannel() const OVERRIDE;
   virtual std::string GetLang() const OVERRIDE;
@@ -124,7 +122,7 @@ class ChromeConfigurator : public Configurator {
  private:
   net::URLRequestContextGetter* url_request_getter_;
   std::string extra_info_;
-  std::string url_source_;
+  GURL url_source_override_;
   bool fast_update_;
   bool pings_enabled_;
   bool deltas_enabled_;
@@ -155,9 +153,11 @@ ChromeConfigurator::ChromeConfigurator(
   background_downloads_enabled_ = false;
 #endif
 
-  url_source_ = GetSwitchArgument(switch_values, kSwitchUrlSource);
-  if (url_source_.empty()) {
-    url_source_ = kDefaultUrlSource;
+  const std::string switch_url_source =
+      GetSwitchArgument(switch_values, kSwitchUrlSource);
+  if (!switch_url_source.empty()) {
+    url_source_override_ = GURL(switch_url_source);
+    DCHECK(url_source_override_.is_valid());
   }
 
   if (HasSwitchValue(switch_values, kSwitchRequestParam))
@@ -188,12 +188,18 @@ int ChromeConfigurator::OnDemandDelay() const {
   return fast_update_ ? 2 : (30 * kDelayOneMinute);
 }
 
-GURL ChromeConfigurator::UpdateUrl() const {
-  return GURL(url_source_);
+std::vector<GURL> ChromeConfigurator::UpdateUrl() const {
+  std::vector<GURL> urls;
+  if (url_source_override_.is_valid()) {
+    urls.push_back(GURL(url_source_override_));
+  } else {
+    urls.push_back(GURL(kDefaultUrlSource));
+  }
+  return urls;
 }
 
-GURL ChromeConfigurator::PingUrl() const {
-  return pings_enabled_ ? GURL(kPingUrl) : GURL();
+std::vector<GURL> ChromeConfigurator::PingUrl() const {
+  return pings_enabled_ ? UpdateUrl() : std::vector<GURL>();
 }
 
 base::Version ChromeConfigurator::GetBrowserVersion() const {

components/component_updater.gypi

@@ -49,6 +49,8 @@
         'component_updater/default_component_installer.h',
         'component_updater/pref_names.cc',
         'component_updater/pref_names.h',
+        'component_updater/request_sender.cc',
+        'component_updater/request_sender.h',
         'component_updater/update_checker.cc',
         'component_updater/update_checker.h',
         'component_updater/update_response.cc',

components/component_updater/BUILD.gn

@@ -30,6 +30,8 @@ source_set("component_updater") {
     "default_component_installer.h",
     "pref_names.cc",
     "pref_names.h",
+    "request_sender.cc",
+    "request_sender.h",
     "update_checker.cc",
     "update_checker.h",
     "update_response.cc",
@@ -75,6 +77,7 @@ source_set("unit_tests") {
     "test/component_patcher_unittest.cc",
     "test/component_updater_ping_manager_unittest.cc",
     "test/crx_downloader_unittest.cc",
+    "test/request_sender_unittest.cc",
     "test/update_checker_unittest.cc",
     "test/update_response_unittest.cc",
   ]

components/component_updater/component_patcher.cc

@@ -9,6 +9,7 @@
 
 #include "base/basictypes.h"
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/json/json_file_value_serializer.h"

components/component_updater/component_updater_configurator.h

@@ -6,6 +6,7 @@
 #define COMPONENTS_COMPONENT_UPDATER_COMPONENT_UPDATER_CONFIGURATOR_H_
 
 #include <string>
+#include <vector>
 
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ -53,12 +54,13 @@ class Configurator {
   // for the same component.
   virtual int OnDemandDelay() const = 0;
 
-  // The url that is going to be used update checks over Omaha protocol.
-  virtual GURL UpdateUrl() const = 0;
+  // The URLs for the update checks. The URLs are tried in order, the first one
+  // that succeeds wins.
+  virtual std::vector<GURL> UpdateUrl() const = 0;
 
-  // The url where the completion pings are sent. Invalid if and only if
-  // pings are disabled.
-  virtual GURL PingUrl() const = 0;
+  // The URLs for pings. Returns an empty vector if and only if pings are
+  // disabled. Similarly, these URLs have a fall back behavior too.
+  virtual std::vector<GURL> PingUrl() const = 0;
 
   // Version of the application. Used to compare the component manifests.
   virtual base::Version GetBrowserVersion() const = 0;

components/component_updater/component_updater_ping_manager.cc

@@ -5,23 +5,33 @@
 #include "components/component_updater/component_updater_ping_manager.h"
 
 #include <string>
+#include <vector>
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
-#include "base/guid.h"
+#include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/sequenced_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/threading/thread_checker.h"
 #include "components/component_updater/component_updater_configurator.h"
 #include "components/component_updater/component_updater_utils.h"
 #include "components/component_updater/crx_update_item.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_delegate.h"
+#include "components/component_updater/request_sender.h"
 #include "url/gurl.h"
 
+namespace net {
+class URLFetcher;
+}  // namespace net
+
 namespace component_updater {
 
+namespace {
+
 // Returns a string literal corresponding to the value of the downloader |d|.
 const char* DownloaderToString(CrxDownloader::DownloadMetrics::Downloader d) {
   switch (d) {
@@ -34,86 +44,9 @@ const char* DownloaderToString(CrxDownloader::DownloadMetrics::Downloader d) {
   }
 }
 
-// Sends a fire and forget ping. The instances of this class have no
-// ownership and they self-delete upon completion.
-class PingSender : public net::URLFetcherDelegate {
- public:
-  PingSender();
-
-  void SendPing(const Configurator& config,
-                net::URLRequestContextGetter* url_request_context_getter,
-                const CrxUpdateItem* item);
-
- private:
-  virtual ~PingSender();
-
-  // Overrides for URLFetcherDelegate.
-  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
-
-  static std::string BuildPing(const Configurator& config,
-                               const CrxUpdateItem* item);
-  static std::string BuildDownloadCompleteEventElements(
-      const CrxUpdateItem* item);
-  static std::string BuildUpdateCompleteEventElement(const CrxUpdateItem* item);
-
-  scoped_ptr<net::URLFetcher> url_fetcher_;
-
-  DISALLOW_COPY_AND_ASSIGN(PingSender);
-};
-
-PingSender::PingSender() {
-}
-
-PingSender::~PingSender() {
-}
-
-void PingSender::OnURLFetchComplete(const net::URLFetcher* source) {
-  delete this;
-}
-
-void PingSender::SendPing(
-    const Configurator& config,
-    net::URLRequestContextGetter* url_request_context_getter,
-    const CrxUpdateItem* item) {
-  DCHECK(item);
-
-  if (!config.PingUrl().is_valid())
-    return;
-
-  url_fetcher_.reset(SendProtocolRequest(config.PingUrl(),
-                                         BuildPing(config, item),
-                                         this,
-                                         url_request_context_getter));
-}
-
-// Builds a ping message for the specified update item.
-std::string PingSender::BuildPing(const Configurator& config,
-                                  const CrxUpdateItem* item) {
-  const char app_element_format[] =
-      "<app appid=\"%s\" version=\"%s\" nextversion=\"%s\">"
-      "%s"
-      "%s"
-      "</app>";
-  const std::string app_element(base::StringPrintf(
-      app_element_format,
-      item->id.c_str(),                                    // "appid"
-      item->previous_version.GetString().c_str(),          // "version"
-      item->next_version.GetString().c_str(),              // "nextversion"
-      BuildUpdateCompleteEventElement(item).c_str(),       // update event
-      BuildDownloadCompleteEventElements(item).c_str()));  // download events
-
-  return BuildProtocolRequest(config.GetBrowserVersion().GetString(),
-                              config.GetChannel(),
-                              config.GetLang(),
-                              config.GetOSLongName(),
-                              app_element,
-                              "");
-}
-
 // Returns a string representing a sequence of download complete events
 // corresponding to each download metrics in |item|.
-std::string PingSender::BuildDownloadCompleteEventElements(
-    const CrxUpdateItem* item) {
+std::string BuildDownloadCompleteEventElements(const CrxUpdateItem* item) {
   using base::StringAppendF;
   std::string download_events;
   for (size_t i = 0; i != item->download_metrics.size(); ++i) {
@@ -153,8 +86,7 @@ std::string PingSender::BuildDownloadCompleteEventElements(
 }
 
 // Returns a string representing one ping event xml element for an update item.
-std::string PingSender::BuildUpdateCompleteEventElement(
-    const CrxUpdateItem* item) {
+std::string BuildUpdateCompleteEventElement(const CrxUpdateItem* item) {
   DCHECK(item->status == CrxUpdateItem::kNoUpdate ||
          item->status == CrxUpdateItem::kUpdated);
 
@@ -189,6 +121,80 @@ std::string PingSender::BuildUpdateCompleteEventElement(
   return ping_event;
 }
 
+// Builds a ping message for the specified update item.
+std::string BuildPing(const Configurator& config, const CrxUpdateItem* item) {
+  const char app_element_format[] =
+      "<app appid=\"%s\" version=\"%s\" nextversion=\"%s\">"
+      "%s"
+      "%s"
+      "</app>";
+  const std::string app_element(base::StringPrintf(
+      app_element_format,
+      item->id.c_str(),                                    // "appid"
+      item->previous_version.GetString().c_str(),          // "version"
+      item->next_version.GetString().c_str(),              // "nextversion"
+      BuildUpdateCompleteEventElement(item).c_str(),       // update event
+      BuildDownloadCompleteEventElements(item).c_str()));  // download events
+
+  return BuildProtocolRequest(config.GetBrowserVersion().GetString(),
+                              config.GetChannel(),
+                              config.GetLang(),
+                              config.GetOSLongName(),
+                              app_element,
+                              "");
+}
+
+// Sends a fire and forget ping. The instances of this class have no
+// ownership and they self-delete upon completion. One instance of this class
+// can send only one ping.
+class PingSender {
+ public:
+  explicit PingSender(const Configurator& config);
+  ~PingSender();
+
+  bool SendPing(const CrxUpdateItem* item);
+
+ private:
+  void OnRequestSenderComplete(const net::URLFetcher* source);
+
+  const Configurator& config_;
+  scoped_ptr<RequestSender> request_sender_;
+  base::ThreadChecker thread_checker_;
+
+  DISALLOW_COPY_AND_ASSIGN(PingSender);
+};
+
+PingSender::PingSender(const Configurator& config) : config_(config) {
+}
+
+PingSender::~PingSender() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+}
+
+void PingSender::OnRequestSenderComplete(const net::URLFetcher* source) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  delete this;
+}
+
+bool PingSender::SendPing(const CrxUpdateItem* item) {
+  DCHECK(item);
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  std::vector<GURL> urls(config_.PingUrl());
+
+  if (urls.empty())
+    return false;
+
+  request_sender_.reset(new RequestSender(config_));
+  request_sender_->Send(
+      BuildPing(config_, item),
+      urls,
+      base::Bind(&PingSender::OnRequestSenderComplete, base::Unretained(this)));
+  return true;
+}
+
+}  // namespace
+
 PingManager::PingManager(const Configurator& config) : config_(config) {
 }
 
@@ -196,10 +202,11 @@ PingManager::~PingManager() {
 }
 
 // Sends a fire and forget ping when the updates are complete. The ping
-// sender object self-deletes after sending the ping.
+// sender object self-deletes after sending the ping has completed asynchrously.
 void PingManager::OnUpdateComplete(const CrxUpdateItem* item) {
-  PingSender* ping_sender(new PingSender);
-  ping_sender->SendPing(config_, config_.RequestContext(), item);
+  PingSender* ping_sender(new PingSender(config_));
+  if (!ping_sender->SendPing(item))
+    delete ping_sender;
 }
 
 }  // namespace component_updater

components/component_updater/component_updater_service.cc

@@ -10,6 +10,7 @@
 
 #include "base/at_exit.h"
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
@@ -146,7 +147,8 @@ class CrxUpdateService : public ComponentUpdateService, public OnDemandUpdater {
   // Overrides for OnDemandUpdater.
   virtual Status OnDemandUpdate(const std::string& component_id) OVERRIDE;
 
-  void UpdateCheckComplete(int error,
+  void UpdateCheckComplete(const GURL& original_url,
+                           int error,
                            const std::string& error_message,
                            const UpdateResponse::Results& results);
   void OnUpdateCheckSucceeded(const UpdateResponse::Results& results);
@@ -613,12 +615,12 @@ bool CrxUpdateService::CheckForUpdates() {
   if (items_to_check.empty())
     return false;
 
-  update_checker_ =
-      UpdateChecker::Create(*config_,
-                            base::Bind(&CrxUpdateService::UpdateCheckComplete,
-                                       base::Unretained(this))).Pass();
-  return update_checker_->CheckForUpdates(items_to_check,
-                                          config_->ExtraRequestParams());
+  update_checker_ = UpdateChecker::Create(*config_).Pass();
+  return update_checker_->CheckForUpdates(
+      items_to_check,
+      config_->ExtraRequestParams(),
+      base::Bind(&CrxUpdateService::UpdateCheckComplete,
+                 base::Unretained(this)));
 }
 
 void CrxUpdateService::UpdateComponent(CrxUpdateItem* workitem) {
@@ -661,10 +663,12 @@ void CrxUpdateService::UpdateComponent(CrxUpdateItem* workitem) {
 }
 
 void CrxUpdateService::UpdateCheckComplete(
+    const GURL& original_url,
     int error,
     const std::string& error_message,
     const UpdateResponse::Results& results) {
   DCHECK(thread_checker_.CalledOnValidThread());
+  VLOG(1) << "Update check completed from: " << original_url.spec();
   update_checker_.reset();
   if (!error)
     OnUpdateCheckSucceeded(results);

components/component_updater/default_component_installer.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"

components/component_updater/request_sender.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 "components/component_updater/request_sender.h"
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/sequenced_task_runner.h"
+#include "components/component_updater/component_updater_configurator.h"
+#include "components/component_updater/component_updater_utils.h"
+#include "net/url_request/url_fetcher.h"
+
+namespace component_updater {
+
+RequestSender::RequestSender(const Configurator& config) : config_(config) {
+}
+
+RequestSender::~RequestSender() {
+}
+
+void RequestSender::Send(const std::string& request_string,
+                         const std::vector<GURL>& urls,
+                         const RequestSenderCallback& request_sender_callback) {
+  if (urls.empty()) {
+    request_sender_callback.Run(NULL);
+    return;
+  }
+
+  request_string_ = request_string;
+  urls_ = urls;
+  request_sender_callback_ = request_sender_callback;
+
+  cur_url_ = urls_.begin();
+
+  SendInternal();
+}
+
+void RequestSender::SendInternal() {
+  DCHECK(cur_url_ != urls_.end());
+  DCHECK(cur_url_->is_valid());
+
+  url_fetcher_.reset(SendProtocolRequest(
+      *cur_url_, request_string_, this, config_.RequestContext()));
+}
+
+void RequestSender::OnURLFetchComplete(const net::URLFetcher* source) {
+  if (GetFetchError(*source) == 0) {
+    request_sender_callback_.Run(source);
+    return;
+  }
+
+  if (++cur_url_ != urls_.end() &&
+      config_.GetSequencedTaskRunner()->PostTask(
+          FROM_HERE,
+          base::Bind(&RequestSender::SendInternal, base::Unretained(this)))) {
+    return;
+  }
+
+  request_sender_callback_.Run(source);
+}
+
+}  // namespace component_updater

components/component_updater/request_sender.h

+// 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.
+
+#ifndef COMPONENTS_COMPONENT_UPDATER_REQUEST_SENDER_H_
+#define COMPONENTS_COMPONENT_UPDATER_REQUEST_SENDER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/compiler_specific.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
+#include "net/url_request/url_fetcher_delegate.h"
+#include "url/gurl.h"
+
+namespace net {
+class URLFetcher;
+}
+
+namespace component_updater {
+
+class Configurator;
+
+// Sends a request to one of the urls provided. The class implements a chain
+// of responsibility design pattern, where the urls are tried in the order they
+// are specified, until the request to one of them succeeds or all have failed.
+class RequestSender : public net::URLFetcherDelegate {
+ public:
+  // The |source| refers to the fetcher object used to make the request. This
+  // parameter can be NULL in some error cases.
+  typedef base::Callback<void(const net::URLFetcher* source)>
+      RequestSenderCallback;
+
+  explicit RequestSender(const Configurator& config);
+  virtual ~RequestSender();
+
+  void Send(const std::string& request_string,
+            const std::vector<GURL>& urls,
+            const RequestSenderCallback& request_sender_callback);
+
+ private:
+  void SendInternal();
+
+  // Overrides for URLFetcherDelegate.
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  const Configurator& config_;
+  std::vector<GURL> urls_;
+  std::vector<GURL>::const_iterator cur_url_;
+  scoped_ptr<net::URLFetcher> url_fetcher_;
+  std::string request_string_;
+  RequestSenderCallback request_sender_callback_;
+
+  DISALLOW_COPY_AND_ASSIGN(RequestSender);
+};
+
+}  // namespace component_updater
+
+#endif  // COMPONENTS_COMPONENT_UPDATER_REQUEST_SENDER_H_

components/component_updater/test/component_patcher_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "base/base_paths.h"
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"

components/component_updater/test/crx_downloader_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"

components/component_updater/test/request_sender_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/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "components/component_updater/request_sender.h"
+#include "components/component_updater/test/test_configurator.h"
+#include "components/component_updater/test/url_request_post_interceptor.h"
+#include "net/url_request/url_fetcher.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace component_updater {
+
+namespace {
+
+const char kUrl1[] = "https://localhost2/path1";
+const char kUrl2[] = "https://localhost2/path2";
+const char kUrlPath1[] = "path1";
+const char kUrlPath2[] = "path2";
+
+}  // namespace
+
+class RequestSenderTest : public testing::Test {
+ public:
+  RequestSenderTest();
+  virtual ~RequestSenderTest();
+
+  // Overrides from testing::Test.
+  virtual void SetUp() OVERRIDE;
+  virtual void TearDown() OVERRIDE;
+
+  void RequestSenderComplete(const net::URLFetcher* source);
+
+ protected:
+  void Quit();
+  void RunThreads();
+  void RunThreadsUntilIdle();
+
+  scoped_ptr<TestConfigurator> config_;
+  scoped_ptr<RequestSender> request_sender_;
+  scoped_ptr<InterceptorFactory> interceptor_factory_;
+
+  URLRequestPostInterceptor* post_interceptor_1;  // Owned by the factory.
+  URLRequestPostInterceptor* post_interceptor_2;  // Owned by the factory.
+
+  const net::URLFetcher* url_fetcher_source_;
+
+ private:
+  base::MessageLoopForIO loop_;
+  base::Closure quit_closure_;
+
+  DISALLOW_COPY_AND_ASSIGN(RequestSenderTest);
+};
+
+RequestSenderTest::RequestSenderTest()
+    : post_interceptor_1(NULL),
+      post_interceptor_2(NULL),
+      url_fetcher_source_(NULL) {
+  net::URLFetcher::SetEnableInterceptionForTests(true);
+}
+
+RequestSenderTest::~RequestSenderTest() {
+  net::URLFetcher::SetEnableInterceptionForTests(false);
+}
+
+void RequestSenderTest::SetUp() {
+  config_.reset(new TestConfigurator(base::MessageLoopProxy::current(),
+                                     base::MessageLoopProxy::current()));
+  interceptor_factory_.reset(
+      new InterceptorFactory(base::MessageLoopProxy::current()));
+  post_interceptor_1 =
+      interceptor_factory_->CreateInterceptorForPath(kUrlPath1);
+  post_interceptor_2 =
+      interceptor_factory_->CreateInterceptorForPath(kUrlPath2);
+  EXPECT_TRUE(post_interceptor_1);
+  EXPECT_TRUE(post_interceptor_2);
+
+  request_sender_.reset();
+}
+
+void RequestSenderTest::TearDown() {
+  request_sender_.reset();
+
+  post_interceptor_1 = NULL;
+  post_interceptor_2 = NULL;
+
+  interceptor_factory_.reset();
+
+  config_.reset();
+
+  RunThreadsUntilIdle();
+}
+
+void RequestSenderTest::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
+  // is set and the service is started.
+  RunThreadsUntilIdle();
+}
+
+void RequestSenderTest::RunThreadsUntilIdle() {
+  base::RunLoop().RunUntilIdle();
+}
+
+void RequestSenderTest::Quit() {
+  if (!quit_closure_.is_null())
+    quit_closure_.Run();
+}
+
+void RequestSenderTest::RequestSenderComplete(const net::URLFetcher* source) {
+  url_fetcher_source_ = source;
+  Quit();
+}
+
+// Tests that when a request to the first url succeeds, the subsequent urls are
+// not tried.
+TEST_F(RequestSenderTest, RequestSendSuccess) {
+  EXPECT_TRUE(post_interceptor_1->ExpectRequest(new PartialMatch("test")));
+
+  std::vector<GURL> urls;
+  urls.push_back(GURL(kUrl1));
+  urls.push_back(GURL(kUrl2));
+  request_sender_.reset(new RequestSender(*config_));
+  request_sender_->Send("test",
+                        urls,
+                        base::Bind(&RequestSenderTest::RequestSenderComplete,
+                                   base::Unretained(this)));
+  RunThreads();
+
+  EXPECT_EQ(1, post_interceptor_1->GetHitCount())
+      << post_interceptor_1->GetRequestsAsString();
+  EXPECT_EQ(1, post_interceptor_1->GetCount())
+      << post_interceptor_1->GetRequestsAsString();
+
+  EXPECT_STREQ("test", post_interceptor_1->GetRequests()[0].c_str());
+  EXPECT_EQ(GURL(kUrl1), url_fetcher_source_->GetOriginalURL());
+  EXPECT_EQ(200, url_fetcher_source_->GetResponseCode());
+}
+
+// Tests that the request succeeds using the second url after the first url
+// has failed.
+TEST_F(RequestSenderTest, RequestSendSuccessWithFallback) {
+  EXPECT_TRUE(post_interceptor_1->ExpectRequest(new PartialMatch("test"), 403));
+  EXPECT_TRUE(post_interceptor_2->ExpectRequest(new PartialMatch("test")));
+
+  std::vector<GURL> urls;
+  urls.push_back(GURL(kUrl1));
+  urls.push_back(GURL(kUrl2));
+  request_sender_.reset(new RequestSender(*config_));
+  request_sender_->Send("test",
+                        urls,
+                        base::Bind(&RequestSenderTest::RequestSenderComplete,
+                                   base::Unretained(this)));
+  RunThreads();
+
+  EXPECT_EQ(1, post_interceptor_1->GetHitCount())
+      << post_interceptor_1->GetRequestsAsString();
+  EXPECT_EQ(1, post_interceptor_1->GetCount())
+      << post_interceptor_1->GetRequestsAsString();
+  EXPECT_EQ(1, post_interceptor_2->GetHitCount())
+      << post_interceptor_2->GetRequestsAsString();
+  EXPECT_EQ(1, post_interceptor_2->GetCount())
+      << post_interceptor_2->GetRequestsAsString();
+
+  EXPECT_STREQ("test", post_interceptor_1->GetRequests()[0].c_str());
+  EXPECT_STREQ("test", post_interceptor_2->GetRequests()[0].c_str());
+  EXPECT_EQ(GURL(kUrl2), url_fetcher_source_->GetOriginalURL());
+  EXPECT_EQ(200, url_fetcher_source_->GetResponseCode());
+}
+
+// Tests that the request fails when both urls have failed.
+TEST_F(RequestSenderTest, RequestSendFailed) {
+  EXPECT_TRUE(post_interceptor_1->ExpectRequest(new PartialMatch("test"), 403));
+  EXPECT_TRUE(post_interceptor_2->ExpectRequest(new PartialMatch("test"), 403));
+
+  std::vector<GURL> urls;
+  urls.push_back(GURL(kUrl1));
+  urls.push_back(GURL(kUrl2));
+  request_sender_.reset(new RequestSender(*config_));
+  request_sender_->Send("test",
+                        urls,
+                        base::Bind(&RequestSenderTest::RequestSenderComplete,
+                                   base::Unretained(this)));
+  RunThreads();
+
+  EXPECT_EQ(1, post_interceptor_1->GetHitCount())
+      << post_interceptor_1->GetRequestsAsString();
+  EXPECT_EQ(1, post_interceptor_1->GetCount())
+      << post_interceptor_1->GetRequestsAsString();
+  EXPECT_EQ(1, post_interceptor_2->GetHitCount())
+      << post_interceptor_2->GetRequestsAsString();
+  EXPECT_EQ(1, post_interceptor_2->GetCount())
+      << post_interceptor_2->GetRequestsAsString();
+
+  EXPECT_STREQ("test", post_interceptor_1->GetRequests()[0].c_str());
+  EXPECT_STREQ("test", post_interceptor_2->GetRequests()[0].c_str());
+  EXPECT_EQ(GURL(kUrl2), url_fetcher_source_->GetOriginalURL());
+  EXPECT_EQ(403, url_fetcher_source_->GetResponseCode());
+}
+
+}  // namespace component_updater

components/component_updater/test/test_configurator.cc

@@ -2,16 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <string>
+#include "components/component_updater/test/test_configurator.h"
 
 #include "base/run_loop.h"
 #include "base/version.h"
 #include "components/component_updater/component_patcher_operation.h"
-#include "components/component_updater/test/test_configurator.h"
 #include "url/gurl.h"
 
 namespace component_updater {
 
+namespace {
+
+std::vector<GURL> MakeDefaultUrls() {
+  std::vector<GURL> urls;
+  urls.push_back(GURL(POST_INTERCEPT_SCHEME
+                      "://" POST_INTERCEPT_HOSTNAME POST_INTERCEPT_PATH));
+  return urls;
+}
+
+}  // namespace
+
 TestConfigurator::TestConfigurator(
     const scoped_refptr<base::SequencedTaskRunner>& worker_task_runner,
     const scoped_refptr<base::SingleThreadTaskRunner>& network_task_runner)
@@ -58,12 +68,11 @@ int TestConfigurator::OnDemandDelay() const {
   return ondemand_time_;
 }
 
-GURL TestConfigurator::UpdateUrl() const {
-  return GURL(POST_INTERCEPT_SCHEME
-              "://" POST_INTERCEPT_HOSTNAME POST_INTERCEPT_PATH);
+std::vector<GURL> TestConfigurator::UpdateUrl() const {
+  return MakeDefaultUrls();
 }
 
-GURL TestConfigurator::PingUrl() const {
+std::vector<GURL> TestConfigurator::PingUrl() const {
   return UpdateUrl();
 }
 

components/component_updater/test/test_configurator.h

@@ -7,6 +7,7 @@
 
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ -15,6 +16,8 @@
 #include "components/component_updater/component_updater_configurator.h"
 #include "net/url_request/url_request_test_util.h"
 
+class GURL;
+
 namespace component_updater {
 
 #define POST_INTERCEPT_SCHEME "https"
@@ -37,8 +40,8 @@ class TestConfigurator : public Configurator {
   virtual int StepDelayMedium() OVERRIDE;
   virtual int MinimumReCheckWait() const OVERRIDE;
   virtual int OnDemandDelay() const OVERRIDE;
-  virtual GURL UpdateUrl() const OVERRIDE;
-  virtual GURL PingUrl() const OVERRIDE;
+  virtual std::vector<GURL> UpdateUrl() const OVERRIDE;
+  virtual std::vector<GURL> PingUrl() const OVERRIDE;
   virtual base::Version GetBrowserVersion() const OVERRIDE;
   virtual std::string GetChannel() const OVERRIDE;
   virtual std::string GetLang() const OVERRIDE;
@@ -55,7 +58,6 @@ class TestConfigurator : public Configurator {
   virtual scoped_refptr<base::SingleThreadTaskRunner>
       GetSingleThreadTaskRunner() const OVERRIDE;
 
-  typedef std::pair<CrxComponent*, int> CheckAtLoopCount;
   void SetLoopCount(int times);
   void SetRecheckTime(int seconds);
   void SetOnDemandTime(int seconds);

components/component_updater/test/update_checker_unittest.cc

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "base/basictypes.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_util.h"
 #include "base/memory/ref_counted.h"
@@ -18,6 +20,7 @@
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
 
 namespace component_updater {
 
@@ -41,7 +44,8 @@ class UpdateCheckerTest : public testing::Test {
   virtual void SetUp() OVERRIDE;
   virtual void TearDown() OVERRIDE;
 
-  void UpdateCheckComplete(int error,
+  void UpdateCheckComplete(const GURL& original_url,
+                           int error,
                            const std::string& error_message,
                            const UpdateResponse::Results& results);
 
@@ -59,6 +63,7 @@ class UpdateCheckerTest : public testing::Test {
   scoped_ptr<InterceptorFactory> interceptor_factory_;
   URLRequestPostInterceptor* post_interceptor_;  // Owned by the factory.
 
+  GURL original_url_;
   int error_;
   std::string error_message_;
   UpdateResponse::Results results_;
@@ -102,7 +107,7 @@ void UpdateCheckerTest::TearDown() {
   config_.reset();
 
   // The PostInterceptor requires the message loop to run to destruct correctly.
-  // TODO: This is fragile and should be fixed.
+  // TODO(sorin): This is fragile and should be fixed.
   RunThreadsUntilIdle();
 }
 
@@ -128,9 +133,11 @@ void UpdateCheckerTest::Quit() {
 }
 
 void UpdateCheckerTest::UpdateCheckComplete(
+    const GURL& original_url,
     int error,
     const std::string& error_message,
     const UpdateResponse::Results& results) {
+  original_url_ = original_url;
   error_ = error;
   error_message_ = error_message;
   results_ = results;
@@ -157,16 +164,17 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccess) {
   EXPECT_TRUE(post_interceptor_->ExpectRequest(
       new PartialMatch("updatecheck"), test_file("updatecheck_reply_1.xml")));
 
-  update_checker_ =
-      UpdateChecker::Create(*config_,
-                            base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
-                                       base::Unretained(this))).Pass();
+  update_checker_ = UpdateChecker::Create(*config_).Pass();
 
   CrxUpdateItem item(BuildCrxUpdateItem());
   std::vector<CrxUpdateItem*> items_to_check;
   items_to_check.push_back(&item);
 
-  update_checker_->CheckForUpdates(items_to_check, "extra=\"params\"");
+  update_checker_->CheckForUpdates(
+      items_to_check,
+      "extra=\"params\"",
+      base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
+                 base::Unretained(this)));
 
   RunThreads();
 
@@ -190,6 +198,7 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccess) {
             post_interceptor_->GetRequests()[0].find("<hw physmemory="));
 
   // Sanity check the arguments of the callback after parsing.
+  EXPECT_EQ(config_->UpdateUrl().front(), original_url_);
   EXPECT_EQ(0, error_);
   EXPECT_TRUE(error_message_.empty());
   EXPECT_EQ(1ul, results_.list.size());
@@ -198,33 +207,32 @@ TEST_F(UpdateCheckerTest, UpdateCheckSuccess) {
   EXPECT_STREQ("1.0", results_.list[0].manifest.version.c_str());
 }
 
-TEST_F(UpdateCheckerTest, UpdateNetworkError) {
-  // Setting this expectation simulates a network error since the
-  // file is not found. Since setting the expectation fails, this function
-  // owns |request_matcher|.
-  scoped_ptr<PartialMatch> request_matcher(new PartialMatch("updatecheck"));
-  EXPECT_FALSE(post_interceptor_->ExpectRequest(request_matcher.get(),
-                                                test_file("no such file")));
+// Simulates a 403 server response error.
+TEST_F(UpdateCheckerTest, UpdateCheckError) {
+  EXPECT_TRUE(
+      post_interceptor_->ExpectRequest(new PartialMatch("updatecheck"), 403));
 
-  update_checker_ =
-      UpdateChecker::Create(*config_,
-                            base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
-                                       base::Unretained(this))).Pass();
+  update_checker_ = UpdateChecker::Create(*config_).Pass();
 
   CrxUpdateItem item(BuildCrxUpdateItem());
   std::vector<CrxUpdateItem*> items_to_check;
   items_to_check.push_back(&item);
 
-  update_checker_->CheckForUpdates(items_to_check, "");
+  update_checker_->CheckForUpdates(
+      items_to_check,
+      "",
+      base::Bind(&UpdateCheckerTest::UpdateCheckComplete,
+                 base::Unretained(this)));
 
   RunThreads();
 
-  EXPECT_EQ(0, post_interceptor_->GetHitCount())
+  EXPECT_EQ(1, post_interceptor_->GetHitCount())
       << post_interceptor_->GetRequestsAsString();
   EXPECT_EQ(1, post_interceptor_->GetCount())
       << post_interceptor_->GetRequestsAsString();
 
-  EXPECT_NE(0, error_);
+  EXPECT_EQ(config_->UpdateUrl().front(), original_url_);
+  EXPECT_EQ(403, error_);
   EXPECT_STREQ("network error", error_message_.c_str());
   EXPECT_EQ(0ul, results_.list.size());
 }

components/component_updater/test/url_request_post_interceptor.cc

@@ -22,12 +22,14 @@ class URLRequestMockJob : public net::URLRequestSimpleJob {
  public:
   URLRequestMockJob(net::URLRequest* request,
                     net::NetworkDelegate* network_delegate,
-                    const std::string& response)
+                    int response_code,
+                    const std::string& response_body)
       : net::URLRequestSimpleJob(request, network_delegate),
-        response_(response) {}
+        response_code_(response_code),
+        response_body_(response_body) {}
 
  protected:
-  virtual int GetResponseCode() const OVERRIDE { return 200; }
+  virtual int GetResponseCode() const OVERRIDE { return response_code_; }
 
   virtual int GetData(std::string* mime_type,
                       std::string* charset,
@@ -35,14 +37,15 @@ class URLRequestMockJob : public net::URLRequestSimpleJob {
                       const net::CompletionCallback& callback) const OVERRIDE {
     mime_type->assign("text/plain");
     charset->assign("US-ASCII");
-    data->assign(response_);
+    data->assign(response_body_);
     return net::OK;
   }
 
  private:
   virtual ~URLRequestMockJob() {}
 
-  std::string response_;
+  int response_code_;
+  std::string response_body_;
   DISALLOW_COPY_AND_ASSIGN(URLRequestMockJob);
 };
 
@@ -71,7 +74,16 @@ GURL URLRequestPostInterceptor::GetUrl() const {
 
 bool URLRequestPostInterceptor::ExpectRequest(
     class RequestMatcher* request_matcher) {
-  expectations_.push(std::make_pair(request_matcher, ""));
+  expectations_.push(std::make_pair(request_matcher,
+                                    ExpectationResponse(kResponseCode200, "")));
+  return true;
+}
+
+bool URLRequestPostInterceptor::ExpectRequest(
+    class RequestMatcher* request_matcher,
+    int response_code) {
+  expectations_.push(
+      std::make_pair(request_matcher, ExpectationResponse(response_code, "")));
   return true;
 }
 
@@ -81,7 +93,9 @@ bool URLRequestPostInterceptor::ExpectRequest(
   std::string response;
   if (filepath.empty() || !base::ReadFileToString(filepath, &response))
     return false;
-  expectations_.push(std::make_pair(request_matcher, response));
+
+  expectations_.push(std::make_pair(
+      request_matcher, ExpectationResponse(kResponseCode200, response)));
   return true;
 }
 
@@ -147,7 +161,7 @@ class URLRequestPostInterceptor::Delegate : public net::URLRequestInterceptor {
 
   void OnCreateInterceptor(URLRequestPostInterceptor* interceptor) {
     DCHECK(io_task_runner_->RunsTasksOnCurrentThread());
-    CHECK(interceptors_.find(interceptor->GetUrl()) == interceptors_.end());
+    DCHECK(interceptors_.find(interceptor->GetUrl()) == interceptors_.end());
 
     interceptors_.insert(std::make_pair(interceptor->GetUrl(), interceptor));
   }
@@ -196,12 +210,14 @@ class URLRequestPostInterceptor::Delegate : public net::URLRequestInterceptor {
       const URLRequestPostInterceptor::Expectation& expectation(
           interceptor->expectations_.front());
       if (expectation.first->Match(request_body)) {
-        const std::string response(expectation.second);
+        const int response_code(expectation.second.response_code);
+        const std::string response_body(expectation.second.response_body);
         delete expectation.first;
         interceptor->expectations_.pop();
         ++interceptor->hit_count_;
 
-        return new URLRequestMockJob(request, network_delegate, response);
+        return new URLRequestMockJob(
+            request, network_delegate, response_code, response_body);
       }
     }
 
@@ -276,8 +292,13 @@ InterceptorFactory::~InterceptorFactory() {
 }
 
 URLRequestPostInterceptor* InterceptorFactory::CreateInterceptor() {
+  return CreateInterceptorForPath(POST_INTERCEPT_PATH);
+}
+
+URLRequestPostInterceptor* InterceptorFactory::CreateInterceptorForPath(
+    const char* url_path) {
   return URLRequestPostInterceptorFactory::CreateInterceptor(
-      base::FilePath::FromUTF8Unsafe(POST_INTERCEPT_PATH));
+      base::FilePath::FromUTF8Unsafe(url_path));
 }
 
 }  // namespace component_updater

components/component_updater/test/url_request_post_interceptor.h

@@ -66,9 +66,12 @@ class URLRequestPostInterceptor {
   // Sets an expection for the body of the POST request and optionally,
   // provides a canned response identified by a |file_path| to be returned when
   // the expectation is met. If no |file_path| is provided, then an empty
-  // response body is served. This class takes ownership of the
-  // |request_matcher| object. Returns |true| if the expectation was set.
+  // response body is served. If |response_code| is provided, then an empty
+  // response body with that response code is returned.
+  // Returns |true| if the expectation was set. This class takes ownership of
+  // the |request_matcher| object.
   bool ExpectRequest(class RequestMatcher* request_matcher);
+  bool ExpectRequest(class RequestMatcher* request_matcher, int response_code);
   bool ExpectRequest(class RequestMatcher* request_matcher,
                      const base::FilePath& filepath);
 
@@ -92,7 +95,16 @@ class URLRequestPostInterceptor {
 
  private:
   friend class URLRequestPostInterceptorFactory;
-  typedef std::pair<const RequestMatcher*, std::string> Expectation;
+
+  static const int kResponseCode200 = 200;
+
+  struct ExpectationResponse {
+    ExpectationResponse(int code, const std::string& body)
+        : response_code(code), response_body(body) {}
+    const int response_code;
+    const std::string response_body;
+  };
+  typedef std::pair<const RequestMatcher*, ExpectationResponse> Expectation;
 
   URLRequestPostInterceptor(
       const GURL& url,
@@ -100,6 +112,7 @@ class URLRequestPostInterceptor {
   ~URLRequestPostInterceptor();
 
   void ClearExpectations();
+
   const GURL url_;
   scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
 
@@ -144,8 +157,12 @@ class InterceptorFactory : public URLRequestPostInterceptorFactory {
       const scoped_refptr<base::SequencedTaskRunner>& io_task_runner);
   ~InterceptorFactory();
 
+  // Creates an interceptor for the url path defined by POST_INTERCEPT_PATH.
   URLRequestPostInterceptor* CreateInterceptor();
 
+  // Creates an interceptor for the given url path.
+  URLRequestPostInterceptor* CreateInterceptorForPath(const char* url_path);
+
  private:
   DISALLOW_COPY_AND_ASSIGN(InterceptorFactory);
 };

components/component_updater/update_checker.cc

@@ -4,19 +4,28 @@
 
 #include "components/component_updater/update_checker.h"
 
+#include <string>
+#include <vector>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/compiler_specific.h"
+#include "base/location.h"
 #include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_checker.h"
 #include "components/component_updater/component_updater_configurator.h"
 #include "components/component_updater/component_updater_utils.h"
 #include "components/component_updater/crx_update_item.h"
+#include "components/component_updater/request_sender.h"
 #include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_delegate.h"
 #include "url/gurl.h"
 
 namespace component_updater {
 
+namespace {
+
 // Builds an update check request for |components|. |additional_attributes| is
 // serialized as part of the <request> element of the request to customize it
 // with data that is not platform or component specific. For each |item|, a
@@ -66,43 +75,31 @@ std::string BuildUpdateCheckRequest(const Configurator& config,
                               additional_attributes);
 }
 
-class UpdateCheckerImpl : public UpdateChecker, public net::URLFetcherDelegate {
+class UpdateCheckerImpl : public UpdateChecker {
  public:
-  UpdateCheckerImpl(const Configurator& config,
-                    const UpdateCheckCallback& update_check_callback);
+  explicit UpdateCheckerImpl(const Configurator& config);
   virtual ~UpdateCheckerImpl();
 
   // Overrides for UpdateChecker.
   virtual bool CheckForUpdates(
       const std::vector<CrxUpdateItem*>& items_to_check,
-      const std::string& additional_attributes) OVERRIDE;
-
-  // Overrides for UrlFetcher.
-  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+      const std::string& additional_attributes,
+      const UpdateCheckCallback& update_check_callback) OVERRIDE;
 
  private:
-  const Configurator& config_;
-  const UpdateCheckCallback update_check_callback_;
+  void OnRequestSenderComplete(const net::URLFetcher* source);
 
-  scoped_ptr<net::URLFetcher> url_fetcher_;
+  const Configurator& config_;
+  UpdateCheckCallback update_check_callback_;
+  scoped_ptr<RequestSender> request_sender_;
 
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(UpdateCheckerImpl);
 };
 
-scoped_ptr<UpdateChecker> UpdateChecker::Create(
-    const Configurator& config,
-    const UpdateCheckCallback& update_check_callback) {
-  scoped_ptr<UpdateCheckerImpl> update_checker(
-      new UpdateCheckerImpl(config, update_check_callback));
-  return update_checker.PassAs<UpdateChecker>();
-}
-
-UpdateCheckerImpl::UpdateCheckerImpl(
-    const Configurator& config,
-    const UpdateCheckCallback& update_check_callback)
-    : config_(config), update_check_callback_(update_check_callback) {
+UpdateCheckerImpl::UpdateCheckerImpl(const Configurator& config)
+    : config_(config) {
 }
 
 UpdateCheckerImpl::~UpdateCheckerImpl() {
@@ -111,24 +108,31 @@ UpdateCheckerImpl::~UpdateCheckerImpl() {
 
 bool UpdateCheckerImpl::CheckForUpdates(
     const std::vector<CrxUpdateItem*>& items_to_check,
-    const std::string& additional_attributes) {
+    const std::string& additional_attributes,
+    const UpdateCheckCallback& update_check_callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (url_fetcher_)
-    return false;  // Another fetch is in progress.
+  if (request_sender_.get()) {
+    NOTREACHED();
+    return false;  // Another update check is in progress.
+  }
 
-  url_fetcher_.reset(SendProtocolRequest(
-      config_.UpdateUrl(),
-      BuildUpdateCheckRequest(config_, items_to_check, additional_attributes),
-      this,
-      config_.RequestContext()));
+  update_check_callback_ = update_check_callback;
 
+  request_sender_.reset(new RequestSender(config_));
+  request_sender_->Send(
+      BuildUpdateCheckRequest(config_, items_to_check, additional_attributes),
+      config_.UpdateUrl(),
+      base::Bind(&UpdateCheckerImpl::OnRequestSenderComplete,
+                 base::Unretained(this)));
   return true;
 }
 
-void UpdateCheckerImpl::OnURLFetchComplete(const net::URLFetcher* source) {
+void UpdateCheckerImpl::OnRequestSenderComplete(const net::URLFetcher* source) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(url_fetcher_.get() == source);
+
+  const GURL original_url(source->GetOriginalURL());
+  VLOG(1) << "Update check request went to: " << original_url.spec();
 
   int error = 0;
   std::string error_message;
@@ -148,8 +152,15 @@ void UpdateCheckerImpl::OnURLFetchComplete(const net::URLFetcher* source) {
     VLOG(1) << "Update request failed: network error";
   }
 
-  url_fetcher_.reset();
-  update_check_callback_.Run(error, error_message, update_response.results());
+  request_sender_.reset();
+  update_check_callback_.Run(
+      original_url, error, error_message, update_response.results());
+}
+
+}  // namespace
+
+scoped_ptr<UpdateChecker> UpdateChecker::Create(const Configurator& config) {
+  return scoped_ptr<UpdateChecker>(new UpdateCheckerImpl(config));
 }
 
 }  // namespace component_updater

components/component_updater/update_checker.h

@@ -12,6 +12,7 @@
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "components/component_updater/update_response.h"
+#include "url/gurl.h"
 
 class GURL;
 
@@ -26,7 +27,8 @@ struct CrxUpdateItem;
 
 class UpdateChecker {
  public:
-  typedef base::Callback<void(int error,
+  typedef base::Callback<void(const GURL& original_url,
+                              int error,
                               const std::string& error_message,
                               const UpdateResponse::Results& results)>
       UpdateCheckCallback;
@@ -38,11 +40,10 @@ class UpdateChecker {
   // as-is, therefore it must be well-formed as an XML attribute string.
   virtual bool CheckForUpdates(
       const std::vector<CrxUpdateItem*>& items_to_check,
-      const std::string& additional_attributes) = 0;
+      const std::string& additional_attributes,
+      const UpdateCheckCallback& update_check_callback) = 0;
 
-  static scoped_ptr<UpdateChecker> Create(
-      const Configurator& config,
-      const UpdateCheckCallback& update_check_callback);
+  static scoped_ptr<UpdateChecker> Create(const Configurator& config);
 
  protected:
   UpdateChecker() {}

components/components_tests.gyp

@@ -79,6 +79,7 @@
             'component_updater/test/component_patcher_unittest.cc',
             'component_updater/test/component_updater_ping_manager_unittest.cc',
             'component_updater/test/crx_downloader_unittest.cc',
+            'component_updater/test/request_sender_unittest.cc',
             'component_updater/test/update_checker_unittest.cc',
             'component_updater/test/update_response_unittest.cc',
             'content_settings/core/common/content_settings_pattern_parser_unittest.cc',
@@ -296,7 +297,7 @@
 
             # Dependencies of cloud_devices
             'components.gyp:cloud_devices_common',
-            
+
             # Dependencies of component_updater
             'components.gyp:component_updater',
             'components.gyp:component_updater_test_support',