Pass delegate's TaskRunner to ResourceDownloader

ResourceDownloader uses BrowserThread::UI to post tasks to the delegate.
This introduces dependency on content/
Let delegate pass a TaskRunner to ResourceDownloader solves the issue.

BUG=803135

Change-Id: I44ebce8c72d65f99f90009ac9ca20709300cb2cd
Reviewed-on: https://chromium-review.googlesource.com/961528Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543131}

content/browser/download/download_manager_impl.cc

@@ -202,7 +202,8 @@ DownloadManagerImpl::UniqueUrlDownloadHandlerPtr BeginResourceDownload(
     base::WeakPtr<DownloadManagerImpl> download_manager,
     const GURL& site_url,
     const GURL& tab_url,
-    const GURL& tab_referrer_url) {
+    const GURL& tab_referrer_url,
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   // Check if the renderer is permitted to request the requested URL.
@@ -235,7 +236,7 @@ DownloadManagerImpl::UniqueUrlDownloadHandlerPtr BeginResourceDownload(
       ResourceDownloader::BeginDownload(
           download_manager, std::move(params), std::move(request),
           std::move(shared_url_loader_factory), site_url, tab_url,
-          tab_referrer_url, download_id, false)
+          tab_referrer_url, download_id, false, task_runner)
           .release());
 }
 
@@ -1159,7 +1160,8 @@ void DownloadManagerImpl::InterceptNavigationOnChecksComplete(
                      render_process_id, render_frame_id, std::move(url_chain),
                      suggested_filename, std::move(response),
                      std::move(cert_status),
-                     std::move(url_loader_client_endpoints)));
+                     std::move(url_loader_client_endpoints),
+                     base::MessageLoop::current()->task_runner()));
 }
 
 // static
@@ -1172,7 +1174,8 @@ void DownloadManagerImpl::CreateDownloadHandlerForNavigation(
     const base::Optional<std::string>& suggested_filename,
     scoped_refptr<network::ResourceResponse> response,
     net::CertStatus cert_status,
-    network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints) {
+    network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   std::unique_ptr<ResourceDownloader> resource_downloader =
@@ -1180,7 +1183,7 @@ void DownloadManagerImpl::CreateDownloadHandlerForNavigation(
           download_manager, std::move(resource_request), render_process_id,
           render_frame_id, std::move(url_chain), suggested_filename,
           std::move(response), std::move(cert_status),
-          std::move(url_loader_client_endpoints));
+          std::move(url_loader_client_endpoints), task_runner);
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
@@ -1220,7 +1223,8 @@ void DownloadManagerImpl::BeginDownloadInternal(
                        std::move(request), std::move(blob_data_handle),
                        storage_partition->url_loader_factory_getter(), id,
                        weak_factory_.GetWeakPtr(), site_url, tab_url,
-                       tab_referrer_url),
+                       tab_referrer_url,
+                       base::MessageLoop::current()->task_runner()),
         base::BindOnce(&DownloadManagerImpl::AddUrlDownloadHandler,
                        weak_factory_.GetWeakPtr()));
   } else {

content/browser/download/download_manager_impl.h

@@ -257,7 +257,8 @@ class CONTENT_EXPORT DownloadManagerImpl : public DownloadManager,
       const base::Optional<std::string>& suggested_filename,
       scoped_refptr<network::ResourceResponse> response,
       net::CertStatus cert_status,
-      network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints);
+      network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
 
   // Factory for creation of downloads items.
   std::unique_ptr<DownloadItemFactory> item_factory_;

content/browser/download/download_worker.cc

@@ -4,6 +4,7 @@
 
 #include "content/browser/download/download_worker.h"
 
+#include "base/message_loop/message_loop.h"
 #include "components/download/public/common/download_create_info.h"
 #include "components/download/public/common/download_interrupt_reasons.h"
 #include "components/download/public/common/input_stream.h"
@@ -46,7 +47,8 @@ std::unique_ptr<UrlDownloadHandler, BrowserThread::DeleteOnIOThread>
 CreateUrlDownloadHandler(
     std::unique_ptr<download::DownloadUrlParameters> params,
     base::WeakPtr<UrlDownloadHandler::Delegate> delegate,
-    scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter) {
+    scoped_refptr<URLLoaderFactoryGetter> url_loader_factory_getter,
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
@@ -56,7 +58,7 @@ CreateUrlDownloadHandler(
         ResourceDownloader::BeginDownload(
             delegate, std::move(params), std::move(request),
             url_loader_factory_getter->GetNetworkFactory(), GURL(), GURL(),
-            GURL(), download::DownloadItem::kInvalidId, true)
+            GURL(), download::DownloadItem::kInvalidId, true, task_runner)
             .release());
   } else {
     // Build the URLRequest, BlobDataHandle is hold in original request for
@@ -96,7 +98,8 @@ void DownloadWorker::SendRequest(
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&CreateUrlDownloadHandler, std::move(params),
-                     weak_factory_.GetWeakPtr(), url_loader_factory_getter),
+                     weak_factory_.GetWeakPtr(), url_loader_factory_getter,
+                     base::MessageLoop::current()->task_runner()),
       base::BindOnce(&DownloadWorker::AddUrlDownloadHandler,
                      weak_factory_.GetWeakPtr()));
 }

