[FileAPI] Refactor how BlobURLLoaderFactory works.

Rather than resolving a mojo Blob to a BlobDataHandle, just forward the
request to the Blob and let the blob create the URL Loader. This way we
decouple BlobURLLoaderFactory from any blob internals, paving the way
for further separating out the blob URL registry from the rest of the
blob system, making it easier to change where parts of the blob system
live (for example moving Blob URLs to be per storage partition, or in
the future perhaps per agent cluster).

This does increase binary size significantly because the added mojom
method results in java bindings being generated for a lot of interfaces
and structs that were previously not generated. In the future this
increase can be eliminated by making it possible to tag methods in
mojom files with what languages they should create bindings for, or by
rewriting BlobURLLoader itself to operate on a mojo Blob, rather than
forward the entire URLRequest to the mojo blob (https://crbug.com/1111835).

Bug: 1106890
Binary-Size: Size increase is unavoidable (see above).
Change-Id: I4fa3c6a5ddf6f8be5ce299e9d1fd95eaef75ec5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2330311Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795695}

content/browser/blob_storage/chrome_blob_storage_context.cc

@@ -248,8 +248,7 @@ ChromeBlobStorageContext::URLLoaderFactoryForUrl(
              mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver,
              const GURL& url) {
             auto blob_remote = context->context()->GetBlobFromPublicURL(url);
-            storage::BlobURLLoaderFactory::Create(
-                std::move(blob_remote), url, context->context()->AsWeakPtr(),
+            storage::BlobURLLoaderFactory::Create(std::move(blob_remote), url,
                                                   std::move(receiver));
           },
           base::WrapRefCounted(GetFor(browser_context)),

content/renderer/service_worker/service_worker_subresource_loader_unittest.cc

@@ -74,6 +74,12 @@ class FakeBlob final : public blink::mojom::Blob {
       client_remote->OnComplete(net::OK, body_.size());
     }
   }
+  void Load(mojo::PendingReceiver<network::mojom::URLLoader>,
+            const std::string& method,
+            const net::HttpRequestHeaders&,
+            mojo::PendingRemote<network::mojom::URLLoaderClient>) override {
+    NOTREACHED();
+  }
   void ReadSideData(ReadSideDataCallback callback) override {
     std::move(callback).Run(side_data_);
   }

storage/browser/blob/blob_impl.cc

@@ -19,6 +19,7 @@
 #include "storage/browser/blob/blob_data_handle.h"
 #include "storage/browser/blob/blob_data_item.h"
 #include "storage/browser/blob/blob_data_snapshot.h"
+#include "storage/browser/blob/blob_url_loader.h"
 #include "storage/browser/blob/mojo_blob_reader.h"
 
 namespace storage {
@@ -126,6 +127,16 @@ void BlobImpl::ReadAll(
                          std::move(handle));
 }
 
