Send blob request body when NetS13nServiceWorker w/o NetworkService

Request bodies may be a blob if NetworkService is disabled. We need to plumb it to a service worker world if we ship NetS13nSW without NetworkService. After this CL, there are three cases of how to send the request body to the service worker's fetch event. (A) NetworkService is on In this case, blobs should be passed as a data pipe. Data pipes will be included by a network::ResourceRequestBody. (B) NetworkService is off and ServiceWorkerServicification is on Blobs should be passed as a blink::mojom::BlobPtrs, but network::ResourceRequestBody cannot contain the blink::mojom::BlobPtrs. Instead, the BlobPtrs are passed as a parameter of DispatchFetchEventParams separately. (C) NetworkService and ServiceWorkerServicification are off All request bodies are passed as a blob. The information of the blob is passed as three parameters of DispatchFetchEventParams. Bug: 830292 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I36eb0e08e80ae9feb5af926af4b97757607c1950 Reviewed-on: https://chromium-review.googlesource.com/1046426 Commit-Queue: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Daniel Murphy <dmurph@chromium.org> Cr-Commit-Position: refs/heads/master@{#561373}
Denger-Network · May 24, 2018 · 8cc2e7e · 8cc2e7e
1 parent eb39631
commit 8cc2e7e
Show file tree

Hide file tree

Showing 12 changed files with 138 additions and 86 deletions.
diff --git a/content/browser/loader/upload_data_stream_builder.cc b/content/browser/loader/upload_data_stream_builder.cc
@@ -98,10 +98,11 @@ std::unique_ptr<net::UploadDataStream> UploadDataStreamBuilder::Build(
             body, file_task_runner, element));
         break;
       case network::DataElement::TYPE_BLOB: {
-        DCHECK_EQ(std::numeric_limits<uint64_t>::max(), element.length());
         DCHECK_EQ(0ul, element.offset());
         std::unique_ptr<storage::BlobDataHandle> handle =
             blob_context->GetBlobDataFromUUID(element.blob_uuid());
+        DCHECK(element.length() == std::numeric_limits<uint64_t>::max() ||
+               element.length() == handle->size());
         element_readers.push_back(
             std::make_unique<storage::UploadBlobElementReader>(
                 std::move(handle)));

diff --git a/content/common/service_worker/dispatch_fetch_event_params.mojom b/content/common/service_worker/dispatch_fetch_event_params.mojom
@@ -19,9 +19,23 @@ struct DispatchFetchEventParams {
   // FetchEvent#request.
   network.mojom.URLRequest request;
 
-  // For Non-S13nServiceWorker these |request_body_*| fields are used to create
-  // FetchEvent#request#body.  For S13nServiceWorker, the body is provided in
-  // |request.request_body|, and these fields are not used.
+  // The following fields are used to create FetchEvent#request#body, depending
+  // on whether S13nServiceWorker/NetworkService are enabled.
+
+  // (A) S13nServiceWorker with NetworkService on:
+  // All information about the request body is provided in
+  // |request.request_body|.
+
+  // (B) S13nServiceWorker with NetworkService off:
+  // All information about the request body except for BlobPtrs is provided in
+  // |request.request_body|. These BlobPtrs need to be passed separately.
+  // Once the NetworkService is enabled, this will be no longer used since all
+  // Blobs are passed as data pipes which can live in |request.request_body|.
+  array<blink.mojom.Blob> request_body_blob_ptrs;
+
+  // (C) non-S13nServiceWorker:
+  // All information to create the request body are packed into a blob. These
+  // params are for passing the blob.
   string request_body_blob_uuid;
   uint64 request_body_blob_size;
   blink.mojom.Blob? request_body_blob;

diff --git a/content/common/service_worker/service_worker_loader_helpers.cc b/content/common/service_worker/service_worker_loader_helpers.cc
@@ -156,47 +156,4 @@ int ServiceWorkerLoaderHelpers::ReadBlobResponseBody(
   return net::OK;
 }
 
-// static
-scoped_refptr<network::ResourceRequestBody>
-ServiceWorkerLoaderHelpers::CloneResourceRequestBody(
-    const network::ResourceRequestBody* body) {
-  auto clone = base::MakeRefCounted<network::ResourceRequestBody>();
-
-  clone->set_identifier(body->identifier());
-  clone->set_contains_sensitive_info(body->contains_sensitive_info());
-  for (const network::DataElement& element : *body->elements()) {
-    switch (element.type()) {
-      case network::DataElement::TYPE_UNKNOWN:
-        NOTREACHED();
-        break;
-      case network::DataElement::TYPE_DATA_PIPE: {
-        clone->AppendDataPipe(element.CloneDataPipeGetter());
-        break;
-      }
-      case network::DataElement::TYPE_RAW_FILE:
-        clone->AppendRawFileRange(element.file().Duplicate(), element.path(),
-                                  element.offset(), element.length(),
-                                  element.expected_modification_time());
-        break;
-      case network::DataElement::TYPE_CHUNKED_DATA_PIPE:
-        NOTREACHED() << "There should be no chunked data pipes going through "
-                        "ServiceWorker";
-        break;
-      case network::DataElement::TYPE_BLOB:
-        NOTREACHED() << "There should be no blob elements in NetworkService";
-        break;
-      case network::DataElement::TYPE_FILE:
-        clone->AppendFileRange(element.path(), element.offset(),
-                               element.length(),
-                               element.expected_modification_time());
-        break;
-      case network::DataElement::TYPE_BYTES:
-        clone->AppendBytes(element.bytes(), element.length());
-        break;
-    }
-  }
-
-  return clone;
-}
-
 }  // namespace content
diff --git a/content/common/service_worker/service_worker_loader_helpers.h b/content/common/service_worker/service_worker_loader_helpers.h
@@ -12,7 +12,6 @@
 #include "third_party/blink/public/mojom/blob/blob.mojom.h"
 
 namespace network {
-class ResourceRequestBody;
 struct ResourceRequest;
 struct ResourceResponseHead;
 }
@@ -49,15 +48,6 @@ class ServiceWorkerLoaderHelpers {
       const net::HttpRequestHeaders& headers,
       base::OnceCallback<void(int net_error)> on_blob_read_complete,
       mojo::ScopedDataPipeConsumerHandle* handle_out);
-
-  // Returns a new copy of the given body. This is useful for service worker
-  // with NetworkService because it sends the ResourceRequestBody over Mojo IPC,
-  // which moves out the DataPipeGetter elements in the Pickle code in
-  // resources_messages.cc. We can't change the Pickle code to call
-  // DataPipeGetter's Clone method because that code can run on different thread
-  // than the DataPipeGetter.
-  static scoped_refptr<network::ResourceRequestBody> CloneResourceRequestBody(
-      const network::ResourceRequestBody* body);
 };
 
 }  // namespace content

