Make ContentDirectoryLoaderFactory always owned by its |receivers_|.

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

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

Bug: 1106995
Change-Id: I5ed88648637eed43f23dfab234115bfa33281ccc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2357964
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Kevin Marshall <kmarshall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812152}

fuchsia/engine/browser/content_directory_loader_factory.cc

@@ -350,8 +350,23 @@ class ContentDirectoryURLLoader : public network::mojom::URLLoader {
 
 }  // namespace
 
-ContentDirectoryLoaderFactory::ContentDirectoryLoaderFactory()
-    : task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
+// static
+mojo::PendingRemote<network::mojom::URLLoaderFactory>
+ContentDirectoryLoaderFactory::Create() {
+  mojo::PendingRemote<network::mojom::URLLoaderFactory> pending_remote;
+
+  // The ContentDirectoryLoaderFactory will delete itself when there are no more
+  // receivers - see the NonNetworkURLLoaderFactoryBase::OnDisconnect method.
+  new ContentDirectoryLoaderFactory(
+      pending_remote.InitWithNewPipeAndPassReceiver());
+
+  return pending_remote;
+}
+
+ContentDirectoryLoaderFactory::ContentDirectoryLoaderFactory(
+    mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver)
+    : content::NonNetworkURLLoaderFactoryBase(std::move(factory_receiver)),
+      task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
           {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
            base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})) {}
 
@@ -445,11 +460,6 @@ void ContentDirectoryLoaderFactory::CreateLoaderAndStart(
                                 base::Passed(std::move(metadata_handle))));
 }
 
-void ContentDirectoryLoaderFactory::Clone(
-    mojo::PendingReceiver<network::mojom::URLLoaderFactory> loader) {
-  receivers_.Add(this, std::move(loader));
-}
-
 void ContentDirectoryLoaderFactory::SetContentDirectoriesForTest(
     std::vector<fuchsia::web::ContentDirectoryProvider> directories) {
   ContentDirectoriesMap* current_process_directories = GetContentDirectories();

fuchsia/engine/browser/content_directory_loader_factory.h

@@ -12,25 +12,36 @@
 #include <string>
 #include <vector>
 
+#include "content/public/browser/non_network_url_loader_factory_base.h"
 #include "fuchsia/engine/web_engine_export.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"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 
-// Creates a URLLoaderFactory which services requests for resources stored
-// under named directories. The directories are accessed using the
-// fuchsia-dir:// scheme.
-class ContentDirectoryLoaderFactory : public network::mojom::URLLoaderFactory {
+// URLLoaderFactory which services requests for resources stored under named
+// directories. The directories are accessed using the fuchsia-dir:// scheme.
+class ContentDirectoryLoaderFactory
+    : public content::NonNetworkURLLoaderFactoryBase {
  public:
-  ContentDirectoryLoaderFactory();
-  ~ContentDirectoryLoaderFactory() override;
+  // Returns mojo::PendingRemote to a newly constructed
+  // ContentDirectoryLoaderFactory.  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).
+  static mojo::PendingRemote<network::mojom::URLLoaderFactory> Create();
 
   // Sets the list of content directories for the duration of the process.
   // Can be called multiple times for clearing or replacing the list.
   static WEB_ENGINE_EXPORT void SetContentDirectoriesForTest(
       std::vector<fuchsia::web::ContentDirectoryProvider> directories);
 
+ private:
+  explicit ContentDirectoryLoaderFactory(
+      mojo::PendingReceiver<network::mojom::URLLoaderFactory> factory_receiver);
+
+  ~ContentDirectoryLoaderFactory() override;
+
   // network::mojom::URLLoaderFactory:
   void CreateLoaderAndStart(
       mojo::PendingReceiver<network::mojom::URLLoader> loader,
@@ -40,10 +51,7 @@ class ContentDirectoryLoaderFactory : public network::mojom::URLLoaderFactory {
       const network::ResourceRequest& request,
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) final;
-  void Clone(
-      mojo::PendingReceiver<network::mojom::URLLoaderFactory> loader) final;
 
- private:
   net::Error OpenFileFromDirectory(
       const std::string& directory_name,
       base::FilePath path,
@@ -52,8 +60,6 @@ class ContentDirectoryLoaderFactory : public network::mojom::URLLoaderFactory {
   // Used for executing blocking URLLoader routines.
   const scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
-  mojo::ReceiverSet<network::mojom::URLLoaderFactory> receivers_;
-
   DISALLOW_COPY_AND_ASSIGN(ContentDirectoryLoaderFactory);
 };
 

fuchsia/engine/browser/web_engine_content_browser_client.cc

@@ -136,8 +136,8 @@ void WebEngineContentBrowserClient::
         NonNetworkURLLoaderFactoryMap* factories) {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kContentDirectories)) {
-    (*uniquely_owned_factories)[cr_fuchsia::kFuchsiaDirScheme] =
-        std::make_unique<ContentDirectoryLoaderFactory>();
+    factories->emplace(cr_fuchsia::kFuchsiaDirScheme,
+                       ContentDirectoryLoaderFactory::Create());
   }
 }
 
@@ -149,8 +149,8 @@ void WebEngineContentBrowserClient::
         NonNetworkURLLoaderFactoryMap* factories) {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kContentDirectories)) {
-    (*uniquely_owned_factories)[cr_fuchsia::kFuchsiaDirScheme] =
-        std::make_unique<ContentDirectoryLoaderFactory>();
+    factories->emplace(cr_fuchsia::kFuchsiaDirScheme,
+                       ContentDirectoryLoaderFactory::Create());
   }
 }