Use blink::mojom::FetchAPIRequest for service workers |
|
Issue descriptionWe have two types of "request" objects in mojom API surface: network.mojom.URLRequest and blink.mojom.FetchAPIRequest. Currently, DispatchFetchEventParams uses network.mojom.URLRequest and other APIs using request objects like BackgroundFetch and CacheStorages are using blink.mojom.FetchAPIRequest. We've had discussion on when to use network.mojom.URLRequest and blink.mojom.FetchAPIRequest at a doc [1], and now we concluded that blink.mojom.FetchAPIRequest would stand for blink::Request (which is web-exposed struct) and network.mojom.URLRequest would stand for blink::ResourceRequest (which is a internal representation of request used in fetch() implementation). This is a prerequisite step to move content.mojom.ServiceWorker interface to blink, since we need to remove a dependency on network.mojom.URLRequest, which is not having a blink-side representation. [1] https://docs.google.com/document/d/1b9KS0DXFnc6jifpHcUhX04XZcJEAkCC4HCbDwDgeM50/edit#
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a115b526e20b66a78426ab092bc43600f9a27a4 commit 9a115b526e20b66a78426ab092bc43600f9a27a4 Author: Leon Han <leon.han@intel.com> Date: Fri Dec 07 04:38:51 2018 [ServiceWorker] A cleanup in service_worker_context_client.cc. The ToWebServiceWorkerRequest() is used only for dispatching fetch event to service workers, and is not supposed to be used for other purposes even in the future. BUG=911930 Change-Id: Ie57f1b65c927ad3aed675b744135ed48f37df9b8 Reviewed-on: https://chromium-review.googlesource.com/c/1365074 Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Commit-Queue: Leon Han <leon.han@intel.com> Cr-Commit-Position: refs/heads/master@{#614595} [modify] https://crrev.com/9a115b526e20b66a78426ab092bc43600f9a27a4/content/renderer/service_worker/service_worker_context_client.cc
,
Dec 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55825ccfad75a22821091ee15977e69d3288f1cf commit 55825ccfad75a22821091ee15977e69d3288f1cf Author: Leon Han <leon.han@intel.com> Date: Mon Dec 17 12:16:20 2018 [OnionSoup] Use FetchAPIRequest for FetchEvent#request dispatched to SW Previously we were using network.mojom.URLRequest (a [Native] mojom struct typemapping with network::ResourceRequest) for FetchEvent#request dispatched to SW, The reason not to use FetchAPIRequest: (see http://crrev.com/c/742866) - SW interception code (for loading both main resources and subresources) gets a network::ResourceRequest from Mojo Loading. - the ResourceRequest has a ResourceRequestBody. - in the subresource case (in the renderer process) we can not turn a ResourceRequestBody into a Blob to be carried as FetchAPIRequest.blob. This CL adds a new member FetchAPIRequest.body to solve the above problem. Now SW interception code can convert a network::ResourceRequest (with body) to a FetchAPIRequest without data loss, then dispatch the FetchAPIRequest as FetchEvent#request to SW. Background: We had a discussion about network.mojom.URLRequest and FetchAPIRequest at https://docs.google.com/document/d/1b9KS0DXFnc6jifpHcUhX04XZcJEAkCC4HCbDwDgeM50/edit# with the concensus: - network.mojom.URLRequest is kind of internal data in the loading stack, while FetchAPIRequest should be a direct representation of a JS Request object for all API implementations like Background Fetch, Cache Storage and Service Worker, with no need to care how the loading logic happens. - Ideally, URLRequest will be typemapping with network::ResourceRequest for Chromium variant and blink::ResourceRequest for Blink variant in the future. - FetchAPIRequest aims to fully support blink::Request. The request body flow is unchanged: (Page/Client) blink::EncodedFormData -> blink::WebHTTPBody -> network::ResourceRequestBody ResourceRequestBody -> blink::WebHTTPBody -> blink::EncodedFormData -> FetchRequestData/FormDataBytesConsumer But in the future with more Onion Soup effort it would be like: (Page/Client) blink::EncodedFormData --typemapping-> network.mojom.ResourceRequestBody --typemapping-> network::ResourceRequestBody network::ResourceRequestBody --typemapping-> network.mojom.ResourceRequestBody --typemapping-> blink::EncodedFormData -> FetchRequestData/FormDataBytesConsumer BUG=911930 Change-Id: Ia96cace7ff3bf7a3d2e66056c606cfb4a02fbb0b Reviewed-on: https://chromium-review.googlesource.com/c/1367365 Commit-Queue: Leon Han <leon.han@intel.com> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#617092} [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/embedded_worker_test_helper.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/embedded_worker_test_helper.h [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/service_worker_browsertest.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/service_worker_fetch_dispatcher.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/service_worker_fetch_dispatcher.h [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/service_worker_navigation_loader.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/service_worker_navigation_loader_unittest.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/service_worker_url_request_job.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/browser/service_worker/service_worker_url_request_job_unittest.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/common/BUILD.gn [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/common/background_fetch/background_fetch_types.cc [add] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/common/fetch/OWNERS [add] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/common/fetch/fetch_request_type_converters.cc [add] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/common/fetch/fetch_request_type_converters.h [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/renderer/service_worker/service_worker_context_client.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/renderer/service_worker/service_worker_context_client_unittest.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/renderer/service_worker/service_worker_provider_context_unittest.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/renderer/service_worker/service_worker_subresource_loader.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/services/network/public/cpp/url_request.typemap [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/services/network/public/mojom/url_loader.mojom [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/third_party/blink/public/mojom/fetch/fetch_api_request.mojom [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/third_party/blink/public/mojom/service_worker/dispatch_fetch_event_params.mojom [modify] https://crrev.com/55825ccfad75a22821091ee15977e69d3288f1cf/third_party/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/793c3c5282125961db5401a2b9485a66798adcd8 commit 793c3c5282125961db5401a2b9485a66798adcd8 Author: Leon Han <leon.han@intel.com> Date: Thu Dec 20 07:49:00 2018 [ServiceWorker] Remove blink.mojom.FetchAPIRequest.client_id blink.mojom.FetchAPIRequest.client_id is not used anywhere so we'd just remove it now. BUG=911930 Change-Id: I0f64df6f91daa7d6bd52f2a3815b2f070e0a5291 Reviewed-on: https://chromium-review.googlesource.com/c/1377603 Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Commit-Queue: Leon Han <leon.han@intel.com> Cr-Commit-Position: refs/heads/master@{#618134} [modify] https://crrev.com/793c3c5282125961db5401a2b9485a66798adcd8/content/browser/cache_storage/cache_storage_cache.cc [modify] https://crrev.com/793c3c5282125961db5401a2b9485a66798adcd8/content/common/background_fetch/background_fetch_types.cc [modify] https://crrev.com/793c3c5282125961db5401a2b9485a66798adcd8/content/common/service_worker/service_worker_types.proto [modify] https://crrev.com/793c3c5282125961db5401a2b9485a66798adcd8/content/common/service_worker/service_worker_types_unittest.cc [modify] https://crrev.com/793c3c5282125961db5401a2b9485a66798adcd8/content/common/service_worker/service_worker_utils.cc [modify] https://crrev.com/793c3c5282125961db5401a2b9485a66798adcd8/third_party/blink/public/mojom/fetch/fetch_api_request.mojom [modify] https://crrev.com/793c3c5282125961db5401a2b9485a66798adcd8/third_party/blink/renderer/modules/cache_storage/inspector_cache_storage_agent.cc
,
Dec 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1ab13f85efc9219f9b32d73f3c57f17af426c747 commit 1ab13f85efc9219f9b32d73f3c57f17af426c747 Author: Leon Han <leon.han@intel.com> Date: Wed Dec 26 12:48:37 2018 [OnionSoup] Remove DispatchFetchEventParams.request_body_blob_ptrs Previously, in case of S13nServiceWorker without NetworkService, to transfer those Blob elements of network::ResourceRequestBody when calling blink.mojom.ServiceWorker.DispatchFetchEvent(), additionally ServiceWorkerSubresourceLoader prepares an array of BlobPtrInfo for those Blob data and transfers it as DispatchFetchEventParams.request_body_blob_ptrs. Then ServiceWorkerContextClient gets it and puts it into a blink::WebHTTPBody as part of WebServiceWorkerRequest to enter Blink. However, we can see that DispatchFetchEventParams.request_body_blob_ptrs is unnecessary to be passed through the Mojo call, ServiceWorkerContextClient itself should be able to get these information from the received network::ResourceRequestBody. This CL lets ServiceWorkerContextClient do that and then removes DispatchFetchEventParams.request_body_blob_ptrs. crrev.com/c/1390323 relies on this CL to have ServiceWorkerContextClient get those BlobPtrInfos from a network::ResourceRequestBody sent by another fetch event dispatcher ServiceWorkerURLRequestJob. BUG=911930 TBR=thestig@chromium.org for trivial changes in base/threading/thread_restrictions.h Change-Id: I383732b60329a9cd7c4204c7fbeedff10eee1990 Reviewed-on: https://chromium-review.googlesource.com/c/1390263 Reviewed-by: Leon Han <leon.han@intel.com> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Leon Han <leon.han@intel.com> Cr-Commit-Position: refs/heads/master@{#618924} [modify] https://crrev.com/1ab13f85efc9219f9b32d73f3c57f17af426c747/base/threading/thread_restrictions.h [modify] https://crrev.com/1ab13f85efc9219f9b32d73f3c57f17af426c747/content/renderer/service_worker/service_worker_context_client.cc [modify] https://crrev.com/1ab13f85efc9219f9b32d73f3c57f17af426c747/content/renderer/service_worker/service_worker_context_client.h [modify] https://crrev.com/1ab13f85efc9219f9b32d73f3c57f17af426c747/content/renderer/service_worker/service_worker_subresource_loader.cc [modify] https://crrev.com/1ab13f85efc9219f9b32d73f3c57f17af426c747/third_party/blink/public/mojom/service_worker/dispatch_fetch_event_params.mojom
,
Dec 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28e779303ba5c598a75dc00352b26d1b82056d14 commit 28e779303ba5c598a75dc00352b26d1b82056d14 Author: Leon Han <leon.han@intel.com> Date: Thu Dec 27 06:57:24 2018 [OnionSoup] Use FetchAPIRequest.body instead of FetchAPIRequest.blob in SW code Previously, in non-S13nServiceWorker case, a network::ResourceRequestBody was being converted to a blob as FetchAPIRequest.blob to transfer data for FetchEvent#request#body dispatched to service workers. With crrev.com/c/1367365 introducing a new field FetchAPIRequest.body of type network::mojom::URLRequestBody which has a typemap to network::ResourceRequestBody, now we can use it instead to avoid the data conversion above. After this CL, FetchAPIRequest.blob has only one user left: Background Fetch API implementation. BUG=911930 Change-Id: I034e3cc539e19f5cf7b9d35809f12a54a110ac8f Reviewed-on: https://chromium-review.googlesource.com/c/1390323 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Commit-Queue: Leon Han <leon.han@intel.com> Cr-Commit-Position: refs/heads/master@{#619010} [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/content/browser/service_worker/service_worker_blob_reader.h [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/content/browser/service_worker/service_worker_url_request_job.cc [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/content/browser/service_worker/service_worker_url_request_job.h [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/content/renderer/service_worker/service_worker_context_client.cc [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/third_party/blink/public/mojom/fetch/fetch_api_request.mojom [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/third_party/blink/public/platform/modules/service_worker/web_service_worker_request.h [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/third_party/blink/renderer/core/fetch/fetch_request_data.cc [modify] https://crrev.com/28e779303ba5c598a75dc00352b26d1b82056d14/third_party/blink/renderer/platform/exported/web_service_worker_request.cc
,
Jan 17
(5 days ago)
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54690cb3a00c16ee7299cac1d8ed96b85d470d3a commit 54690cb3a00c16ee7299cac1d8ed96b85d470d3a Author: Makoto Shimazu <shimazu@chromium.org> Date: Thu Jan 17 10:06:45 2019 Set same type nullable to typemap of scoped_refptr<ResourceRequestBody> This is a cleanup of https://crrev.com/c/1367365. nullable_is_same_type is well suited for typemapping to scoped_refptr<>. Bug: 911930 Change-Id: Iff6800eb000ebaf84b109e5989f4c2dd192f1503 Reviewed-on: https://chromium-review.googlesource.com/c/1415236 Commit-Queue: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#623626} [modify] https://crrev.com/54690cb3a00c16ee7299cac1d8ed96b85d470d3a/content/browser/service_worker/service_worker_fetch_dispatcher.cc [modify] https://crrev.com/54690cb3a00c16ee7299cac1d8ed96b85d470d3a/content/browser/service_worker/service_worker_navigation_loader_unittest.cc [modify] https://crrev.com/54690cb3a00c16ee7299cac1d8ed96b85d470d3a/content/renderer/service_worker/service_worker_context_client.cc [modify] https://crrev.com/54690cb3a00c16ee7299cac1d8ed96b85d470d3a/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc [modify] https://crrev.com/54690cb3a00c16ee7299cac1d8ed96b85d470d3a/services/network/public/cpp/url_request.typemap
,
Yesterday
(28 hours ago)
The only work left here is to use FetchAPIRequest.body instead of FetchAPIRequest.blob throughout Background Fetch API implementations, which would happen after we got the typemap (network.mojom.ResourceRequestBody, blink::EncodedFormData) ready. |
|
►
Sign in to add a comment |
|
Comment 1 by leon....@intel.com
, Dec 6