Upload support in download manager

DownloadManager today has some minimal support to upload a byte array.
We are extending it to upload blobs in this CL. The client will embed
the blob data inside the post body (ResourceRequestBody) of the
DownloadUrlParameters which will be subsequently passed to the
URLRequest/ResourceRequest.

Bug: 812327
Change-Id: Ia230961294d3bdbcf6284fde0e078c5844a991bf
Reviewed-on: https://chromium-review.googlesource.com/919622Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537225}

chrome/browser/extensions/api/downloads/downloads_api.cc

@@ -1074,8 +1074,12 @@ bool DownloadsDownloadFunction::RunAsync() {
       downloads::ToString(options.method);
   if (!method_string.empty())
     download_params->set_method(method_string);
-  if (options.body.get())
-    download_params->set_post_body(*options.body);
+  if (options.body.get()) {
+    download_params->set_post_body(
+        network::ResourceRequestBody::CreateFromBytes(options.body->data(),
+                                                      options.body->size()));
+  }
+
   download_params->set_callback(base::Bind(
       &DownloadsDownloadFunction::OnStarted, this,
       creator_suggested_filename, options.conflict_action));

components/download/public/common/BUILD.gn

@@ -37,6 +37,8 @@ component("public") {
     "//components/download/internal/common:internal",
     "//crypto",
     "//net",
+    "//services/network/public/cpp",
+    "//storage/browser",
     "//ui/base",
   ]
 

components/download/public/common/DEPS

@@ -4,5 +4,7 @@ include_rules = [
   # TODO(qinmin): remove this once network service is enabled.
   "+net/url_request/url_request_context_getter.h",
   "+net/traffic_annotation/network_traffic_annotation.h",
+  "+services/network/public/cpp",
+  "+storage/browser",
   "+ui/base",
 ]

components/download/public/common/download_url_parameters.h

@@ -22,6 +22,8 @@
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/resource_request_body.h"
+#include "storage/browser/blob/blob_data_handle.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -61,6 +63,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
   typedef std::pair<std::string, std::string> RequestHeadersNameValuePair;
   typedef std::vector<RequestHeadersNameValuePair> RequestHeadersType;
 
+  using BlobStorageContextGetter =
+      base::OnceCallback<storage::BlobStorageContext*()>;
+
   // Constructs a download not associated with a frame.
   //
   // It is not safe to have downloads not associated with a frame and
@@ -131,7 +136,14 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
   void set_method(const std::string& method) { method_ = method; }
 
   // Body of the HTTP POST request.
-  void set_post_body(const std::string& post_body) { post_body_ = post_body; }
+  void set_post_body(scoped_refptr<network::ResourceRequestBody> post_body) {
+    post_body_ = post_body;
+  }
+
+  // The blob storage context to be used for uploading blobs, if any.
+  void set_blob_storage_context_getter(BlobStorageContextGetter blob_getter) {
+    blob_storage_context_getter_ = std::move(blob_getter);
+  }
 
   // If |prefer_cache| is true and the response to |url| is in the HTTP cache,
   // it will be used without validation. If |method| is POST, then |post_id_|
@@ -227,7 +239,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
   const std::string& etag() const { return etag_; }
   bool use_if_range() const { return use_if_range_; }
   const std::string& method() const { return method_; }
-  const std::string& post_body() const { return post_body_; }
+  scoped_refptr<network::ResourceRequestBody> post_body() { return post_body_; }
   int64_t post_id() const { return post_id_; }
   bool prefer_cache() const { return prefer_cache_; }
   const GURL& referrer() const { return referrer_; }
@@ -237,6 +249,9 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
   const std::string& referrer_encoding() const { return referrer_encoding_; }
   const base::Optional<url::Origin>& initiator() const { return initiator_; }
   const std::string& request_origin() const { return request_origin_; }
+  BlobStorageContextGetter get_blob_storage_context_getter() {
+    return std::move(blob_storage_context_getter_);
+  }
 
   // These will be -1 if the request is not associated with a frame. See
   // the constructors for more.
@@ -286,7 +301,8 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
   std::string etag_;
   bool use_if_range_;
   std::string method_;
-  std::string post_body_;
+  scoped_refptr<network::ResourceRequestBody> post_body_;
+  BlobStorageContextGetter blob_storage_context_getter_;
   int64_t post_id_;
   bool prefer_cache_;
   GURL referrer_;

content/browser/download/download_browsertest.cc

@@ -32,7 +32,6 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "components/download/public/common/download_danger_type.h"
-#include "content/browser/byte_stream.h"
 #include "content/browser/download/download_file_factory.h"
 #include "content/browser/download/download_file_impl.h"
 #include "content/browser/download/download_item_impl.h"
@@ -655,6 +654,16 @@ net::EmbeddedTestServer::HandleRequestCallback CreateEchoCookieHandler(
   return base::BindRepeating(&HandleRequestAndEchoCookies, relative_url);
 }
 
+// A request handler that takes the content of the request and sends it back on
+// the response.
+std::unique_ptr<net::test_server::HttpResponse> HandleUploadRequest(
+    const net::test_server::HttpRequest& request) {
+  std::unique_ptr<net::test_server::BasicHttpResponse> response(
+      (new net::test_server::BasicHttpResponse()));
+  response->set_content(request.content);
+  return std::move(response);
+}
+
 // Helper class to "flatten" handling of
 // TestDownloadHttpResponse::OnPauseHandler.
 class TestRequestPauseHandler {
@@ -3177,6 +3186,42 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, FetchErrorResponseBody) {
   }
 }
 