+void BlobImpl::Load(
+    mojo::PendingReceiver<network::mojom::URLLoader> loader,
+    const std::string& method,
+    const net::HttpRequestHeaders& headers,
+    mojo::PendingRemote<network::mojom::URLLoaderClient> client) {
+  BlobURLLoader::CreateAndStart(std::move(loader), method, headers,
+                                std::move(client),
+                                std::make_unique<BlobDataHandle>(*handle_));
+}
+
 void BlobImpl::ReadSideData(ReadSideDataCallback callback) {
   handle_->RunOnConstructionComplete(base::BindOnce(
       [](BlobDataHandle handle, ReadSideDataCallback callback,

storage/browser/blob/blob_impl.h

@@ -42,6 +42,11 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobImpl
   void ReadAll(
       mojo::ScopedDataPipeProducerHandle handle,
       mojo::PendingRemote<blink::mojom::BlobReaderClient> client) override;
+  void Load(
+      mojo::PendingReceiver<network::mojom::URLLoader> loader,
+      const std::string& method,
+      const net::HttpRequestHeaders& headers,
+      mojo::PendingRemote<network::mojom::URLLoaderClient> client) override;
   void ReadSideData(ReadSideDataCallback callback) override;
   void CaptureSnapshot(CaptureSnapshotCallback callback) override;
   void GetInternalUUID(GetInternalUUIDCallback callback) override;

storage/browser/blob/blob_url_loader.cc

@@ -84,15 +84,27 @@ void BlobURLLoader::CreateAndStart(
     const network::ResourceRequest& request,
     mojo::PendingRemote<network::mojom::URLLoaderClient> client,
     std::unique_ptr<BlobDataHandle> blob_handle) {
-  new BlobURLLoader(std::move(url_loader_receiver), request, std::move(client),
-                    std::move(blob_handle));
+  new BlobURLLoader(std::move(url_loader_receiver), request.method,
+                    request.headers, std::move(client), std::move(blob_handle));
+}
+
+// static
+void BlobURLLoader::CreateAndStart(
+    mojo::PendingReceiver<network::mojom::URLLoader> url_loader_receiver,
+    const std::string& method,
+    const net::HttpRequestHeaders& headers,
+    mojo::PendingRemote<network::mojom::URLLoaderClient> client,
+    std::unique_ptr<BlobDataHandle> blob_handle) {
+  new BlobURLLoader(std::move(url_loader_receiver), method, headers,
+                    std::move(client), std::move(blob_handle));
 }
 
 BlobURLLoader::~BlobURLLoader() = default;
 
 BlobURLLoader::BlobURLLoader(
     mojo::PendingReceiver<network::mojom::URLLoader> url_loader_receiver,
-    const network::ResourceRequest& request,
+    const std::string& method,
+    const net::HttpRequestHeaders& headers,
     mojo::PendingRemote<network::mojom::URLLoaderClient> client,
     std::unique_ptr<BlobDataHandle> blob_handle)
     : receiver_(this, std::move(url_loader_receiver)),
@@ -101,10 +113,11 @@ BlobURLLoader::BlobURLLoader(
   // PostTask since it might destruct.
   base::SequencedTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(&BlobURLLoader::Start,
-                                weak_factory_.GetWeakPtr(), request));
+                                weak_factory_.GetWeakPtr(), method, headers));
 }
 
-void BlobURLLoader::Start(const network::ResourceRequest& request) {
+void BlobURLLoader::Start(const std::string& method,
+                          const net::HttpRequestHeaders& headers) {
   if (!blob_handle_) {
     OnComplete(net::ERR_FILE_NOT_FOUND, 0);
     delete this;
@@ -112,15 +125,14 @@ void BlobURLLoader::Start(const network::ResourceRequest& request) {
   }
 
   // We only support GET request per the spec.
-  if (request.method != "GET") {
+  if (method != "GET") {
     OnComplete(net::ERR_METHOD_NOT_SUPPORTED, 0);
     delete this;
     return;
   }
 
   std::string range_header;
-  if (request.headers.GetHeader(net::HttpRequestHeaders::kRange,
-                                &range_header)) {
+  if (headers.GetHeader(net::HttpRequestHeaders::kRange, &range_header)) {
     // We only care about "Range" header here.
     std::vector<net::HttpByteRange> ranges;
     if (net::HttpUtil::ParseRangeHeader(range_header, &ranges)) {

storage/browser/blob/blob_url_loader.h

@@ -36,6 +36,12 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLLoader
       const network::ResourceRequest& request,
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       std::unique_ptr<BlobDataHandle> blob_handle);
+  static void CreateAndStart(
+      mojo::PendingReceiver<network::mojom::URLLoader> url_loader_receiver,
+      const std::string& method,
+      const net::HttpRequestHeaders& headers,
+      mojo::PendingRemote<network::mojom::URLLoaderClient> client,
+      std::unique_ptr<BlobDataHandle> blob_handle);
   ~BlobURLLoader() override;
 
  private:
@@ -44,8 +50,14 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLLoader
       const network::ResourceRequest& request,
       mojo::PendingRemote<network::mojom::URLLoaderClient> client,
       std::unique_ptr<BlobDataHandle> blob_handle);
+  BlobURLLoader(
+      mojo::PendingReceiver<network::mojom::URLLoader> url_loader_receiver,
+      const std::string& method,
+      const net::HttpRequestHeaders& headers,
+      mojo::PendingRemote<network::mojom::URLLoaderClient> client,
+      std::unique_ptr<BlobDataHandle> blob_handle);
 
-  void Start(const network::ResourceRequest& request);
+  void Start(const std::string& method, const net::HttpRequestHeaders& headers);
 
   // network::mojom::URLLoader implementation:
   void FollowRedirect(

storage/browser/blob/blob_url_loader_factory.cc

@@ -7,9 +7,8 @@
 #include "base/bind.h"
 #include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/bindings/remote.h"
-#include "storage/browser/blob/blob_data_handle.h"
+#include "services/network/public/mojom/url_loader.mojom.h"
 #include "storage/browser/blob/blob_storage_context.h"
-#include "storage/browser/blob/blob_url_loader.h"
 
 namespace storage {
 
@@ -27,8 +26,7 @@ void CreateFactoryForToken(
   GURL blob_url;
   if (context)
     context->registry().GetTokenMapping(token, &blob_url, &blob);
-  BlobURLLoaderFactory::Create(std::move(blob), blob_url, context,
-                               std::move(receiver));
+  BlobURLLoaderFactory::Create(std::move(blob), blob_url, std::move(receiver));
 }
 
 }  // namespace
@@ -37,32 +35,9 @@ void CreateFactoryForToken(
 void BlobURLLoaderFactory::Create(
     mojo::PendingRemote<blink::mojom::Blob> pending_blob,
     const GURL& blob_url,
-    base::WeakPtr<BlobStorageContext> context,
     mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver) {
-  if (!pending_blob) {
-    new BlobURLLoaderFactory(nullptr, blob_url, std::move(receiver));
-    return;
-  }
-  // Not every URLLoaderFactory user deals with the URLLoaderFactory simply
-  // disconnecting very well, so make sure we always at least bind the receiver
-  // to some factory that can then fail with a network error. Hence the callback
-  // is wrapped in WrapCallbackWithDefaultInvokeIfNotRun.
-  mojo::Remote<blink::mojom::Blob> blob(std::move(pending_blob));
-  blink::mojom::Blob* raw_blob = blob.get();
-  raw_blob->GetInternalUUID(mojo::WrapCallbackWithDefaultInvokeIfNotRun(
-      base::BindOnce(
-          [](mojo::Remote<blink::mojom::Blob>,
-             base::WeakPtr<BlobStorageContext> context, const GURL& blob_url,
-             mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver,
-             const std::string& uuid) {
-            std::unique_ptr<BlobDataHandle> blob;
-            if (context)
-              blob = context->GetBlobDataFromUUID(uuid);
-            new BlobURLLoaderFactory(std::move(blob), blob_url,
+  new BlobURLLoaderFactory(std::move(pending_blob), blob_url,
                            std::move(receiver));
-          },
-          std::move(blob), std::move(context), blob_url, std::move(receiver)),
-      ""));
 }
 
 // static
@@ -96,9 +71,14 @@ void BlobURLLoaderFactory::CreateLoaderAndStart(
         ->OnComplete(network::URLLoaderCompletionStatus(net::ERR_INVALID_URL));
     return;
   }
-  BlobURLLoader::CreateAndStart(
-      std::move(loader), request, std::move(client),
-      handle_ ? std::make_unique<BlobDataHandle>(*handle_) : nullptr);
+  if (!blob_) {
+    mojo::Remote<network::mojom::URLLoaderClient>(std::move(client))
+        ->OnComplete(
+            network::URLLoaderCompletionStatus(net::ERR_FILE_NOT_FOUND));
+    return;
+  }
+  blob_->Load(std::move(loader), request.method, request.headers,
+              std::move(client));
 }
 
 void BlobURLLoaderFactory::Clone(
@@ -107,10 +87,10 @@ void BlobURLLoaderFactory::Clone(
 }
 
 BlobURLLoaderFactory::BlobURLLoaderFactory(
-    std::unique_ptr<BlobDataHandle> handle,
+    mojo::PendingRemote<blink::mojom::Blob> blob,
     const GURL& blob_url,
     mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver)
-    : handle_(std::move(handle)), url_(blob_url) {
+    : blob_(std::move(blob)), url_(blob_url) {
   receivers_.Add(this, std::move(receiver));
   receivers_.set_disconnect_handler(base::BindRepeating(
       &BlobURLLoaderFactory::OnConnectionError, base::Unretained(this)));

storage/browser/blob/blob_url_loader_factory.h

@@ -10,11 +10,11 @@
 #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"
+#include "third_party/blink/public/mojom/blob/blob.mojom.h"
 #include "third_party/blink/public/mojom/blob/blob_url_store.mojom.h"
 
 namespace storage {
 
-class BlobDataHandle;
 class BlobStorageContext;
 
 // URLLoaderFactory that can create loaders for exactly one url, loading the
@@ -26,7 +26,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLLoaderFactory
   static void Create(
       mojo::PendingRemote<blink::mojom::Blob> blob,
       const GURL& blob_url,
-      base::WeakPtr<BlobStorageContext> context,
       mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver);
 
   // Creates a factory for a BlobURLToken. The token is used to look up the blob
@@ -52,13 +51,13 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLLoaderFactory
 
  private:
   BlobURLLoaderFactory(
-      std::unique_ptr<BlobDataHandle> handle,
+      mojo::PendingRemote<blink::mojom::Blob> blob,
       const GURL& blob_url,
       mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver);
   ~BlobURLLoaderFactory() override;
   void OnConnectionError();
 
-  std::unique_ptr<BlobDataHandle> handle_;
+  mojo::Remote<blink::mojom::Blob> blob_;
   GURL url_;
 
   mojo::ReceiverSet<network::mojom::URLLoaderFactory> receivers_;

storage/browser/blob/blob_url_store_impl.cc

@@ -132,7 +132,7 @@ void BlobURLStoreImpl::ResolveAsURLLoaderFactory(
     mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver) {
   BlobURLLoaderFactory::Create(
       context_ ? context_->GetBlobFromPublicURL(url) : mojo::NullRemote(), url,
-      context_, std::move(receiver));
+      std::move(receiver));
 }
 
 void BlobURLStoreImpl::ResolveForNavigation(

storage/browser/test/fake_blob.cc

@@ -25,6 +25,7 @@ void FakeBlob::AsDataPipeGetter(
     mojo::PendingReceiver<network::mojom::DataPipeGetter>) {
   NOTREACHED();
 }
+
 void FakeBlob::ReadRange(uint64_t offset,
                          uint64_t size,
                          mojo::ScopedDataPipeProducerHandle,
@@ -37,6 +38,13 @@ void FakeBlob::ReadAll(mojo::ScopedDataPipeProducerHandle,
   NOTREACHED();
 }
 
+void FakeBlob::Load(mojo::PendingReceiver<network::mojom::URLLoader>,
+                    const std::string& method,
+                    const net::HttpRequestHeaders&,
+                    mojo::PendingRemote<network::mojom::URLLoaderClient>) {
+  NOTREACHED();
+}
+
 void FakeBlob::ReadSideData(ReadSideDataCallback) {
   NOTREACHED();
 }

storage/browser/test/fake_blob.h

@@ -26,6 +26,10 @@ class FakeBlob : public blink::mojom::Blob {
                  mojo::PendingRemote<blink::mojom::BlobReaderClient>) override;
   void ReadAll(mojo::ScopedDataPipeProducerHandle,
                mojo::PendingRemote<blink::mojom::BlobReaderClient>) override;
+  void Load(mojo::PendingReceiver<network::mojom::URLLoader>,
+            const std::string& method,
+            const net::HttpRequestHeaders&,
+            mojo::PendingRemote<network::mojom::URLLoaderClient>) override;
   void ReadSideData(ReadSideDataCallback) override;
   void CaptureSnapshot(CaptureSnapshotCallback) override;
   void GetInternalUUID(GetInternalUUIDCallback callback) override;

third_party/blink/public/mojom/blob/blob.mojom

@@ -8,6 +8,7 @@ import "mojo/public/mojom/base/big_buffer.mojom";
 import "mojo/public/mojom/base/time.mojom";
 import "services/network/public/mojom/data_pipe_getter.mojom";
 import "services/network/public/mojom/http_request_headers.mojom";
+import "services/network/public/mojom/url_loader.mojom";
 
 // Interface that can be implemented to be informed of certain information while
 // reading the data for a blob.
@@ -49,6 +50,15 @@ interface Blob {
   ReadRange(uint64 offset, uint64 length, handle<data_pipe_producer> pipe,
             pending_remote<BlobReaderClient>? client);
 
+  // Provides a response to a request for a blob: URL to a URLLoaderClient.
+  // This takes into account optional Range request headers, but otherwise
+  // ignores the headers. Also verifies the |request_method| is a supported
+  // http request method for blob: URLs, and replies with an error if not.
+  Load(pending_receiver<network.mojom.URLLoader> loader,
+       string request_method,
+       network.mojom.HttpRequestHeaders headers,
+       pending_remote<network.mojom.URLLoaderClient> client);
+
   // Reads the side-data (if any) associated with this blob. This is the same
   // data that would be passed to OnReceivedCachedMetadata if you were reading
   // this blob through a blob URL.

third_party/blink/renderer/platform/blob/testing/fake_blob.cc

@@ -76,6 +76,14 @@ void FakeBlob::ReadAll(
     client_remote->OnComplete(0 /* OK */, body_.length());
 }
 
+void FakeBlob::Load(
+    mojo::PendingReceiver<network::mojom::blink::URLLoader>,
+    const String& method,
+    const net::HttpRequestHeaders&,
+    mojo::PendingRemote<network::mojom::blink::URLLoaderClient>) {
+  NOTREACHED();
+}
+
 void FakeBlob::ReadSideData(ReadSideDataCallback callback) {
   NOTREACHED();
 }

third_party/blink/renderer/platform/blob/testing/fake_blob.h

@@ -31,6 +31,11 @@ class FakeBlob : public mojom::blink::Blob {
                  mojo::PendingRemote<mojom::blink::BlobReaderClient>) override;
   void ReadAll(mojo::ScopedDataPipeProducerHandle,
                mojo::PendingRemote<mojom::blink::BlobReaderClient>) override;
+  void Load(
+      mojo::PendingReceiver<network::mojom::blink::URLLoader>,
+      const String& method,
+      const net::HttpRequestHeaders&,
+      mojo::PendingRemote<network::mojom::blink::URLLoaderClient>) override;
   void ReadSideData(ReadSideDataCallback) override;
   void CaptureSnapshot(CaptureSnapshotCallback) override;
   void GetInternalUUID(GetInternalUUIDCallback) override;