[Mojo Blob URLs] Refactor Blob URL loading for subresources.

This changes the XHR and fetch codepaths to resolve blob URLs earlier
(i.e when the request is created rather than when the fetch is made), in
order to fix bugs where the blob URL is revoked after creating a request
but before fetching.

This also addresses race conditions between revoking blob URLs and
fetching them by resolving a blob URL as soon as possible for subresource
requests.

Not addressed yet are navigation and download requests, that will be
done in follow ups.

Bug: 800901, 800898, 695031, 807435
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ifd5eb11abaf04c091c51e607fdaeafa0c89bc1a9
Reviewed-on: https://chromium-review.googlesource.com/893606
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535845}

content/renderer/renderer_blink_platform_impl.cc

@@ -34,6 +34,7 @@
 #include "content/common/gpu_stream_constants.h"
 #include "content/common/origin_trials/trial_policy_impl.h"
 #include "content/common/render_message_filter.mojom.h"
+#include "content/common/wrapper_shared_url_loader_factory.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/service_manager_connection.h"
@@ -345,6 +346,17 @@ RendererBlinkPlatformImpl::CreateDefaultURLLoaderFactory() {
       CreateDefaultURLLoaderFactoryBundle());
 }
 
+std::unique_ptr<blink::WebURLLoaderFactory>
+RendererBlinkPlatformImpl::WrapURLLoaderFactory(
+    mojo::ScopedMessagePipeHandle url_loader_factory_handle) {
+  return std::make_unique<content::WebURLLoaderFactoryImpl>(
+      RenderThreadImpl::current()->resource_dispatcher()->GetWeakPtr(),
+      base::MakeRefCounted<WrapperSharedURLLoaderFactory>(
+          network::mojom::URLLoaderFactoryPtrInfo(
+              std::move(url_loader_factory_handle),
+              network::mojom::URLLoaderFactory::Version_)));
+}
+
 std::unique_ptr<blink::WebDataConsumerHandle>
 RendererBlinkPlatformImpl::CreateDataConsumerHandle(
     mojo::ScopedDataPipeConsumerHandle handle) {

content/renderer/renderer_blink_platform_impl.h

@@ -250,6 +250,8 @@ class CONTENT_EXPORT RendererBlinkPlatformImpl : public BlinkPlatformImpl {
 
   std::unique_ptr<blink::WebURLLoaderFactory> CreateDefaultURLLoaderFactory()
       override;
+  std::unique_ptr<blink::WebURLLoaderFactory> WrapURLLoaderFactory(
+      mojo::ScopedMessagePipeHandle url_loader_factory_handle) override;
   std::unique_ptr<blink::WebDataConsumerHandle> CreateDataConsumerHandle(
       mojo::ScopedDataPipeConsumerHandle handle) override;
   void RequestPurgeMemory() override;

content/renderer/service_worker/service_worker_fetch_context_impl.cc

@@ -5,6 +5,7 @@
 #include "content/renderer/service_worker/service_worker_fetch_context_impl.h"
 
 #include "base/feature_list.h"
+#include "content/common/wrapper_shared_url_loader_factory.h"
 #include "content/public/common/content_features.h"
 #include "content/public/renderer/url_loader_throttle_provider.h"
 #include "content/renderer/loader/request_extra_data.h"
@@ -41,6 +42,17 @@ ServiceWorkerFetchContextImpl::CreateURLLoaderFactory() {
       resource_dispatcher_->GetWeakPtr(), std::move(url_loader_factory_));
 }
 
+std::unique_ptr<blink::WebURLLoaderFactory>
+ServiceWorkerFetchContextImpl::WrapURLLoaderFactory(
+    mojo::ScopedMessagePipeHandle url_loader_factory_handle) {
+  return std::make_unique<content::WebURLLoaderFactoryImpl>(
+      resource_dispatcher_->GetWeakPtr(),
+      base::MakeRefCounted<WrapperSharedURLLoaderFactory>(
+          network::mojom::URLLoaderFactoryPtrInfo(
+              std::move(url_loader_factory_handle),
+              network::mojom::URLLoaderFactory::Version_)));
+}
+
 void ServiceWorkerFetchContextImpl::WillSendRequest(
     blink::WebURLRequest& request) {
   RequestExtraData* extra_data = new RequestExtraData();

content/renderer/service_worker/service_worker_fetch_context_impl.h

@@ -25,6 +25,8 @@ class ServiceWorkerFetchContextImpl : public blink::WebWorkerFetchContext {
   // blink::WebWorkerFetchContext implementation:
   void InitializeOnWorkerThread() override;
   std::unique_ptr<blink::WebURLLoaderFactory> CreateURLLoaderFactory() override;
+  std::unique_ptr<blink::WebURLLoaderFactory> WrapURLLoaderFactory(
+      mojo::ScopedMessagePipeHandle url_loader_factory_handle) override;
   void WillSendRequest(blink::WebURLRequest&) override;
   bool IsControlledByServiceWorker() const override;
   blink::WebURL SiteForCookies() const override;

content/renderer/service_worker/worker_fetch_context_impl.cc

@@ -152,6 +152,17 @@ WorkerFetchContextImpl::CreateURLLoaderFactory() {
   return factory;
 }
 
+std::unique_ptr<blink::WebURLLoaderFactory>
+WorkerFetchContextImpl::WrapURLLoaderFactory(
+    mojo::ScopedMessagePipeHandle url_loader_factory_handle) {
+  return std::make_unique<content::WebURLLoaderFactoryImpl>(
+      resource_dispatcher_->GetWeakPtr(),
+      base::MakeRefCounted<WrapperSharedURLLoaderFactory>(
+          network::mojom::URLLoaderFactoryPtrInfo(
+              std::move(url_loader_factory_handle),
+              network::mojom::URLLoaderFactory::Version_)));
+}
+
 void WorkerFetchContextImpl::WillSendRequest(blink::WebURLRequest& request) {
   RequestExtraData* extra_data = new RequestExtraData();
   extra_data->set_service_worker_provider_id(service_worker_provider_id_);

content/renderer/service_worker/worker_fetch_context_impl.h

@@ -46,6 +46,8 @@ class WorkerFetchContextImpl : public blink::WebWorkerFetchContext,
   // blink::WebWorkerFetchContext implementation:
   void InitializeOnWorkerThread() override;
   std::unique_ptr<blink::WebURLLoaderFactory> CreateURLLoaderFactory() override;
+  std::unique_ptr<blink::WebURLLoaderFactory> WrapURLLoaderFactory(
+      mojo::ScopedMessagePipeHandle url_loader_factory_handle) override;
   void WillSendRequest(blink::WebURLRequest&) override;
   bool IsControlledByServiceWorker() const override;
   void SetIsOnSubframe(bool) override;

third_party/WebKit/LayoutTests/FlagExpectations/site-per-process

@@ -133,6 +133,7 @@ Bug(none) external/wpt/html/browsers/origin/relaxing-the-same-origin-restriction
 
 # TODO(lukasza, alexmos): Triage these failures.
 Bug(none) external/wpt/FileAPI/url/multi-global-origin-serialization.sub.html [ Failure ]
+Bug(none) virtual/mojo-blob-urls/external/wpt/FileAPI/url/multi-global-origin-serialization.sub.html [ Failure ]
 Bug(none) external/wpt/IndexedDB/interleaved-cursors.html [ Timeout ]
 Bug(none) external/wpt/xhr/xmlhttprequest-sync-default-feature-policy.sub.html [ Timeout ]
 Bug(none) external/wpt/content-security-policy/frame-ancestors/frame-ancestors-nested-cross-in-cross-none-block.html [ Timeout ]

third_party/WebKit/LayoutTests/NeverFixTests

@@ -769,6 +769,7 @@ external/wpt/entries-api/interfaces-manual.html [ WontFix ]
 external/wpt/FileAPI/FileReader/test_notreadableerrors-manual.html [ WontFix ]
 external/wpt/FileAPI/FileReader/test_securityerrors-manual.html [ WontFix ]
 external/wpt/FileAPI/url/url_createobjecturl_file_img-manual.html [ WontFix ]
+virtual/mojo-blob-urls/external/wpt/FileAPI/url/url_createobjecturl_file_img-manual.html [ WontFix ]
 external/wpt/geolocation-API/getCurrentPosition_permission_allow-manual.html [ WontFix ]
 external/wpt/geolocation-API/getCurrentPosition_permission_deny-manual.html [ WontFix ]
 external/wpt/geolocation-API/getCurrentPosition_permission-manual.html [ WontFix ]

third_party/WebKit/LayoutTests/TestExpectations

@@ -1680,6 +1680,7 @@ crbug.com/800898 external/wpt/FileAPI/url/url-in-tags-revoke.window.html [ Pass
 crbug.com/800898 virtual/mojo-blobs/external/wpt/FileAPI/url/url-with-fetch.any.worker.html [ Pass Failure ]
 crbug.com/800898 virtual/mojo-blobs/external/wpt/FileAPI/url/url-with-xhr.any.worker.html [ Pass Failure ]
 crbug.com/800898 virtual/mojo-blobs/external/wpt/FileAPI/url/url-in-tags-revoke.window.html [ Pass Failure ]
+crbug.com/800898 virtual/mojo-blob-urls/external/wpt/FileAPI/url/url-in-tags-revoke.window.html [ Pass Failure ]
 
 # Websockets
 crbug.com/803558 external/wpt/websockets/Create-Secure-extensions-empty.htm [ Timeout ]

third_party/WebKit/LayoutTests/VirtualTestSuites

@@ -613,5 +613,10 @@
     "prefix": "navigation-mojo-response",
     "base": "external/wpt/service-workers/service-worker",
     "args": ["--enable-features=NavigationMojoResponse"]
+  },
+  {
+    "prefix": "mojo-blob-urls",
+    "base": "external/wpt/FileAPI/url",
+    "args": ["--enable-blink-features=MojoBlobURLs"]
   }
 ]

third_party/WebKit/LayoutTests/virtual/mojo-blob-urls/external/wpt/FileAPI/url/README.txt

@@ -0,0 +1 @@
+This directory is for testing blob URLs over mojo.

third_party/WebKit/LayoutTests/virtual/mojo-blob-urls/external/wpt/FileAPI/url/cross-global-revoke.sub-expected.txt

@@ -0,0 +1,6 @@
+This is a testharness.js-based test.
+PASS It is possible to revoke same-origin blob URLs from different frames.
+PASS It is possible to revoke same-origin blob URLs from a different worker global.
+PASS It is not possible to revoke cross-origin blob URLs.
+Harness: the test ran to completion.
+

third_party/WebKit/LayoutTests/virtual/mojo-blob-urls/external/wpt/FileAPI/url/url-with-fetch.any-expected.txt

@@ -0,0 +1,18 @@
+This is a testharness.js-based test.
+PASS Blob URLs can be used in fetch
+PASS fetch with a fragment should succeed
+PASS fetch of a revoked URL should fail
+PASS Only exact matches should revoke URLs, using fetch
+PASS Appending a query string should cause fetch to fail
+PASS Appending a path should cause fetch to fail
+PASS fetch with method "HEAD" should fail
+PASS fetch with method "POST" should fail
+PASS fetch with method "DELETE" should fail
+PASS fetch with method "OPTIONS" should fail
+PASS fetch with method "PUT" should fail
+PASS fetch with method "CUSTOM" should fail
+PASS fetch should return Content-Type from Blob
+PASS Revoke blob URL after creating Request, will fetch
+PASS Revoke blob URL after calling fetch, fetch should succeed
+Harness: the test ran to completion.
+

third_party/WebKit/LayoutTests/virtual/mojo-blob-urls/external/wpt/FileAPI/url/url-with-xhr.any-expected.txt

@@ -0,0 +1,17 @@
+This is a testharness.js-based test.
+PASS Blob URLs can be used in XHR
+PASS XHR with a fragment should succeed
+PASS XHR of a revoked URL should fail
+PASS Only exact matches should revoke URLs, using XHR
+PASS Appending a query string should cause XHR to fail
+PASS Appending a path should cause XHR to fail
+PASS XHR with method "HEAD" should fail
+PASS XHR with method "POST" should fail
+PASS XHR with method "DELETE" should fail
+PASS XHR with method "OPTIONS" should fail
+PASS XHR with method "PUT" should fail
+PASS XHR with method "CUSTOM" should fail
+PASS XHR should return Content-Type from Blob
+PASS Revoke blob URL after open(), will fetch
+Harness: the test ran to completion.
+

third_party/WebKit/Source/core/fetch/FetchManager.cpp

@@ -752,6 +752,7 @@ void FetchManager::Loader::PerformHTTPFetch() {
     }
     request.SetKeepalive(true);
   }
+
   // "3. Append `Host`, ..."
   // FIXME: Implement this when the spec is fixed.
 
@@ -768,6 +769,13 @@ void FetchManager::Loader::PerformHTTPFetch() {
   ResourceLoaderOptions resource_loader_options;
   resource_loader_options.data_buffering_policy = kDoNotBufferData;
   resource_loader_options.security_origin = request_->Origin().get();
+  if (request_->URLLoaderFactory()) {
+    network::mojom::blink::URLLoaderFactoryPtr factory_clone;
+    request_->URLLoaderFactory()->Clone(MakeRequest(&factory_clone));
+    resource_loader_options.url_loader_factory = base::MakeRefCounted<
+        base::RefCountedData<network::mojom::blink::URLLoaderFactoryPtr>>(
+        std::move(factory_clone));
+  }
 
   ThreadableLoaderOptions threadable_loader_options;
 

third_party/WebKit/Source/core/fetch/FetchRequestData.cpp

@@ -87,6 +87,9 @@ FetchRequestData* FetchRequestData::Clone(ScriptState* script_state) {
     buffer_ = new1;
     request->buffer_ = new2;
   }
+  if (url_loader_factory_) {
+    url_loader_factory_->Clone(MakeRequest(&request->url_loader_factory_));
+  }
   return request;
 }
 
@@ -97,6 +100,7 @@ FetchRequestData* FetchRequestData::Pass(ScriptState* script_state) {
     buffer_ = new BodyStreamBuffer(script_state, BytesConsumer::CreateClosed());
     buffer_->CloseAndLockAndDisturb();
   }
+  request->url_loader_factory_ = std::move(url_loader_factory_);
   return request;
 }
 

third_party/WebKit/Source/core/fetch/FetchRequestData.h

@@ -17,6 +17,7 @@
 #include "public/platform/modules/fetch/fetch_api_request.mojom-shared.h"
 #include "public/platform/modules/serviceworker/WebServiceWorkerRequest.h"
 #include "services/network/public/interfaces/fetch_api.mojom-blink.h"
+#include "services/network/public/interfaces/url_loader_factory.mojom-blink.h"
 
 namespace blink {
 
@@ -90,6 +91,13 @@ class FetchRequestData final
   bool Keepalive() const { return keepalive_; }
   void SetKeepalive(bool b) { keepalive_ = b; }
 
+  network::mojom::blink::URLLoaderFactory* URLLoaderFactory() const {
+    return url_loader_factory_.get();
+  }
+  void SetURLLoaderFactory(network::mojom::blink::URLLoaderFactoryPtr factory) {
+    url_loader_factory_ = std::move(factory);
+  }
+
   // We use these strings instead of "no-referrer" and "client" in the spec.
   static AtomicString NoReferrerString() { return AtomicString(); }
   static AtomicString ClientReferrerString() {
@@ -131,6 +139,11 @@ class FetchRequestData final
   String mime_type_;
   String integrity_;
   bool keepalive_;
+  // A specific factory that should be used for this request instead of whatever
+  // the system would otherwise decide to use to load this request.
+  // Currently used for blob: URLs, to ensure they can still be loaded even if
+  // the URL got revoked after creating the request.
+  network::mojom::blink::URLLoaderFactoryPtr url_loader_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(FetchRequestData);
 };

third_party/WebKit/Source/core/fetch/Request.cpp

@@ -9,6 +9,7 @@
 #include "core/fetch/BodyStreamBuffer.h"
 #include "core/fetch/FetchManager.h"
 #include "core/fetch/RequestInit.h"
+#include "core/fileapi/PublicURLManager.h"
 #include "core/loader/ThreadableLoader.h"
 #include "platform/bindings/V8PrivateProperty.h"
 #include "platform/loader/cors/CORS.h"
@@ -49,6 +50,11 @@ FetchRequestData* CreateCopyOfFetchRequestDataForFetch(
   request->SetRedirect(original->Redirect());
   request->SetIntegrity(original->Integrity());
   request->SetKeepalive(original->Keepalive());
+  if (original->URLLoaderFactory()) {
+    network::mojom::blink::URLLoaderFactoryPtr factory_clone;
+    original->URLLoaderFactory()->Clone(MakeRequest(&factory_clone));
+    request->SetURLLoaderFactory(std::move(factory_clone));
+  }
   return request;
 }
 
@@ -129,6 +135,18 @@ Request* Request::CreateRequestWithRequestOrString(
     // single URL with a copy of |parsedURL|."
     request->SetURL(parsed_url);
 
+    // Parsing URLs should also resolve blob URLs. This is important because
+    // fetching of a blob URL should work even after the URL is revoked as long
+    // as the request was created while the URL was still valid.
+    if (parsed_url.ProtocolIs("blob") &&
+        RuntimeEnabledFeatures::MojoBlobURLsEnabled()) {
+      network::mojom::blink::URLLoaderFactoryPtr url_loader_factory;
+      ExecutionContext::From(script_state)
+          ->GetPublicURLManager()
+          .Resolve(parsed_url, MakeRequest(&url_loader_factory));
+      request->SetURLLoaderFactory(std::move(url_loader_factory));
+    }
+
     // We don't use fallback values. We set these flags directly in below.
     // - "Set |fallbackMode| to "cors"."
     // - "Set |fallbackCredentials| to "omit"."

third_party/WebKit/Source/core/fileapi/PublicURLManager.cpp

@@ -155,6 +155,18 @@ void PublicURLManager::Revoke(const KURL& url) {
   url_to_registry_.erase(it);
 }
 
+void PublicURLManager::Resolve(
+    const KURL& url,
+    network::mojom::blink::URLLoaderFactoryRequest factory_request) {
+  DCHECK(RuntimeEnabledFeatures::MojoBlobURLsEnabled());
+  DCHECK(url.ProtocolIs("blob"));
+  if (!url_store_) {
+    BlobDataHandle::GetBlobRegistry()->URLStoreForOrigin(
+        GetExecutionContext()->GetSecurityOrigin(), MakeRequest(&url_store_));
+  }
+  url_store_->ResolveAsURLLoaderFactory(url, std::move(factory_request));
+}
+
 void PublicURLManager::ContextDestroyed(ExecutionContext*) {
   if (is_stopped_)
     return;

third_party/WebKit/Source/core/fileapi/PublicURLManager.h

@@ -32,6 +32,7 @@
 #include "platform/wtf/HashMap.h"
 #include "platform/wtf/HashSet.h"
 #include "platform/wtf/text/WTFString.h"
+#include "services/network/public/interfaces/url_loader_factory.mojom-blink.h"
 #include "third_party/WebKit/common/blob/blob_url_store.mojom-blink.h"
 
 namespace blink {
@@ -55,6 +56,9 @@ class CORE_EXPORT PublicURLManager final
   String RegisterURL(URLRegistrable*);
   // Revokes the given URL.
   void Revoke(const KURL&);
+  // When mojo Blob URLs are enabled this resolves the provided URL to a
+  // factory capable of creating loaders for the specific URL.
+  void Resolve(const KURL&, network::mojom::blink::URLLoaderFactoryRequest);
 
   // ContextLifecycleObserver interface.
   void ContextDestroyed(ExecutionContext*) override;

third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

@@ -35,6 +35,7 @@
 #include "bindings/core/v8/ScriptController.h"
 #include "bindings/core/v8/V8BindingForCore.h"
 #include "core/dom/Document.h"
+#include "core/fileapi/PublicURLManager.h"
 #include "core/frame/ContentSettingsClient.h"
 #include "core/frame/Deprecation.h"
 #include "core/frame/FrameConsole.h"
@@ -1213,9 +1214,32 @@ void FrameFetchContext::ParseAndPersistClientHints(
 
 std::unique_ptr<WebURLLoader> FrameFetchContext::CreateURLLoader(
     const ResourceRequest& request,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    const ResourceLoaderOptions& options) {
   DCHECK(!IsDetached());
   WrappedResourceRequest webreq(request);
+
+  network::mojom::blink::URLLoaderFactoryPtr url_loader_factory;
+  if (options.url_loader_factory) {
+    options.url_loader_factory->data->Clone(MakeRequest(&url_loader_factory));
+  }
+  // Resolve any blob: URLs that haven't been resolved yet. The XHR and fetch()
+  // API implementations resolve blob URLs earlier because there can be
+  // arbitrarily long delays between creating requests with those APIs and
+  // actually creating the URL loader here. Other subresource loading will
+  // immediately create the URL loader so resolving those blob URLs here is
+  // simplest.
+  if (document_ && request.Url().ProtocolIs("blob") &&
+      RuntimeEnabledFeatures::MojoBlobURLsEnabled() && !url_loader_factory) {
+    document_->GetPublicURLManager().Resolve(request.Url(),
+                                             MakeRequest(&url_loader_factory));
+  }
+  if (url_loader_factory) {
+    return Platform::Current()
+        ->WrapURLLoaderFactory(url_loader_factory.PassInterface().PassHandle())
+        ->CreateURLLoader(webreq, task_runner);
+  }
+
   if (MasterDocumentLoader()->GetServiceWorkerNetworkProvider()) {
     auto loader = MasterDocumentLoader()
                       ->GetServiceWorkerNetworkProvider()

third_party/WebKit/Source/core/loader/FrameFetchContext.h

@@ -153,7 +153,8 @@ class CORE_EXPORT FrameFetchContext final : public BaseFetchContext {
 
   std::unique_ptr<WebURLLoader> CreateURLLoader(
       const ResourceRequest&,
-      scoped_refptr<base::SingleThreadTaskRunner>) override;
+      scoped_refptr<base::SingleThreadTaskRunner>,
+      const ResourceLoaderOptions&) override;
 
   ResourceLoadScheduler::ThrottlingPolicy InitialLoadThrottlingPolicy()
       const override {

third_party/WebKit/Source/core/loader/ThreadableLoaderTest.cpp

@@ -240,6 +240,11 @@ class WebWorkerFetchContextForTest : public WebWorkerFetchContext {
     return std::make_unique<WebURLLoaderFactoryWithMock>(
         Platform::Current()->GetURLLoaderMockFactory());
   }
+  std::unique_ptr<WebURLLoaderFactory> WrapURLLoaderFactory(
+      mojo::ScopedMessagePipeHandle) override {
+    return std::make_unique<WebURLLoaderFactoryWithMock>(
+        Platform::Current()->GetURLLoaderMockFactory());
+  }
 
   void WillSendRequest(WebURLRequest&) override {}
   bool IsControlledByServiceWorker() const override { return false; }