+// Verify that the upload body of a request is received correctly by the server.
+IN_PROC_BROWSER_TEST_F(DownloadContentTest, UploadBytes) {
+  net::EmbeddedTestServer server;
+  const std::string kUploadURL = "/upload";
+  std::string kUploadString = "Test upload body";
+
+  server.RegisterRequestHandler(base::BindRepeating(&HandleUploadRequest));
+  ASSERT_TRUE(server.Start());
+  GURL url = server.GetURL(kUploadURL);
+
+  std::unique_ptr<download::DownloadUrlParameters> download_parameters(
+      DownloadRequestUtils::CreateDownloadForWebContentsMainFrame(
+          shell()->web_contents(), url, TRAFFIC_ANNOTATION_FOR_TESTS));
+
+  download_parameters->set_post_body(
+      network::ResourceRequestBody::CreateFromBytes(kUploadString.data(),
+                                                    kUploadString.size()));
+
+  DownloadManager* download_manager = DownloadManagerForShell(shell());
+  std::unique_ptr<DownloadTestObserver> observer(CreateWaiter(shell(), 1));
+  download_manager->DownloadUrl(std::move(download_parameters));
+  observer->WaitForFinished();
+  std::vector<download::DownloadItem*> items;
+  download_manager->GetAllDownloads(&items);
+  EXPECT_EQ(1u, items.size());
+
+  // Verify the response body in the file. It should match the request content.
+  {
+    base::ScopedAllowBlockingForTesting allow_blocking;
+    std::string file_content;
+    ASSERT_TRUE(
+        base::ReadFileToString(items[0]->GetTargetFilePath(), &file_content));
+    EXPECT_EQ(kUploadString, file_content);
+  }
+}
+
 // Verify the case that the first response is HTTP 200, and then interrupted,
 // and the second response is HTTP 404, the response body of 404 should be
 // fetched.

content/browser/download/download_manager_impl.cc

@@ -142,6 +142,8 @@ DownloadManagerImpl::UniqueUrlDownloadHandlerPtr BeginDownload(
     base::WeakPtr<DownloadManagerImpl> download_manager) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
