Make FileURLLoaderFactory always owned by its |receivers_|.

This CL introduces FileURLLoaderFactory::Create static method that
allows creating a FileURLLoaderFactory that is owned by its |receivers_|
and will self-delete when the last receiver disconnects.

This CL removes the ability to directly construct and own a
std::unique_ptr<FileURLLoaderFactory>, because this ability means that
the factory can be destructed while receivers bound via the Clone method
are still alive (see the associated bug).  In particular, this CL 1)
stops exposing FileURLLoaderFactory constructor as a public member and
2) stops returning a std::unique_ptr from the
CreateFileURLLoaderFactory public //content API.

AwContentBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories
needs to be able to provide a FileURLLoaderFactory when
|aw_settings->GetAllowFileAccess()|.  This necessitates tweaking the
ContentBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories
method to ask for both NonNetworkURLLoaderFactoryDeprecatedMap
(factories to be owned by the caller) and a new
NonNetworkURLLoaderFactoryMap (receivers-owned factories).  The former
parameter type should be eventually deprecated and removed everywhere.

Bug: 1106995
Change-Id: Id710f611c60942d932fd1dc71f7e313f227f5424
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2337411
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804370}

android_webview/browser/aw_content_browser_client.cc

@@ -853,6 +853,7 @@ bool AwContentBrowserClient::HandleExternalProtocol(
 void AwContentBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories(
     int render_process_id,
     int render_frame_id,
+    NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
     NonNetworkURLLoaderFactoryMap* factories) {
   WebContents* web_contents = content::WebContents::FromRenderFrameHost(
       content::RenderFrameHost::FromID(render_process_id, render_frame_id));
@@ -861,10 +862,11 @@ void AwContentBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories(
   if (aw_settings && aw_settings->GetAllowFileAccess()) {
     AwBrowserContext* aw_browser_context =
         AwBrowserContext::FromWebContents(web_contents);
-    auto file_factory = CreateFileURLLoaderFactory(
-        aw_browser_context->GetPath(),
-        aw_browser_context->GetSharedCorsOriginAccessList());
-    factories->emplace(url::kFileScheme, std::move(file_factory));
+    factories->emplace(
+        url::kFileScheme,
+        content::CreateFileURLLoaderFactory(
+            aw_browser_context->GetPath(),
+            aw_browser_context->GetSharedCorsOriginAccessList()));
   }
 }
 

android_webview/browser/aw_content_browser_client.h

@@ -185,6 +185,7 @@ class AwContentBrowserClient : public content::ContentBrowserClient {
   void RegisterNonNetworkSubresourceURLLoaderFactories(
       int render_process_id,
       int render_frame_id,
+      NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
       NonNetworkURLLoaderFactoryMap* factories) override;
   bool ShouldIsolateErrorPage(bool in_main_frame) override;
   bool ShouldEnableStrictSiteIsolation() override;

chrome/browser/browser_switcher/browser_switcher_service.cc

@@ -99,8 +99,8 @@ XmlDownloader::XmlDownloader(Profile* profile,
                              base::TimeDelta first_fetch_delay,
                              base::RepeatingCallback<void()> all_done_callback)
     : service_(service), all_done_callback_(std::move(all_done_callback)) {
-  file_url_factory_ =
-      content::CreateFileURLLoaderFactory(base::FilePath(), nullptr);
+  file_url_factory_.Bind(
+      content::CreateFileURLLoaderFactory(base::FilePath(), nullptr));
   other_url_factory_ =
       content::BrowserContext::GetDefaultStoragePartition(profile)
           ->GetURLLoaderFactoryForBrowserProcess();

chrome/browser/browser_switcher/browser_switcher_service.h

@@ -17,6 +17,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/browser_switcher/browser_switcher_prefs.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "url/gurl.h"
@@ -91,7 +92,7 @@ class XmlDownloader {
 
   network::mojom::URLLoaderFactory* GetURLLoaderFactoryForURL(const GURL& url);
 
-  std::unique_ptr<network::mojom::URLLoaderFactory> file_url_factory_;
+  mojo::Remote<network::mojom::URLLoaderFactory> file_url_factory_;
   scoped_refptr<network::SharedURLLoaderFactory> other_url_factory_;
 
   // This |BrowserSwitcherService| owns this object.

chrome/browser/chrome_content_browser_client.cc

@@ -4480,12 +4480,13 @@ ChromeContentBrowserClient::CreateURLLoaderThrottles(
 void ChromeContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
     int frame_tree_node_id,
     base::UkmSourceId ukm_source_id,
+    NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
     NonNetworkURLLoaderFactoryMap* factories) {
 #if BUILDFLAG(ENABLE_EXTENSIONS) || defined(OS_CHROMEOS)
   content::WebContents* web_contents =
       content::WebContents::FromFrameTreeNodeId(frame_tree_node_id);
 #if BUILDFLAG(ENABLE_EXTENSIONS)
-  factories->emplace(
+  uniquely_owned_factories->emplace(
       extensions::kExtensionScheme,
       extensions::CreateExtensionNavigationURLLoaderFactory(
           web_contents->GetBrowserContext(), ukm_source_id,
@@ -4494,17 +4495,18 @@ void ChromeContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
 #if defined(OS_CHROMEOS)
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  factories->emplace(content::kExternalFileScheme,
-                     std::make_unique<chromeos::ExternalFileURLLoaderFactory>(
-                         profile, content::ChildProcessHost::kInvalidUniqueID));
+  uniquely_owned_factories->emplace(
+      content::kExternalFileScheme,
+      std::make_unique<chromeos::ExternalFileURLLoaderFactory>(
+          profile, content::ChildProcessHost::kInvalidUniqueID));
 #endif  // defined(OS_CHROMEOS)
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS) || defined(OS_CHROMEOS)
 }
 
 void ChromeContentBrowserClient::
     RegisterNonNetworkWorkerMainResourceURLLoaderFactories(
         content::BrowserContext* browser_context,
-        NonNetworkURLLoaderFactoryMap* factories) {
+        NonNetworkURLLoaderFactoryDeprecatedMap* factories) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   DCHECK(browser_context);
   DCHECK(factories);
@@ -4518,7 +4520,7 @@ void ChromeContentBrowserClient::
 void ChromeContentBrowserClient::
     RegisterNonNetworkServiceWorkerUpdateURLLoaderFactories(
         content::BrowserContext* browser_context,
-        NonNetworkURLLoaderFactoryMap* factories) {
+        NonNetworkURLLoaderFactoryDeprecatedMap* factories) {
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   DCHECK(browser_context);
   DCHECK(factories);
@@ -4613,6 +4615,7 @@ void ChromeContentBrowserClient::
     RegisterNonNetworkSubresourceURLLoaderFactories(
         int render_process_id,
         int render_frame_id,
+        NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
         NonNetworkURLLoaderFactoryMap* factories) {
 #if defined(OS_CHROMEOS) || BUILDFLAG(ENABLE_EXTENSIONS)
   content::RenderFrameHost* frame_host =
@@ -4624,17 +4627,19 @@ void ChromeContentBrowserClient::
   if (web_contents) {
     Profile* profile =
         Profile::FromBrowserContext(web_contents->GetBrowserContext());
-    factories->emplace(content::kExternalFileScheme,
-                       std::make_unique<chromeos::ExternalFileURLLoaderFactory>(
-                           profile, render_process_id));
+    uniquely_owned_factories->emplace(
+        content::kExternalFileScheme,
+        std::make_unique<chromeos::ExternalFileURLLoaderFactory>(
+            profile, render_process_id));
   }
 #endif  // defined(OS_CHROMEOS)
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   auto factory = extensions::CreateExtensionURLLoaderFactory(render_process_id,
                                                              render_frame_id);
   if (factory)
-    factories->emplace(extensions::kExtensionScheme, std::move(factory));
+    uniquely_owned_factories->emplace(extensions::kExtensionScheme,
+                                      std::move(factory));
 
   // This logic should match
   // ChromeExtensionWebContentsObserver::RenderFrameCreated.
@@ -4648,7 +4653,7 @@ void ChromeContentBrowserClient::
   // The test below matches what's done by ShouldServiceRequestIOThread in
   // local_ntp_source.cc.
   if (instant_service->IsInstantProcess(render_process_id)) {
-    factories->emplace(
+    uniquely_owned_factories->emplace(
         chrome::kChromeSearchScheme,
         content::CreateWebUIURLLoader(
             frame_host, chrome::kChromeSearchScheme,
@@ -4690,7 +4695,7 @@ void ChromeContentBrowserClient::
     allowed_webui_hosts.emplace_back(chrome::kChromeUIAppIconHost);
   }
   if (!allowed_webui_hosts.empty()) {
-    factories->emplace(
+    uniquely_owned_factories->emplace(
         content::kChromeUIScheme,
         content::CreateWebUIURLLoader(frame_host, content::kChromeUIScheme,
                                       std::move(allowed_webui_hosts)));
@@ -4702,8 +4707,9 @@ void ChromeContentBrowserClient::
       extensions::ProcessManager::Get(web_contents->GetBrowserContext())
           ->GetBackgroundHostForExtension(extension->id());
   if (host) {
-    factories->emplace(url::kFileScheme, std::make_unique<FileURLLoaderFactory>(
-                                             render_process_id));
+    uniquely_owned_factories->emplace(
+        url::kFileScheme,
+        std::make_unique<FileURLLoaderFactory>(render_process_id));
   }
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 }

chrome/browser/chrome_content_browser_client.h

@@ -460,16 +460,18 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   void RegisterNonNetworkNavigationURLLoaderFactories(
       int frame_tree_node_id,
       base::UkmSourceId ukm_source_id,
+      NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
       NonNetworkURLLoaderFactoryMap* factories) override;
   void RegisterNonNetworkWorkerMainResourceURLLoaderFactories(
       content::BrowserContext* browser_context,
-      NonNetworkURLLoaderFactoryMap* factories) override;
+      NonNetworkURLLoaderFactoryDeprecatedMap* factories) override;
   void RegisterNonNetworkServiceWorkerUpdateURLLoaderFactories(
       content::BrowserContext* browser_context,
-      NonNetworkURLLoaderFactoryMap* factories) override;
+      NonNetworkURLLoaderFactoryDeprecatedMap* factories) override;
   void RegisterNonNetworkSubresourceURLLoaderFactories(
       int render_process_id,
       int render_frame_id,
+      NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
       NonNetworkURLLoaderFactoryMap* factories) override;
   bool WillCreateURLLoaderFactory(
       content::BrowserContext* browser_context,

chrome/browser/devtools/devtools_ui_bindings.cc

@@ -80,6 +80,7 @@
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 #include "services/network/public/cpp/simple_url_loader_stream_consumer.h"
+#include "services/network/public/cpp/wrapper_shared_url_loader_factory.h"
 #include "services/network/public/mojom/url_response_head.mojom.h"
 #include "third_party/blink/public/mojom/renderer_preferences.mojom.h"
 #include "third_party/blink/public/public_buildflags.h"
@@ -886,9 +887,13 @@ void DevToolsUIBindings::LoadNetworkResource(const DispatchCallback& callback,
 
   NetworkResourceLoader::URLLoaderFactoryHolder url_loader_factory;
   if (gurl.SchemeIsFile()) {
-    url_loader_factory = content::CreateFileURLLoaderFactory(
-        base::FilePath() /* profile_path */,
-        nullptr /* shared_cors_origin_access_list */);
+    mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote =
+        content::CreateFileURLLoaderFactory(
+            base::FilePath() /* profile_path */,
+            nullptr /* shared_cors_origin_access_list */);
+    url_loader_factory = network::SharedURLLoaderFactory::Create(
+        std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
+            std::move(pending_remote)));
   } else if (content::HasWebUIScheme(gurl)) {
     content::WebContents* target_tab =
         DevToolsWindow::AsDevToolsWindow(web_contents_)

chromecast/browser/cast_content_browser_client.cc

@@ -868,6 +868,7 @@ CastContentBrowserClient::CreateThrottlesForNavigation(
 void CastContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
     int frame_tree_node_id,
     base::UkmSourceId ukm_source_id,
+    NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
     NonNetworkURLLoaderFactoryMap* factories) {
 #if BUILDFLAG(ENABLE_CHROMECAST_EXTENSIONS)
   content::WebContents* web_contents =
@@ -877,15 +878,17 @@ void CastContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
       extensions::CreateExtensionNavigationURLLoaderFactory(
           browser_context, ukm_source_id,
           !!extensions::WebViewGuest::FromWebContents(web_contents));
-  factories->emplace(extensions::kExtensionScheme,
-                     std::make_unique<CastExtensionURLLoaderFactory>(
-                         browser_context, std::move(extension_factory)));
+  uniquely_owned_factories->emplace(
+      extensions::kExtensionScheme,
+      std::make_unique<CastExtensionURLLoaderFactory>(
+          browser_context, std::move(extension_factory)));
 #endif
 }
 
 void CastContentBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories(
     int render_process_id,
     int render_frame_id,
+    NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
     NonNetworkURLLoaderFactoryMap* factories) {
   if (render_frame_id == MSG_ROUTING_NONE) {
     LOG(ERROR) << "Service worker not supported.";
@@ -898,12 +901,13 @@ void CastContentBrowserClient::RegisterNonNetworkSubresourceURLLoaderFactories(
   auto* browser_context = frame_host->GetProcess()->GetBrowserContext();
   auto extension_factory = extensions::CreateExtensionURLLoaderFactory(
       render_process_id, render_frame_id);
-  factories->emplace(extensions::kExtensionScheme,
-                     std::make_unique<CastExtensionURLLoaderFactory>(
-                         browser_context, std::move(extension_factory)));
+  uniquely_owned_factories->emplace(
+      extensions::kExtensionScheme,
+      std::make_unique<CastExtensionURLLoaderFactory>(
+          browser_context, std::move(extension_factory)));
 #endif
 
-  factories->emplace(
+  uniquely_owned_factories->emplace(
       kChromeResourceScheme,
       content::CreateWebUIURLLoader(
           frame_host, kChromeResourceScheme,

chromecast/browser/cast_content_browser_client.h

@@ -229,10 +229,12 @@ class CastContentBrowserClient
   void RegisterNonNetworkNavigationURLLoaderFactories(
       int frame_tree_node_id,
       base::UkmSourceId ukm_source_id,
+      NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
       NonNetworkURLLoaderFactoryMap* factories) override;
   void RegisterNonNetworkSubresourceURLLoaderFactories(
       int render_process_id,
       int render_frame_id,
+      NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
       NonNetworkURLLoaderFactoryMap* factories) override;
   void OnNetworkServiceCreated(
       network::mojom::NetworkService* network_service) override;

content/browser/download/download_browsertest.cc

@@ -163,6 +163,7 @@ class DownloadTestContentBrowserClient : public TestContentBrowserClient {
   void RegisterNonNetworkNavigationURLLoaderFactories(
       int frame_tree_node_id,
       base::UkmSourceId ukm_source_id,
+      NonNetworkURLLoaderFactoryDeprecatedMap* uniquely_owned_factories,
       NonNetworkURLLoaderFactoryMap* factories) override {
     if (!enable_register_non_network_url_loader_)
       return;
@@ -173,16 +174,17 @@ class DownloadTestContentBrowserClient : public TestContentBrowserClient {
             "HTTP/1.1 200 OK\nContent-Type: multipart/related\n\n",
             "This is a test for download mhtml through non http/https urls",
             /* network_accessed */ true, net::OK);
-    factories->emplace(url::kContentScheme,
-                       std::move(content_url_loader_factory));
+    uniquely_owned_factories->emplace(url::kContentScheme,
+                                      std::move(content_url_loader_factory));
 #endif  // OS_ANDROID
 
     auto file_url_loader_factory =
         std::make_unique<FakeNetworkURLLoaderFactory>(
             "HTTP/1.1 200 OK\nContent-Type: multipart/related\n\n",
             "This is a test for download mhtml through non http/https urls",
             /* network_accessed */ true, net::OK);
-    factories->emplace(url::kFileScheme, std::move(file_url_loader_factory));
+    uniquely_owned_factories->emplace(url::kFileScheme,
+                                      std::move(file_url_loader_factory));
   }
 
  private:

content/browser/download/download_manager_impl.cc

@@ -1290,15 +1290,6 @@ void DownloadManagerImpl::BeginResourceDownloadOnChecksComplete(
   if (blob_url_loader_factory) {
     DCHECK(params->url().SchemeIsBlob());
     pending_url_loader_factory = blob_url_loader_factory->Clone();
-  } else if (params->url().SchemeIsFile()) {
-    pending_url_loader_factory =
-        CreatePendingSharedURLLoaderFactoryFromURLLoaderFactory(
-            std::make_unique<FileURLLoaderFactory>(
-                browser_context_->GetPath(),
-                browser_context_->GetSharedCorsOriginAccessList(),
-                // USER_VISIBLE because download should progress
-                // even when there is high priority work to do.
-                base::TaskPriority::USER_VISIBLE));
   } else if (rfh && params->url().SchemeIs(content::kChromeUIScheme)) {
     mojo::PendingRemote<network::mojom::URLLoaderFactory>
         url_loader_factory_remote;
@@ -1327,22 +1318,43 @@ void DownloadManagerImpl::BeginResourceDownloadOnChecksComplete(
         CreatePendingSharedURLLoaderFactoryFromURLLoaderFactory(
             std::make_unique<DataURLLoaderFactory>(params->url()));
   } else if (rfh && !blink::IsURLHandledByNetworkService(params->url())) {
+    ContentBrowserClient::NonNetworkURLLoaderFactoryDeprecatedMap
+        non_network_uniquely_owned_factories;
     ContentBrowserClient::NonNetworkURLLoaderFactoryMap
         non_network_url_loader_factories;
+
+    // USER_VISIBLE because download should progress
+    // even when there is high priority work to do.
+    base::TaskPriority file_factory_priority = base::TaskPriority::USER_VISIBLE;
+    non_network_url_loader_factories.emplace(
+        url::kFileScheme, FileURLLoaderFactory::Create(
+                              browser_context_->GetPath(),
+                              browser_context_->GetSharedCorsOriginAccessList(),
+                              file_factory_priority));
+
     GetContentClient()
         ->browser()
         ->RegisterNonNetworkSubresourceURLLoaderFactories(
             params->render_process_host_id(),
             params->render_frame_host_routing_id(),
+            &non_network_uniquely_owned_factories,
             &non_network_url_loader_factories);
-    auto it = non_network_url_loader_factories.find(params->url().scheme());
-    if (it == non_network_url_loader_factories.end()) {
-      DLOG(ERROR) << "No URLLoaderFactory found to download " << params->url();
-      return;
-    } else {
+    auto it = non_network_uniquely_owned_factories.find(params->url().scheme());
+    if (it != non_network_uniquely_owned_factories.end()) {
       pending_url_loader_factory =
           CreatePendingSharedURLLoaderFactoryFromURLLoaderFactory(
               std::move(it->second));
+    } else {
+      auto it2 = non_network_url_loader_factories.find(params->url().scheme());
+      if (it2 != non_network_url_loader_factories.end()) {
+        pending_url_loader_factory =
+            std::make_unique<network::WrapperPendingSharedURLLoaderFactory>(
+                std::move(it2->second));
+      } else {
+        DLOG(ERROR) << "No URLLoaderFactory found to download "
+                    << params->url();
+        return;
+      }
     }
   } else {
     StoragePartitionImpl* storage_partition =

content/browser/download/save_file_manager.cc

@@ -253,6 +253,7 @@ void SaveFileManager::SaveURL(SaveItemId save_item_id,
 
     network::mojom::URLLoaderFactory* factory = nullptr;
     std::unique_ptr<network::mojom::URLLoaderFactory> url_loader_factory;
+    mojo::Remote<network::mojom::URLLoaderFactory> factory_remote;
     auto* rfh = RenderFrameHostImpl::FromID(render_process_host_id,
                                             render_frame_routing_id);
 
@@ -264,10 +265,10 @@ void SaveFileManager::SaveURL(SaveItemId save_item_id,
       url_loader_factory = std::make_unique<DataURLLoaderFactory>();
       factory = url_loader_factory.get();
     } else if (url.SchemeIsFile()) {
-      url_loader_factory = std::make_unique<FileURLLoaderFactory>(
+      factory_remote.Bind(FileURLLoaderFactory::Create(
           context->GetPath(), context->GetSharedCorsOriginAccessList(),
-          base::TaskPriority::USER_VISIBLE);
-      factory = url_loader_factory.get();
+          base::TaskPriority::USER_VISIBLE));
+      factory = factory_remote.get();
     } else if (url.SchemeIsFileSystem() && rfh) {
       auto* storage_partition_impl =
           static_cast<StoragePartitionImpl*>(storage_partition);