Make ExternalFileURLLoaderFactory always owned by its |receivers_|.

This CL introduces ExternalFileURLLoaderFactory::Create static method
that allows creating a ExternalFileURLLoaderFactory 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<ExternalFileURLLoaderFactory>, 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 stops exposing ExternalFileURLLoaderFactory constructor as a
public member, which forces construction to go via the new Create static
method.

This CL mostly just follows the pattern established earlier by
https://crrev.com/c/2337411.

Bug: 1106995
Change-Id: I4bd235cf9132668d80657aece34de6fe7b60f78b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357620Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812139}

chrome/browser/chrome_content_browser_client.cc

@@ -4485,10 +4485,9 @@ void ChromeContentBrowserClient::RegisterNonNetworkNavigationURLLoaderFactories(
 #if defined(OS_CHROMEOS)
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
-  uniquely_owned_factories->emplace(
-      content::kExternalFileScheme,
-      std::make_unique<chromeos::ExternalFileURLLoaderFactory>(
-          profile, content::ChildProcessHost::kInvalidUniqueID));
+  factories->emplace(content::kExternalFileScheme,
+                     chromeos::ExternalFileURLLoaderFactory::Create(
+                         profile, content::ChildProcessHost::kInvalidUniqueID));
 #endif  // defined(OS_CHROMEOS)
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS) || defined(OS_CHROMEOS)
 }
@@ -4642,10 +4641,9 @@ void ChromeContentBrowserClient::
   if (web_contents) {
     Profile* profile =
         Profile::FromBrowserContext(web_contents->GetBrowserContext());
-    uniquely_owned_factories->emplace(
-        content::kExternalFileScheme,
-        std::make_unique<chromeos::ExternalFileURLLoaderFactory>(
-            profile, render_process_id));
+    factories->emplace(content::kExternalFileScheme,
+                       chromeos::ExternalFileURLLoaderFactory::Create(
+                           profile, render_process_id));
   }
 #endif  // defined(OS_CHROMEOS)
 

chrome/browser/chromeos/fileapi/external_file_url_loader_factory.cc

@@ -341,8 +341,10 @@ class ExternalFileURLLoader : public network::mojom::URLLoader {
 
 ExternalFileURLLoaderFactory::ExternalFileURLLoaderFactory(
     void* profile_id,
-    int render_process_host_id)
-    : profile_id_(profile_id),
+    int render_process_host_id,
+    mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver)
+    : content::NonNetworkURLLoaderFactoryBase(std::move(factory_receiver)),
+      profile_id_(profile_id),
       render_process_host_id_(render_process_host_id) {}
 
 ExternalFileURLLoaderFactory::~ExternalFileURLLoaderFactory() = default;
@@ -369,9 +371,19 @@ void ExternalFileURLLoaderFactory::CreateLoaderAndStart(
                      request, std::move(loader), std::move(client)));
 }
 
-void ExternalFileURLLoaderFactory::Clone(
-    mojo::PendingReceiver<network::mojom::URLLoaderFactory> loader) {
-  receivers_.Add(this, std::move(loader));
+// static
+mojo::PendingRemote<network::mojom::URLLoaderFactory>
+ExternalFileURLLoaderFactory::Create(void* profile_id,
+                                     int render_process_host_id) {
+  mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote;
+
+  // The ExternalFileURLLoaderFactory will delete itself when there are no more
+  // receivers - see the NonNetworkURLLoaderFactoryBase::OnDisconnect method.
+  new ExternalFileURLLoaderFactory(
+      profile_id, render_process_host_id,
+      pending_remote.InitWithNewPipeAndPassReceiver());
+
+  return pending_remote;
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/fileapi/external_file_url_loader_factory.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_CHROMEOS_FILEAPI_EXTERNAL_FILE_URL_LOADER_FACTORY_H_
 
 #include "base/macros.h"
+#include "content/public/browser/non_network_url_loader_factory_base.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
@@ -15,15 +16,30 @@ namespace chromeos {
 
 // URLLoaderFactory that creates URLLoader instances for URLs with the
 // externalfile scheme.
-class ExternalFileURLLoaderFactory : public network::mojom::URLLoaderFactory {
+class ExternalFileURLLoaderFactory
+    : public content::NonNetworkURLLoaderFactoryBase {
  public:
+  // Returns mojo::PendingRemote to a newly constructed
+  // ExternalFileURLLoaderFactory.  The factory is self-owned - it will delete
+  // itself once there are no more receivers (including the receiver associated
+  // with the returned mojo::PendingRemote and the receivers bound by the Clone
+  // method).
+  //
   // |render_process_host_id| is used to verify that the child process has
   // permission to request the URL. It should be
   // ChildProcessHost::kInvalidUniqueID for navigations.
-  ExternalFileURLLoaderFactory(void* profile_id, int render_process_host_id);
-  ~ExternalFileURLLoaderFactory() override;
+  static mojo::PendingRemote<network::mojom::URLLoaderFactory> Create(
+      void* profile_id,
+      int render_process_host_id);
 
  private:
+  ExternalFileURLLoaderFactory(
+      void* profile_id,
+      int render_process_host_id,
+      mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver);
+
+  ~ExternalFileURLLoaderFactory() override;
+
   friend class ExternalFileURLLoaderFactoryTest;
 
   // network::mojom::URLLoaderFactory:
@@ -36,10 +52,7 @@ class ExternalFileURLLoaderFactory : public network::mojom::URLLoaderFactory {
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation)
       override;
-  void Clone(
-      mojo::PendingReceiver<network::mojom::URLLoaderFactory> loader) override;
 
-  mojo::ReceiverSet<network::mojom::URLLoaderFactory> receivers_;
   void* profile_id_;
   const int render_process_host_id_;
 

chrome/browser/chromeos/fileapi/external_file_url_loader_factory_unittest.cc

@@ -72,8 +72,8 @@ class ExternalFileURLLoaderFactoryTest : public testing::Test {
                                  kFileSystemId, "Test FileSystem"));
 
     // Create the URLLoaderFactory.
-    url_loader_factory_ = std::make_unique<ExternalFileURLLoaderFactory>(
-        profile, render_process_host_id());
+    url_loader_factory_.Bind(ExternalFileURLLoaderFactory::Create(
+        profile, render_process_host_id()));
   }
 
   virtual int render_process_host_id() {
@@ -106,7 +106,7 @@ class ExternalFileURLLoaderFactoryTest : public testing::Test {
  private:
   content::BrowserTaskEnvironment task_environment_;
 
-  std::unique_ptr<ExternalFileURLLoaderFactory> url_loader_factory_;
+  mojo::Remote<network::mojom::URLLoaderFactory> url_loader_factory_;
 
   std::unique_ptr<TestingProfileManager> profile_manager_;
   std::unique_ptr<chromeos::FakeChromeUserManager> user_manager_;