DownloadUrlParameters

content/browser/download/download_manager_impl.cc

Index: content/browser/download/download_manager_impl.cc
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 9bdb48aa7bfbe2382d1e3b34056dbf0f6c79d308..2919cb4e8e94f60e65d70cb9242fe770957655e3 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -30,10 +30,12 @@
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_manager_delegate.h"
 #include "content/public/browser/download_persistent_store_info.h"
+#include "content/public/browser/download_url_parameters.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents_delegate.h"
+#include "net/base/load_flags.h"
 #include "net/base/upload_data.h"
 
 // TODO(benjhayden): Change this to DCHECK when we have more debugging
@@ -55,52 +57,64 @@ using content::WebContents;
 
 namespace {
 
-// Param structs exist because base::Bind can only handle 6 args.
-struct URLParams {
-  URLParams(const GURL& url, const GURL& referrer, int64 post_id, bool cache)
-    : url_(url), referrer_(referrer), post_id_(post_id), prefer_cache_(cache) {}
-  GURL url_;
-  GURL referrer_;
-  int64 post_id_;
-  bool prefer_cache_;
-};
-
-struct RenderParams {
-  RenderParams(int rpi, int rvi)
-    : render_process_id_(rpi), render_view_id_(rvi) {}
-  int render_process_id_;
-  int render_view_id_;
-};
-
-void BeginDownload(
-    const URLParams& url_params,
-    const content::DownloadSaveInfo& save_info,
-    ResourceDispatcherHostImpl* resource_dispatcher_host,
-    const RenderParams& render_params,
-    content::ResourceContext* context,
-    const content::DownloadManager::OnStartedCallback& callback) {
-  scoped_ptr<net::URLRequest> request(
-      new net::URLRequest(url_params.url_, resource_dispatcher_host));
-  request->set_referrer(url_params.referrer_.spec());
-  if (url_params.post_id_ >= 0) {
+static const char kHeaderNameKey[] = "name";
+static const char kHeaderBinaryValueKey[] = "binaryValue";
+static const char kHeaderValueKey[] = "value";
+
+void BeginDownload(content::DownloadUrlParameters* params) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  scoped_ptr<net::URLRequest> request(new net::URLRequest(
+      params->url(), params->resource_dispatcher_host()));
+  request->set_referrer(params->referrer().spec());
+  request->set_load_flags(request->load_flags() | params->load_flags());
+  request->set_method(params->method());
+  if (!params->post_body().empty())
+    request->AppendBytesToUpload(params->post_body().data(),
+                                 params->post_body().size());
+  if (params->post_id() >= 0) {
     // The POST in this case does not have an actual body, and only works
     // when retrieving data from cache. This is done because we don't want
     // to do a re-POST without user consent, and currently don't have a good
     // plan on how to display the UI for that.
-    DCHECK(url_params.prefer_cache_);
-    request->set_method("POST");
+    DCHECK(params->prefer_cache());
+    DCHECK(params->method() == "POST");
     scoped_refptr<net::UploadData> upload_data = new net::UploadData();
-    upload_data->set_identifier(url_params.post_id_);
+    upload_data->set_identifier(params->post_id());
     request->set_upload(upload_data);
   }
-  resource_dispatcher_host->BeginDownload(
+  if (params->extra_headers() != NULL) {
+    for (size_t index = 0;
+         index < params->extra_headers()->GetSize();
+         ++index) {
+      base::DictionaryValue* header = NULL;
+      std::string name, value;
+      CHECK(params->extra_headers()->GetDictionary(index, &header));
+      CHECK(header->GetString(kHeaderNameKey, &name));
+      if (header->HasKey(kHeaderBinaryValueKey)) {
+        base::ListValue* binary_value = NULL;
+        CHECK(header->GetList(kHeaderBinaryValueKey, &binary_value));
+        for (size_t char_i = 0; char_i < binary_value->GetSize(); ++char_i) {
+          int char_value = 0;
+          CHECK(binary_value->GetInteger(char_i, &char_value));
+          if ((0 <= char_value) &&
+              (char_value <= 0xff)) {
+            value.push_back(char_value);
+          }
+        }
+      } else if (header->HasKey(kHeaderValueKey)) {
+        CHECK(header->GetString(kHeaderValueKey, &value));
+      }
+      request->SetExtraRequestHeaderByName(name, value, false/*overwrite*/);
+    }
+  }

[Randy Smith (Not in Mondays), 2012/04/30 19:44:48]

I'm not sure I completely follow this format, but it looks complex for this
level fo the system--it looks more "extension-y".  Is there a reason not to have
a list or vector of pairs of std::string s, representing a list of header/value
pairs?

[benjhayden, 2012/05/02 15:11:54]

Done.

+  params->resource_dispatcher_host()->BeginDownload(
       request.Pass(),
-      context,
-      render_params.render_process_id_,
-      render_params.render_view_id_,
-      url_params.prefer_cache_,
-      save_info,
-      callback);
+      params->resource_context(),
+      params->render_process_host_id(),
+      params->render_view_host_routing_id(),
+      params->prefer_cache(),
+      params->save_info(),
+      params->callback());
 }
 
 class MapValueIteratorAdapter {
@@ -148,6 +162,43 @@ void EnsureNoPendingDownloadJobsOnIO(bool* result) {
 
 namespace content {
 
+DownloadUrlParameters::DownloadUrlParameters(
+    const GURL& url,
+    int render_process_host_id,
+    int render_view_host_routing_id,
+    content::ResourceContext* resource_context,
+    const content::DownloadSaveInfo& save_info)
+  : extra_headers_(NULL),
+    load_flags_(0),
+    method_("GET"),
+    post_id_(-1),
+    prefer_cache_(false),
+    render_process_host_id_(render_process_host_id),
+    render_view_host_routing_id_(render_view_host_routing_id),
+    resource_context_(resource_context),
+    resource_dispatcher_host_(ResourceDispatcherHostImpl::Get()),
+    save_info_(save_info),
+    url_(url) {
+  DCHECK(resource_dispatcher_host_);
+}
+
+DownloadUrlParameters::~DownloadUrlParameters() {
+}
+
+// static
+DownloadUrlParameters* DownloadUrlParameters::FromWebContents(
+    WebContents* web_contents,
+    const GURL& url,
+    const content::DownloadSaveInfo& save_info) {
+  return new DownloadUrlParameters(
+      url,
+      web_contents->GetRenderProcessHost()->GetID(),
+      web_contents->GetRenderViewHost()->GetRoutingID(),
+      web_contents->GetBrowserContext()->GetResourceContext(),
+      save_info);
+}
+
+
 // static
 DownloadManager* DownloadManager::Create(
     content::DownloadManagerDelegate* delegate,
@@ -850,36 +901,10 @@ int DownloadManagerImpl::RemoveAllDownloads() {
   return RemoveDownloadsBetween(base::Time(), base::Time());
 }
 
-// Initiate a download of a specific URL. We send the request to the
-// ResourceDispatcherHost, and let it send us responses like a regular
-// download.
 void DownloadManagerImpl::DownloadUrl(
-    const GURL& url,
-    const GURL& referrer,
-    const std::string& referrer_charset,
-    bool prefer_cache,
-    int64 post_id,
-    const content::DownloadSaveInfo& save_info,
-    WebContents* web_contents,
-    const OnStartedCallback& callback) {
-  ResourceDispatcherHostImpl* resource_dispatcher_host =
-      ResourceDispatcherHostImpl::Get();
-  DCHECK(resource_dispatcher_host);
-
-  // We send a pointer to content::ResourceContext, instead of the usual
-  // reference, so that a copy of the object isn't made.
-  // base::Bind can't handle 7 args, so we use URLParams and RenderParams.
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(
-          &BeginDownload,
-          URLParams(url, referrer, post_id, prefer_cache),
-          save_info,
-          resource_dispatcher_host,
-          RenderParams(web_contents->GetRenderProcessHost()->GetID(),
-                       web_contents->GetRenderViewHost()->GetRoutingID()),
-          web_contents->GetBrowserContext()->GetResourceContext(),
-          callback));
+    content::DownloadUrlParameters* parameters) {
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE, base::Bind(
+      &BeginDownload, base::Owned(parameters)));
 }
 
 void DownloadManagerImpl::AddObserver(Observer* observer) {