+  params->set_blob_storage_context_getter(
+      base::BindOnce(&BlobStorageContextGetter, resource_context));
   std::unique_ptr<net::URLRequest> url_request =
       DownloadRequestCore::CreateRequestOnIOThread(download_id, params.get());
   if (blob_data_handle) {

content/browser/download/download_utils.cc

@@ -8,13 +8,17 @@
 #include "base/memory/ptr_util.h"
 #include "base/process/process_handle.h"
 #include "base/strings/stringprintf.h"
+#include "base/task_scheduler/post_task.h"
 #include "components/download/downloader/in_progress/download_entry.h"
 #include "components/download/downloader/in_progress/in_progress_cache.h"
 #include "components/download/public/common/download_save_info.h"
 #include "components/download/public/common/download_url_parameters.h"
+#include "content/browser/blob_storage/chrome_blob_storage_context.h"
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_interrupt_reasons_utils.h"
 #include "content/browser/download/download_stats.h"
+#include "content/browser/loader/upload_data_stream_builder.h"
+#include "content/browser/resource_context_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/download_manager_delegate.h"
@@ -119,6 +123,14 @@ std::unique_ptr<net::HttpRequestHeaders> GetAdditionalRequestHeaders(
 
 }  // namespace
 
+storage::BlobStorageContext* BlobStorageContextGetter(
+    ResourceContext* resource_context) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  ChromeBlobStorageContext* blob_context =
+      GetChromeBlobStorageContextForResourceContext(resource_context);
+  return blob_context->context();
+}
+
 download::DownloadInterruptReason HandleRequestCompletionStatus(
     net::Error error_code,
     bool has_strong_validators,
@@ -186,9 +198,8 @@ std::unique_ptr<network::ResourceRequest> CreateResourceRequest(
     request->render_frame_id = params->render_frame_host_routing_id();
 
   bool has_upload_data = false;
-  if (!params->post_body().empty()) {
-    request->request_body = network::ResourceRequestBody::CreateFromBytes(
-        params->post_body().data(), params->post_body().size());
+  if (params->post_body()) {
+    request->request_body = params->post_body();
     has_upload_data = true;
   }
 
@@ -230,12 +241,17 @@ std::unique_ptr<net::URLRequest> CreateURLRequestOnIOThread(
                           params->GetNetworkTrafficAnnotation()));
   request->set_method(params->method());
 
-  if (!params->post_body().empty()) {
-    const std::string& body = params->post_body();
-    std::unique_ptr<net::UploadElementReader> reader(
-        net::UploadOwnedBytesElementReader::CreateWithString(body));
-    request->set_upload(
-        net::ElementsUploadDataStream::CreateWithReader(std::move(reader), 0));
+  if (params->post_body()) {
+    storage::BlobStorageContext* blob_context =
+        params->get_blob_storage_context_getter().Run();
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner =
+        base::CreateSingleThreadTaskRunnerWithTraits(
+            {base::MayBlock(), base::TaskPriority::USER_VISIBLE});
+    std::unique_ptr<net::UploadDataStream> upload_data_stream =
+        UploadDataStreamBuilder::Build(params->post_body().get(), blob_context,
+                                       nullptr /*FileSystemContext*/,
+                                       task_runner.get());
+    request->set_upload(std::move(upload_data_stream));
   }
 
   if (params->post_id() >= 0) {

content/browser/download/download_utils.h

@@ -26,9 +26,14 @@ namespace network {
 struct ResourceRequest;
 }
 
+namespace storage {
+class BlobStorageContext;
+}
+
 namespace content {
 
 class BrowserContext;
+class ResourceContext;
 struct DownloadCreateInfo;
 
 // Handle the url request completion status and return the interrupt reasons.
@@ -65,6 +70,9 @@ CONTENT_EXPORT base::Optional<download::DownloadEntry> GetInProgressEntry(
     const std::string& guid,
     BrowserContext* browser_context);
 
+storage::BlobStorageContext* BlobStorageContextGetter(
+    ResourceContext* resource_context);
+
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_UTILS_H_

content/browser/download/parallel_download_job.cc

@@ -6,11 +6,13 @@
 
 #include <algorithm>
 
+#include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
 #include "content/browser/download/download_create_info.h"
 #include "content/browser/download/download_stats.h"
+#include "content/browser/download/download_utils.h"
 #include "content/browser/download/parallel_download_utils.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/public/browser/browser_context.h"
@@ -287,6 +289,11 @@ void ParallelDownloadJob::CreateRequest(int64_t offset, int64_t length) {
   download_params->set_referrer(download_item_->GetReferrerUrl());
   download_params->set_referrer_policy(net::URLRequest::NEVER_CLEAR_REFERRER);
 
+  download_params->set_blob_storage_context_getter(
+      base::BindOnce(&BlobStorageContextGetter,
+                     DownloadItemUtils::GetBrowserContext(download_item_)
+                         ->GetResourceContext()));
+
   // Send the request.
   worker->SendRequest(std::move(download_params),
                       static_cast<StoragePartitionImpl*>(storage_partition)