New issue
Advanced search Search tips

Issue 890748 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

SignedExchange prefetch requests from Service Worker controlled pages aren't handled by SignedExchangePrefetchHandler when NetworkService or ServiceWorkerServicification is enabled

Project Member Reported by horo@chromium.org, Oct 1

Issue description

When 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.
 
Maybe this was regressed by this change?
https://chromium-review.googlesource.com/c/chromium/src/+/1116411
Labels: -Pri-2 Pri-1
Summary: SignedExchange prefetch requests from Service Worker controlled pages aren't handled by SignedExchangePrefetchHandler when NetworkService or ServiceWorkerServicification is enabled (was: SignedExchange prefetch requests from Service Worker controlled pages aren't handled by SignedExchangePrefetchHandler when NetworkService is enabled)
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.
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.
Owner: horo@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
Status: Started (was: Verified)
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.
Project Member

Comment 9 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)

Sign in to add a comment