Implement support for fallback update check urls in the component updater

components/component_updater/update_checker.cc

Index: components/component_updater/update_checker.cc
diff --git a/components/component_updater/update_checker.cc b/components/component_updater/update_checker.cc
index 49b2821e7972dcd6117896f863ed55c7a71e2185..93a23f2a767d114e24f6d11ebd41f9841049e8bf 100644
--- a/components/component_updater/update_checker.cc
+++ b/components/component_updater/update_checker.cc
@@ -4,19 +4,26 @@
 
 #include "components/component_updater/update_checker.h"
 
+#include <string>
+#include <vector>
+
 #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 +73,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 +106,30 @@ 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();

[waffles, 2014/09/12 22:38:26]

Should we really have NOTREACHED here? DVLOG might be better.

[Sorin Jianu, 2014/09/12 23:24:00]

as discussed, the code stays, we should not be overlapping update checks.

+    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_, config_.UpdateUrl()));
+  request_sender_->Send(
+      BuildUpdateCheckRequest(config_, items_to_check, additional_attributes),
+      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 +149,16 @@ 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) {
+  scoped_ptr<UpdateCheckerImpl> update_checker(new UpdateCheckerImpl(config));
+  return update_checker.PassAs<UpdateChecker>();
 }
 
 }  // namespace component_updater