diff --git a/content/renderer/loader/web_url_request_util.cc b/content/renderer/loader/web_url_request_util.cc
@@ -24,8 +24,9 @@
 #include "services/network/public/mojom/request_context_frame_type.mojom.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
-#include "third_party/blink/public/mojom/blob/blob.mojom.h"
+#include "third_party/blink/public/mojom/blob/blob_registry.mojom.h"
 #include "third_party/blink/public/platform/file_path_conversion.h"
+#include "third_party/blink/public/platform/interface_provider.h"
 #include "third_party/blink/public/platform/modules/fetch/fetch_api_request.mojom-shared.h"
 #include "third_party/blink/public/platform/platform.h"
 #include "third_party/blink/public/platform/web_data.h"
@@ -296,10 +297,17 @@ int GetLoadFlagsForWebURLRequest(const WebURLRequest& request) {
 
 WebHTTPBody GetWebHTTPBodyForRequestBody(
     const network::ResourceRequestBody& input) {
+  return GetWebHTTPBodyForRequestBodyWithBlobPtrs(input, {});
+}
+
+WebHTTPBody GetWebHTTPBodyForRequestBodyWithBlobPtrs(
+    const network::ResourceRequestBody& input,
+    std::vector<blink::mojom::BlobPtrInfo> blob_ptrs) {
   WebHTTPBody http_body;
   http_body.Initialize();
   http_body.SetIdentifier(input.identifier());
   http_body.SetContainsPasswordData(input.contains_sensitive_info());
+  auto blob_ptr_iter = blob_ptrs.begin();
   for (auto& element : *input.elements()) {
     switch (element.type()) {
       case network::DataElement::TYPE_BYTES:
@@ -314,12 +322,16 @@ WebHTTPBody GetWebHTTPBodyForRequestBody(
             element.expected_modification_time().ToDoubleT());
         break;
       case network::DataElement::TYPE_BLOB:
-        http_body.AppendBlob(WebString::FromASCII(element.blob_uuid()));
+        if (blob_ptrs.empty()) {
+          http_body.AppendBlob(WebString::FromASCII(element.blob_uuid()));
+        } else {
+          DCHECK(blob_ptr_iter != blob_ptrs.end());
+          blink::mojom::BlobPtrInfo& blob = *blob_ptr_iter++;
+          http_body.AppendBlob(WebString::FromASCII(element.blob_uuid()),
+                               element.length(), blob.PassHandle());
+        }
         break;
       case network::DataElement::TYPE_DATA_PIPE: {
-        // Append the cloned data pipe to the |http_body|. This might not be
-        // needed for all callsites today but it respects the constness of
-        // |input|, as opposed to moving the data pipe out of |input|.
         http_body.AppendDataPipe(
             element.CloneDataPipeGetter().PassInterface().PassHandle());
         break;
@@ -334,6 +346,25 @@ WebHTTPBody GetWebHTTPBodyForRequestBody(
   return http_body;
 }
 
+std::vector<blink::mojom::BlobPtrInfo> GetBlobPtrsForRequestBody(
+    const network::ResourceRequestBody& input) {
+  std::vector<blink::mojom::BlobPtrInfo> blob_ptrs;
+  blink::mojom::BlobRegistryPtr blob_registry;
+  for (auto& element : *input.elements()) {
+    if (element.type() == network::DataElement::TYPE_BLOB) {
+      blink::mojom::BlobPtrInfo blob_ptr;
+      if (!blob_registry) {
+        blink::Platform::Current()->GetInterfaceProvider()->GetInterface(
+            mojo::MakeRequest(&blob_registry));
+      }
+      blob_registry->GetBlobFromUUID(mojo::MakeRequest(&blob_ptr),
+                                     element.blob_uuid());
+      blob_ptrs.push_back(std::move(blob_ptr));
+    }
+  }
+  return blob_ptrs;
+}
+
 scoped_refptr<network::ResourceRequestBody> GetRequestBodyForWebURLRequest(
     const WebURLRequest& request) {
   scoped_refptr<network::ResourceRequestBody> request_body;
@@ -390,7 +421,8 @@ scoped_refptr<network::ResourceRequestBody> GetRequestBodyForWebHTTPBody(
 
           request_body->AppendDataPipe(std::move(data_pipe_getter_ptr));
         } else {
-          request_body->AppendBlob(element.blob_uuid.Utf8());
+          request_body->AppendBlob(element.blob_uuid.Utf8(),
+                                   element.blob_length);
         }
         break;
       }

diff --git a/content/renderer/loader/web_url_request_util.h b/content/renderer/loader/web_url_request_util.h
@@ -13,6 +13,7 @@
 #include "net/http/http_request_headers.h"
 #include "services/network/public/cpp/resource_request_body.h"
 #include "services/network/public/mojom/request_context_frame_type.mojom.h"
+#include "third_party/blink/public/mojom/blob/blob_registry.mojom.h"
 #include "third_party/blink/public/platform/web_mixed_content_context_type.h"
 #include "third_party/blink/public/platform/web_url_request.h"
 
@@ -40,6 +41,19 @@ int GetLoadFlagsForWebURLRequest(const blink::WebURLRequest& request);
 blink::WebHTTPBody GetWebHTTPBodyForRequestBody(
     const network::ResourceRequestBody& input);
 
+// Takes a ResourceRequestBody with additional |blob_ptrs| which corresponds to
+// each Blob entries, and converts into WebHTTPBody.
+// TODO(kinuko): Remove this once Network Service is shipped.
+blink::WebHTTPBody GetWebHTTPBodyForRequestBodyWithBlobPtrs(
+    const network::ResourceRequestBody& input,
+    std::vector<blink::mojom::BlobPtrInfo> blob_ptrs);
+
+// Takes a ResourceRequestBody and gets blob pointers for Blob entries.
+// Used only in non-NetworkService cases but with S13nServiceWorker.
+// TODO(kinuko): Remove this once Network Service is shipped.
+std::vector<blink::mojom::BlobPtrInfo> GetBlobPtrsForRequestBody(
+    const network::ResourceRequestBody& input);
+
 // Takes a WebHTTPBody and converts into a ResourceRequestBody.
 scoped_refptr<network::ResourceRequestBody> GetRequestBodyForWebHTTPBody(
     const blink::WebHTTPBody& httpBody);

diff --git a/content/renderer/service_worker/service_worker_context_client.cc b/content/renderer/service_worker/service_worker_context_client.cc
@@ -8,6 +8,7 @@
 #include <memory>
 #include <utility>
 
+#include "base/feature_list.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ -51,6 +52,7 @@
 #include "ipc/ipc_message_macros.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_response_headers.h"
+#include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/public/mojom/request_context_frame_type.mojom.h"
 #include "storage/common/blob_storage/blob_handle.h"
@@ -185,6 +187,7 @@ void ToWebServiceWorkerRequest(const network::ResourceRequest& request,
                                uint64_t request_body_blob_size,
                                blink::mojom::BlobPtrInfo request_body_blob,
                                const std::string& client_id,
+                               std::vector<blink::mojom::BlobPtrInfo> blob_ptrs,
                                blink::WebServiceWorkerRequest* web_request) {
   DCHECK(web_request);
   web_request->SetURL(blink::WebURL(request.url));
@@ -198,16 +201,19 @@ void ToWebServiceWorkerRequest(const network::ResourceRequest& request,
 
   // Non-S13nServiceWorker: The body is provided as a blob.
   if (request_body_blob) {
-    DCHECK(!ServiceWorkerUtils::IsServicificationEnabled());
+    DCHECK(!base::FeatureList::IsEnabled(network::features::kNetworkService));
     mojo::ScopedMessagePipeHandle blob_pipe = request_body_blob.PassHandle();
     web_request->SetBlob(blink::WebString::FromASCII(request_body_blob_uuid),
                          request_body_blob_size, std::move(blob_pipe));
   }
   // S13nServiceWorker: The body is provided in |request|.
   else if (request.request_body) {
     DCHECK(ServiceWorkerUtils::IsServicificationEnabled());
-    blink::WebHTTPBody body =
-        GetWebHTTPBodyForRequestBody(*request.request_body);
+    // |blob_ptrs| should be empty when Network Service is enabled.
+    DCHECK(!base::FeatureList::IsEnabled(network::features::kNetworkService) ||
+           blob_ptrs.empty());
+    blink::WebHTTPBody body = GetWebHTTPBodyForRequestBodyWithBlobPtrs(
+        *request.request_body, std::move(blob_ptrs));
     body.SetUniqueBoundary();
     web_request->SetBody(body);
   }
@@ -1566,7 +1572,8 @@ void ServiceWorkerContextClient::DispatchFetchEvent(
   ToWebServiceWorkerRequest(
       std::move(params->request), params->request_body_blob_uuid,
       params->request_body_blob_size, std::move(params->request_body_blob),
-      params->client_id, &web_request);
+      params->client_id, std::move(params->request_body_blob_ptrs),
+      &web_request);
   proxy_->DispatchFetchEvent(event_id, web_request, navigation_preload_sent);
 }
 

diff --git a/content/renderer/service_worker/service_worker_subresource_loader.cc b/content/renderer/service_worker/service_worker_subresource_loader.cc
@@ -6,17 +6,27 @@
 
 #include "base/atomic_sequence_num.h"
 #include "base/callback.h"
+#include "base/feature_list.h"
 #include "base/optional.h"
 #include "content/common/service_worker/service_worker_loader_helpers.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/common/content_features.h"
+#include "content/renderer/loader/web_url_request_util.h"
+#include "content/renderer/render_thread_impl.h"
+#include "content/renderer/renderer_blink_platform_impl.h"
 #include "content/renderer/service_worker/controller_service_worker_connector.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "net/url_request/redirect_util.h"
 #include "net/url_request/url_request.h"
+#include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "third_party/blink/public/mojom/blob/blob.mojom.h"
+#include "third_party/blink/public/platform/interface_provider.h"
+#include "third_party/blink/public/platform/modules/serviceworker/web_service_worker_request.h"
+#include "third_party/blink/public/platform/web_http_body.h"
+#include "third_party/blink/public/platform/web_string.h"
 #include "ui/base/page_transition_types.h"
 
 namespace content {
@@ -227,31 +237,23 @@ void ServiceWorkerSubresourceLoader::DispatchFetchEvent() {
     return;
   }
 
-  // TODO(kinuko): Implement request timeout and ask the browser to kill
-  // the controller if it takes too long. (crbug.com/774374)
-
-  // Passing the request body over Mojo moves it out. But the request body
-  // may be needed later, in the case where the service worker doesn't provide a
-  // response in the fetch event. So instead send a cloned body. (Note that
-  // we can't do the reverse, i.e., send the original and restore the clone
-  // later.  By always sending the clone, we ensure the original ResourceRequest
-  // passed into the constructor always points to a valid ResourceRequestBody,
-  // even if this loader gets destructed.)
-  if (resource_request_.request_body) {
-    inflight_fetch_request_->request_body =
-        ServiceWorkerLoaderHelpers::CloneResourceRequestBody(
-            resource_request_.request_body.get());
-  }
-
   auto params = mojom::DispatchFetchEventParams::New();
   params->request = *inflight_fetch_request_;
   params->client_id = controller_connector_->client_id();
+
+  // S13nServiceWorker without NetworkService:
+  // BlobPtr for each blob data element in the request body needs to be created
+  // before dispatching the fetch event for keeping the blob alive.
+  if (resource_request_.request_body &&
+      !base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    params->request_body_blob_ptrs =
+        GetBlobPtrsForRequestBody(*resource_request_.request_body);
+  }
+
   controller->DispatchFetchEvent(
       std::move(params), std::move(response_callback_ptr),
       base::BindOnce(&ServiceWorkerSubresourceLoader::OnFetchEventFinished,
                      weak_factory_.GetWeakPtr()));
-  // |inflight_fetch_request_->request_body| should not be used after this
-  // point.
 }
 
 void ServiceWorkerSubresourceLoader::OnFetchEventFinished(

diff --git a/services/network/public/cpp/resource_request_body.cc b/services/network/public/cpp/resource_request_body.cc
@@ -56,11 +56,15 @@ void ResourceRequestBody::AppendRawFileRange(
 }
 
 void ResourceRequestBody::AppendBlob(const std::string& uuid) {
+  AppendBlob(uuid, std::numeric_limits<uint64_t>::max());
+}
+
+void ResourceRequestBody::AppendBlob(const std::string& uuid, uint64_t length) {
   DCHECK(elements_.empty() ||
          elements_.front().type() != DataElement::TYPE_CHUNKED_DATA_PIPE);
 
   elements_.push_back(DataElement());
-  elements_.back().SetToBlob(uuid);
+  elements_.back().SetToBlobRange(uuid, 0 /* offset */, length);
 }
 
 void ResourceRequestBody::AppendDataPipe(

diff --git a/services/network/public/cpp/resource_request_body.h b/services/network/public/cpp/resource_request_body.h
@@ -43,7 +43,19 @@ class COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequestBody
                           uint64_t length,
                           const base::Time& expected_modification_time);
 
+  // Appends a blob. If the 2-parameter version is used, the resulting body can
+  // be read by Blink, which is needed when the body is sent to Blink, e.g., for
+  // service worker interception. The length must be size of the entire blob,
+  // not a subrange of it. If the length is unknown, use the 1-parameter
+  // version, but this means the body/blob won't be readable by Blink (that's OK
+  // if this ResourceRequestBody will only be sent to the browser process and
+  // won't be sent to Blink).
+  //
+  // TODO(crbug.com/846167): Remove these functions when NetworkService is
+  // enabled, as blobs are passed via AppendDataPipe in that case.
   void AppendBlob(const std::string& uuid);
+  void AppendBlob(const std::string& uuid, uint64_t length);
+
   void AppendDataPipe(mojom::DataPipeGetterPtr data_pipe_getter);
 
   // |chunked_data_pipe_getter| will provide the upload body for a chunked