Implement a background downloader using BITS in Windows Chrome.

chrome/browser/component_updater/crx_downloader.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright (c) 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 "chrome/browser/component_updater/crx_downloader.h"
 #include "chrome/browser/component_updater/url_fetcher_downloader.h"
 #include "content/public/browser/browser_thread.h"
 
+#if defined(OS_WIN)
+#include "chrome/browser/component_updater/background_downloader_win.h"
+#endif
+
 using content::BrowserThread;
 
 namespace component_updater {
 
-// This factory method builds the chain of downloaders. Currently, there is only
-// a url fetcher downloader but more downloaders can be chained up to handle
-// the request.
+// On Windows, the first downloader in the chain is a background downloader,
+// which uses the BITS service.
 CrxDownloader* CrxDownloader::Create(
+    bool is_background_download,
     net::URLRequestContextGetter* context_getter,
     scoped_refptr<base::SequencedTaskRunner> task_runner,
     const DownloadCallback& download_callback) {
-  CrxDownloader* crx_downloader =
-      new UrlFetcherDownloader(context_getter, task_runner);
+  scoped_ptr<CrxDownloader> url_fetcher_downloader(
+      new UrlFetcherDownloader(scoped_ptr<CrxDownloader>().Pass(),
+                               context_getter,
+                               task_runner,
+                               download_callback));
+#if defined (OS_WIN)
+  if (is_background_download) {
+    return new BackgroundDownloader(url_fetcher_downloader.Pass(),
+                                    context_getter,
+                                    task_runner,
+                                    download_callback);
+  }
+#endif
 
-  crx_downloader->download_callback_ = download_callback;
-
-  return crx_downloader;
+  return url_fetcher_downloader.release();
 }
 
-CrxDownloader::CrxDownloader() : current_url_(0) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+CrxDownloader::CrxDownloader(
+    scoped_ptr<CrxDownloader> successor,
+    const DownloadCallback& download_callback)
+    : is_active_(false),
+      successor_(successor.Pass()),
+      download_callback_(download_callback) {
+      DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
 CrxDownloader::~CrxDownloader() {
 }
 
-void CrxDownloader::StartDownloadFromUrl(const GURL& url) {
+bool CrxDownloader::StartDownloadFromUrl(const GURL& url) {
   std::vector<GURL> urls;
   urls.push_back(url);
-  StartDownload(urls);
+  return StartDownload(urls);
 }
 
-void CrxDownloader::StartDownload(const std::vector<GURL>& urls) {
+bool CrxDownloader::StartDownload(const std::vector<GURL>& urls) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  if (urls.empty())
-    return;
+  if (urls.empty() || is_active_)
+    return false;
+
+  // |is_active_| defensively protects against mutating the container of urls
+  // once the download has started.
+  is_active_ =  true;
 
   urls_ = urls;
+  current_url_ = urls_.begin();
 
-  current_url_ = 0;
-  DoStartDownload(urls[current_url_]);
+  DoStartDownload(*current_url_);
+  return true;
 }
 
-// Handles the fallback in the case of multiple urls and routing of the
-// download to the following successor in the chain.
-void CrxDownloader::OnDownloadComplete(bool is_handled,
-                                       int error,
-                                       const base::FilePath& response) {
+void CrxDownloader::OnDownloadComplete(bool is_handled, const Result& result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  // If an error has occured, try the next url if possible, then move on
-  // to the successor in the chain, if the request has not been handled
-  if (error) {
-    ++current_url_;
-    if (current_url_ != urls_.size()) {
-      DoStartDownload(urls_[current_url_]);
+  // If an error has occured, try the next url if there is any, then move on
+  // to the successor in the chain if the request has not been handled.
+  // In the case of an error, and if the downloader has received a 5xx error, as
+  // indicated by the |is_handled| flag, remove that url from the list of
+  // urls so the url is not handed over to the successor.
+  if (result.error) {
+    if (!is_handled)
+      ++current_url_;
+    else
+      current_url_ = urls_.erase(current_url_);
+
+    if (current_url_ != urls_.end()) {
+      DoStartDownload(*current_url_);
       return;
     }
 
     if (!is_handled && successor_) {
       successor_->StartDownload(urls_);

[waffles, 2013/12/05 01:15:07]

What if this guy returns false?

I think this won't happen with the way the code is now, but maybe better to be
safe than sorry?

[Sorin Jianu, 2013/12/05 01:43:07]

I will like to get rid of the is_active_ defensive programming too, so that
becomes a  moot point.

       return;
     }
   }
 
-  DCHECK(is_handled || !error || !successor_);
+  DCHECK(is_handled || !result.error || !successor_);
 
-  download_callback_.Run(error, response);
+  download_callback_.Run(result);
 }
 
 }  // namespace component_updater