SignedExchange prefetch requests from Service Worker controlled pages aren't handled by SignedExchangePrefetchHandler when NetworkService or ServiceWorkerServicification is enabled |
|||||||
Issue descriptionWhen NetworkService is not enabled, or the page is not controlled by a service worker, the prefetch request of <link rel="prefetch"> is handled by PrefetchURLLoader. And PrefetchURLLoader::OnReceiveResponse() passes the response to a SignedExchangePrefetchHandler, and the certificate fetching and sub resource preloading will be triggered. But when NetworkService is enabled, and the page is controlled by a service worker, the prefetch request is not handled by PrefetchURLLoader correctly. So, the certificate fetching and sub resource preloading doesn't work.
,
Oct 2
When ServiceWorkerServicification is enabled, the SignedExchange prefetch from SW controlled page doesn't work even if NetworkService is disabled. ServiceWorkerServicification is almost ready to ship. So changing the priority to 1.
,
Oct 2
Thanks for catching this! Letting prefetches skip SW feels more legitimate longer term but this needs spec changes, we should just plumb the factory to SW context for now.
,
Oct 2
,
Oct 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2cf21db73cae3e714f3c553652efc585cdea67a4 commit 2cf21db73cae3e714f3c553652efc585cdea67a4 Author: Tsuyoshi Horo <horo@chromium.org> Date: Thu Oct 04 08:01:11 2018 Hold prefetch_loader_factory in ChildURLLoaderFactoryBundle Currently PrefetchURLLoader in the browser process handles prefetch requests to reduce the memory consumption by skipping sending back the body to the renderer process, and to handle signed exchanges. But when NetworkService or ServiceWorkerServicification feature is enabled, prefetch requests from Service Worker controlled pages don't go to the PrefetchURLLoader. So SignedExchange's prefetching processes, such as certificate fetching and sub resource preloading will not be triggered. To fix this problem, this CL moves the prefetch_loader_factory from RenderFrameImpl to ChildURLLoaderFactoryBundle. BuildServiceWorkerNetworkProviderForNavigation() clones the ChildURLLoaderFactoryBundle with the prefetch_loader_factory, and passes it to the ServiceWorkerNetworkProvider as a fallback_loader_factory which will be used when respondWith() of the fetch event is not called in the service worker. Note: This CL moves the routing logic of prefetch request from RenderFrameImpl::FrameURLLoaderFactory to ChildURLLoaderFactoryBundle. Calling fetch(event.request) in service workers for prefetch requests doesn't go to the PrefetchURLLoader. So the signed exchange handling is not triggerd. This change is supposed to be tentative and should go away, as we're likely moving the special prefetch handling code into the network service. So we can remove this change when that happens. Bug: 890748 Change-Id: I6b5362b180a133d9274a20045ef82eaad9034ee4 Reviewed-on: https://chromium-review.googlesource.com/c/1256468 Commit-Queue: Tsuyoshi Horo <horo@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#596540} [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/browser/web_package/signed_exchange_request_handler_browsertest.cc [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/loader/child_url_loader_factory_bundle.cc [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/loader/child_url_loader_factory_bundle.h [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/loader/tracked_child_url_loader_factory_bundle.cc [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/loader/tracked_child_url_loader_factory_bundle.h [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/render_frame_impl.cc [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/render_frame_impl.h [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/service_worker/service_worker_context_client.cc [modify] https://crrev.com/2cf21db73cae3e714f3c553652efc585cdea67a4/content/renderer/shared_worker/embedded_shared_worker_stub.cc
,
Oct 9
,
Oct 9
,
Oct 10
Sorry, it is not fixed. The cl https://chromium-review.googlesource.com/c/chromium/src/+/1256468 fixed only for NetworkService flag. When ServiceWorkerServicification is enabled without SignedExchangePrefetchHandler, SignedExchange prefetch requests from Service Worker controlled pages aren't handled by SignedExchangePrefetchHandler yet.
,
Oct 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7f452e5a1642ee650fcf70be15b5b9616d3719d6 commit 7f452e5a1642ee650fcf70be15b5b9616d3719d6 Author: Tsuyoshi Horo <horo@chromium.org> Date: Thu Oct 11 02:24:18 2018 Create subresource_loader_factories for prefetching when ServiceWorkerServicification is enabled http://crrev.com/c/1256468 was intended to fix the issue that the prefetching logic of signed exchange doesn't work in service worker controlled pages when NetworkService is enabled. But the CL doesn't fix the issue with ServiceWorkerServicification. This is because |prefetch_loader_factory| is not correctly passed to the renderer when ServiceWorkerServicification is enbaled. This CL fix this problem by: - Change RenderFrameHostImpl::CommitNavigation() to create prefetch_loader_factory using the default factory and pass it to the renderer process when ServiceWorkerServicification is enabled. - Add ChildURLLoaderFactoryBundle::SetPrefetchLoaderFactory() to set the prefetch_loader_factory from RenderFrameImpl::SetupLoaderFactoryBundle(). Bug: 890748 , 892000 Change-Id: I056f4a550ace20e271f66463350337c961c0eae0 Reviewed-on: https://chromium-review.googlesource.com/c/1272419 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Commit-Queue: Tsuyoshi Horo <horo@chromium.org> Cr-Commit-Position: refs/heads/master@{#598636} [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/browser/frame_host/render_frame_host_impl.cc [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/browser/loader/prefetch_url_loader_service.cc [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/browser/loader/prefetch_url_loader_service.h [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/browser/web_package/signed_exchange_request_handler_browsertest.cc [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/renderer/loader/child_url_loader_factory_bundle.cc [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/renderer/loader/child_url_loader_factory_bundle.h [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/renderer/render_frame_impl.cc [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/renderer/service_worker/service_worker_context_client.cc [modify] https://crrev.com/7f452e5a1642ee650fcf70be15b5b9616d3719d6/content/renderer/shared_worker/embedded_shared_worker_stub.cc
,
Oct 11
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by horo@chromium.org
, Oct 1