content/browser/download/resource_downloader.cc

@@ -8,10 +8,6 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "components/download/public/common/stream_handle_input_stream.h"
-#include "content/browser/blob_storage/blob_url_loader_factory.h"
-#include "content/browser/download/download_utils.h"
-
-#include "content/public/browser/web_contents.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace network {
@@ -70,11 +66,12 @@ std::unique_ptr<ResourceDownloader> ResourceDownloader::BeginDownload(
     const GURL& tab_url,
     const GURL& tab_referrer_url,
     uint32_t download_id,
-    bool is_parallel_request) {
+    bool is_parallel_request,
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   auto downloader = std::make_unique<ResourceDownloader>(
       delegate, std::move(request), params->render_process_host_id(),
       params->render_frame_host_routing_id(), site_url, tab_url,
-      tab_referrer_url, download_id);
+      tab_referrer_url, download_id, task_runner);
 
   downloader->Start(std::move(shared_url_loader_factory), std::move(params),
                     is_parallel_request);
@@ -92,10 +89,11 @@ ResourceDownloader::InterceptNavigationResponse(
     const base::Optional<std::string>& suggested_filename,
     const scoped_refptr<network::ResourceResponse>& response,
     net::CertStatus cert_status,
-    network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints) {
+    network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
   auto downloader = std::make_unique<ResourceDownloader>(
       delegate, std::move(resource_request), render_process_id, render_frame_id,
-      GURL(), GURL(), GURL(), download::DownloadItem::kInvalidId);
+      GURL(), GURL(), GURL(), download::DownloadItem::kInvalidId, task_runner);
   downloader->InterceptResponse(std::move(response), std::move(url_chain),
                                 suggested_filename, cert_status,
                                 std::move(url_loader_client_endpoints));
@@ -110,7 +108,8 @@ ResourceDownloader::ResourceDownloader(
     const GURL& site_url,
     const GURL& tab_url,
     const GURL& tab_referrer_url,
-    uint32_t download_id)
+    uint32_t download_id,
+    const scoped_refptr<base::SingleThreadTaskRunner>& task_runner)
     : delegate_(delegate),
       resource_request_(std::move(resource_request)),
       download_id_(download_id),
@@ -119,6 +118,7 @@ ResourceDownloader::ResourceDownloader(
       site_url_(site_url),
       tab_url_(tab_url),
       tab_referrer_url_(tab_referrer_url),
+      delegate_task_runner_(task_runner),
       weak_ptr_factory_(this) {}
 
 ResourceDownloader::~ResourceDownloader() = default;
@@ -202,8 +202,8 @@ void ResourceDownloader::OnResponseStarted(
   download_create_info->render_process_id = render_process_id_;
   download_create_info->render_frame_id = render_frame_id_;
 
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
+  delegate_task_runner_->PostTask(
+      FROM_HERE,
       base::BindOnce(&UrlDownloadHandler::Delegate::OnUrlDownloadStarted,
                      delegate_, std::move(download_create_info),
                      std::make_unique<download::StreamHandleInputStream>(

content/browser/download/resource_downloader.h

@@ -7,8 +7,8 @@
 
 #include "components/download/public/common/download_response_handler.h"
 #include "content/browser/download/url_download_handler.h"
-#include "content/public/browser/ssl_status.h"
 #include "mojo/public/cpp/bindings/binding.h"
+#include "net/cert/cert_status_flags.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
 
@@ -32,7 +32,8 @@ class ResourceDownloader : public UrlDownloadHandler,
       const GURL& tab_url,
       const GURL& tab_referrer_url,
       uint32_t download_id,
-      bool is_parallel_request);
+      bool is_parallel_request,
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
 
   // Create a ResourceDownloader from a navigation that turns to be a download.
   // No URLLoader is created, but the URLLoaderClient implementation is
@@ -46,16 +47,19 @@ class ResourceDownloader : public UrlDownloadHandler,
       const base::Optional<std::string>& suggested_filename,
       const scoped_refptr<network::ResourceResponse>& response,
       net::CertStatus cert_status,
-      network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints);
+      network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
 
-  ResourceDownloader(base::WeakPtr<UrlDownloadHandler::Delegate> delegate,
+  ResourceDownloader(
+      base::WeakPtr<UrlDownloadHandler::Delegate> delegate,
       std::unique_ptr<network::ResourceRequest> resource_request,
       int render_process_id,
       int render_frame_id,
       const GURL& site_url,
       const GURL& tab_url,
       const GURL& tab_referrer_url,
-                     uint32_t download_id);
+      uint32_t download_id,
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);
   ~ResourceDownloader() override;
 
   // download::DownloadResponseHandler::Delegate
@@ -120,6 +124,9 @@ class ResourceDownloader : public UrlDownloadHandler,
   // URLLoader status when intercepting the navigation request.
   base::Optional<network::URLLoaderCompletionStatus> url_loader_status_;
 
+  // TaskRunner to post callbacks to the |delegate_|
+  scoped_refptr<base::SingleThreadTaskRunner> delegate_task_runner_;
+
   base::WeakPtrFactory<ResourceDownloader> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ResourceDownloader);