New issue
Advanced search Search tips

Issue 873575 link

Starred by 2 users
Status: Fixed
Owner:
falken@chromium.org
Closed: Aug 28
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 873061



Sign in to add a comment

Decide how throttles, esp GoogleUrlLoaderThrottle, should work with service worker.

Project Member Reported by falken@chromium.org, Aug 13 
Chrome adds throttles to outgoing navigation requests. One of the throttles is GoogleUrlThrottle.

There are three unknowns:
1. How do these interact with service worker in the legacy path (non-S13nSW, non-NetworkService).
2. How do these interact with service worker in the new path (S13nSW, NetworkService).
3. What the desired behavior actually is.

At least, the new path behavior looks weird. We first dispatch the fetch event, get a response from the SW, and then make the throttles and ThrottlingURLLoader. That would mean the throttles only get applied to the response. But throttles also want to apply to the request, especially GoogleUrlLoaderThrottle.

It's possible the legacy path is doing the same thing.

GoogleUrlLoaderThrottle does at least three things:
- Adds variation ids to the header.
- Adjusts requests for YouTube restrictions.
- Adjusts requests for Google Search safe mode.

Thus it would seem at least GoogleUrlLoaderThrottle should apply before the request goes to the service worker. Otherwise a case like this would bypass the throttle:
  e.respondWith(fetch(e.request));

OTOH in issues like issue 595993 and issue 613483, we were leaning toward header injection etc to happen after SW interception. This is because they had surprising behavior when mixed with CORS that servers weren't expecting.

Another possibility is to add the throttles to the fetch() request, perhaps by checking if e.request.mode is 'navigation'. (Of course the SW can mint a new Request though.)

Also, it looks like some throttles get applied to subresource requests too. E.g., chrome/renderer/url_loader_throttle_provider_impl.cc. It's another question whether the throttles should be applied before or after sw.

Comment 1 by kinuko@chromium.org, Aug 13 

In the new path the current expected behavior (not necessarily same as spec'ed behavior) is something like:

(1) For navigation requests:
browser -> throttles -> SW

(2) For subresource requests:
page -> throttles -> SW

(3) For requests from SWs:
SW -> throttles -> network

When do you see 'throttles only get applied to the response' is happening?

Comment 2 by falken@chromium.org, Aug 13 

I was just doing code reading so haven't confirmed but this looks like the throttle happens after the response for navigation requests to SW:

MaybeStartLoader tries an interceptor:
      next_interceptor->MaybeCreateLoader(
          *resource_request_, resource_context_,
          base::BindOnce(&URLLoaderRequestController::MaybeStartLoader,
                         base::Unretained(this), next_interceptor));

ServiceWorkerNavigationLoader dispatches the fetch event and gets a response, then calls the callback MaybeStartLoader with StartResponse:
  std::move(loader_callback_)
      .Run(base::BindOnce(&ServiceWorkerNavigationLoader::StartResponse,
                          weak_factory_.GetWeakPtr(), std::move(response),
                          version, std::move(body_as_stream)));

MaybeStartLoader applies the throttles:
    if (single_request_handler) {
      // |interceptor| wants to handle the request with
      // |single_request_handler|.
      DCHECK(interceptor);
      std::vector<std::unique_ptr<content::URLLoaderThrottle>> throttles =
          CreateURLLoaderThrottles();
      throttles.push_back(std::make_unique<MimeSniffingThrottle>());
      default_loader_used_ = false;
      url_loader_ = ThrottlingURLLoader::CreateLoaderAndStart(

Comment 3 by kinuko@chromium.org, Aug 13 

Ah... I see so that part's a bit complicated with SW or AppCache.

For the particular case with e.respondWith(fetch(e.request)) a request that's (re-)issued by a SW should get its throttle, so that's good.

When throttles change the navigation request URL the throttle internally makes a redirect, so it might actually go to the Service Worker twice...

Comment 4 by kinuko@chromium.org, Aug 13 

Hm I guess this concern might be real (i.e. not about not a request going through the throttles, but about SW might be kicked twice). AppCache is probably fine, but for SW (where perf matters a lot) we might need not to start a SW until the given loader callback is called, ... or might need to change how throttles kick in in NavigationURLLoaderImpl.

For the former: we may want to make SWNavigationLoader::StartRequest a loader callback (rather than StartResponse), and let the loader somehow make a special redirect to the network loader (without going through other interceptors) on fallback cases. Not sure how uglier it'd look.

It's not immediately clear how the latter can be done.

Comment 5 by falken@chromium.org, Aug 13 

Thanks I see... it's good to know fetch() is getting throttled for nav requests already.

I didn't realize the throttle could make SW get invoked twice.

Regarding the request going through the throttles before SW, I'd imagine that someday if Google and YouTube (or other sites) start moving code from server to service worker, they'd expect the SW to see the headers the server would see. But I guess that hasn't been an issue yet, and so maybe avoiding kicking SW twice is indeed the real concern here.

Comment 6 by falken@chromium.org, Aug 14 

My impression of the desired possible paths:

(1) SW elects to intercept -> throttle -> SW intercepts -> SW responds
(2) SW elects to intercept -> throttle -> SW intercepts -> SW falls back to network -> request goes to to network without re-throttling (maybe by returning to NavigationURLLoaderImpl, or SW loader doing the network request)
(3) SW elects to intercept -> throttle changes request URL -> request restarts in NavigationURLLoaderImpl (possibly resulting in going to the original SW or not, depending on if the new request URL is in-scope).

Comment 7 by falken@chromium.org, Aug 14 

Some findings that I don't quite understand yet. I added a dummy header in GoogleURLLoaderThrottle::WillStartRequest and in AppendVariationHeaders in variations_http_headers.cc.

- default/legacy: GoogleUrlLoaderThrottle is applied to all requests and has an effect on subresource requests, but doesn't actually have an effect for main resource requests. The variations header is added by AppendVariationHeaders.
- ServicifiedServiceWorker: same as default/legacy
- NetworkService: GoogleUrlLoaderThrottle is applied and has an effect on all requests.

Comment 8 by falken@chromium.org, Aug 14 

"The variations header is added by AppendVariationHeaders" -> meant in the case of main resource requests. For subresource requests it's added by GoogleUrlLoaderThrottle.

Comment 9 by falken@chromium.org, Aug 14 

There may be a problem that the service worker interceptor is created with MaybeCreateLoader(const ResourceRequest&) before the throttling.

So even if SWNavigationLoader::StartRequest is used as the loader callback, SWNavigationLoader is using a copy of the resource request which won't have the same headers as the post-throttled request.

If a throttle changes the URL and not just the headers, it probably ends up OK because that simulates a redirect in NavigationURLLoaderImpl.

I'm wondering if we should just be throttling once before making and consulting the interceptors.

Comment 10 by falken@chromium.org, Aug 14 

Ah, or SingleRequestURLLoaderFactory should be passing resource_request to the RequestHandler.

Comment 11 by kinuko@chromium.org, Aug 14 

Yes, I agree that #10 could be the possibility, and that may be less confusing (but in anyways we need to change when to start sending requests vs when to apply throttling).

And you're right that it could somehow be okay in terms of correctness if the throttler also changes the URL as it makes a redirect, but if it only changes headers currently that throttling is not applied to the request that goes to SW.

Comment 12 by kinuko@chromium.org, Aug 14 

For double-throttling for the requests that go through SW (while the desired behavior is (2) in #6): I think it should probably be addressed separately, as some throttlers still need to be applied to both cases.

Comment 13 by falken@chromium.org, Aug 14 

My WIP at https://chromium-review.googlesource.com/c/chromium/src/+/1174204 intends to apply throttles before SW interception, and if fallback occurs, doesn't apply them again.

That would seem to both let the SW see the post-throttled request and eliminate double-throttling.

But I'm confused by c#12. Are there cases where double-throttling is needed?

Comment 14 by falken@chromium.org, Aug 15

Owner: falken@chromium.org
Status: Started (was: Available)

Comment 15 by falken@chromium.org, Aug 16 

After being super confused about how the default/legacy path doesn't expose x-client-data-header to SWs (given that it's injected by ResourceDispatcherHostImpl),  I discovered it's excluded manually:
https://cs.chromium.org/chromium/src/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc?l=342&rcl=c840686b6474a55b7dd98f01d115c27da867c203 from https://codereview.chromium.org/666973003.

As far as I can tell, X-Client-Data is the only header on this list. This is quite a mess given issue 595993 and similar problems. There are some headers that need to be added before SW and some headers that need to be added after and there's not a consistent mechanism to accomplish that.
Project Member

Comment 16 by bugdroid1@chromium.org, Aug 17 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7da35746e044414f0ecda8fad9b92c88bd26f21

commit b7da35746e044414f0ecda8fad9b92c88bd26f21
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Aug 17 01:14:19 2018

service worker: Filter embedder-injected headers at the renderer instead of browser.

The legacy/default path was filtering X-Client-Data at
ServiceWorkerURLRequestJob.

The S13nSW/NetworkService path did not need to filter this yet, because
it gets injected by GoogleURLLoaderThrottle in that path, and throttling
happens after SW interception. However the plan is to move throttling in
front of SW interception, so the SW would start seeing the headers. This
is needed so the navigation preload request can have the header too, and
also looks right for correctness as some throttles also mutate the
request in ways that the service worker should observe; and also would
prevent wasteful double-interception when the request URL changes.

Since in S13nSW/NetworkService the header can also be injected by the
renderer and sent directly to the service worker, filter the headers
just before the event is dispatched.

Bug:  873061 ,  873575 
Change-Id: I6a2011a2d1e6d1d6c08b1fc254b2cd7ae013d9f3
Reviewed-on: https://chromium-review.googlesource.com/1177418
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583916}
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/chrome/renderer/chrome_content_renderer_client.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/chrome/renderer/chrome_content_renderer_client.h
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/components/variations/net/variations_http_headers.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/components/variations/net/variations_http_headers.h
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/browser/service_worker/service_worker_context_core.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/browser/service_worker/service_worker_url_request_job.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/public/browser/service_worker_context.h
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/public/renderer/content_renderer_client.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/public/renderer/content_renderer_client.h
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/renderer/service_worker/service_worker_context_client.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/renderer/service_worker/service_worker_context_client_unittest.cc
[modify] https://crrev.com/b7da35746e044414f0ecda8fad9b92c88bd26f21/content/renderer/service_worker/service_worker_type_util.h
Project Member

Comment 17 by bugdroid1@chromium.org, Aug 20 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/24bc3bf5c21ede7f0b2d145dfea192d2959543c2

commit 24bc3bf5c21ede7f0b2d145dfea192d2959543c2
Author: Matt Falkenhagen <falken@chromium.org>
Date: Mon Aug 20 10:25:38 2018

service worker: Add tests for throttling and service worker interception.

This adds tests that won't pass yet for a smaller CL. Split off of
https://chromium-review.googlesource.com/c/chromium/src/+/1174204

Tests that:
- FetchEventForNavigationHasThrottledRequest:
    The service worker fetch event observes headers modified by a
    throttle during navigation.  This is valid in both NetworkService
    and S13nServiceWorker.
- RedirectOccursBeforeFetchEvent:
    The service worker only sees the post-redirect request when a
    throttle redirects. This is valid in both NetworkService and
    S13nServiceWorker.
- NavigationHasThrottledRequestHeadersAfterNetworkFallback:
    After a service worker falls back to network, the request
    contains the headers modified by throttles during navigation.
    This is only valid for NetworkService. Headers are not
    propagated to the network request in the S13nServiceWorker
    case.
- NavigationPreloadHasThrottledRequestHeaders:
    When navigation preload is enabled, the navigation preload
    request contains the headers modified by throttles
    during navigation. This is valid for both NetworkService
    and S13nServiceWorker.

Bug:  873575 
Change-Id: Id7d74a9cf7f0a4f15eebe06882039a108e176263
Reviewed-on: https://chromium-review.googlesource.com/1180832
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584394}
[modify] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/test/data/service_worker/create_service_worker.html
[add] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/test/data/service_worker/echo_request_headers.js
[add] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/test/data/service_worker/echo_request_headers.js.mock-http-headers
[modify] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/test/data/service_worker/fetch_event_pass_through.js
[modify] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/test/data/service_worker/fetch_event_pass_through.js.mock-http-headers
[add] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/test/data/service_worker/navigation_preload_worker.js
[add] https://crrev.com/24bc3bf5c21ede7f0b2d145dfea192d2959543c2/content/test/data/service_worker/navigation_preload_worker.js.mock-http-headers
Project Member

Comment 18 by bugdroid1@chromium.org, Aug 22 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4376512e75b378efdacc9ad1d7af6a8871f92ccf

commit 4376512e75b378efdacc9ad1d7af6a8871f92ccf
Author: Matt Falkenhagen <falken@chromium.org>
Date: Wed Aug 22 19:37:22 2018

service worker: Apply navigation throttles before the fetch event.

First some background (true both before and after this CL).
NavigationURLLoaderImpl roughly operates like this:
1. Ask interceptors whether they want to handle a request until
   one says yes (call MaybeCreateLoader).
2. Apply URLLoaderThrottles to the request, possibly rewriting it.
3. Have the interceptor handle the request (invoke its RequestHandler).

But before this CL, the interceptor could not observe the rewritten
request. Therefore, this CL:
1. Adds |resource_request| to RequestHandler signature.
2. Renames the request passed to MaybeCreateLoader to
   |tentative_resource_request| to make more clear that it is not
   necessarily the final request.

Of course, in the current code, interceptors are treating
|tentative_resource_request| as the final request, so this CL adds TODOs
to those interceptors.

The motivation of this CL is service worker interception. Before this
CL, service worker interception would dispatch the fetch event and get a
response before calling the LoaderCallback indicating whether the
service worker wants to handle the request. This was convenient for
network fallback in case the service worker fetch event did not provide
a response.

This CL changes service worker interception to operate after throttling.
This has several motivations:
1. The service worker fetch event may provide an inappropriate response
   for the request if it sees the pre-throttled request rather than the
   post-throttled request.
2. If throttling changes the URL, it would result in a simulated
   redirect, so the service worker interception would have been
   wasteful.
3. This also fixes the missing variation header for navigation preload
   requests, since the throttles will add the header before the fetch
   event.

Change-Id: If13ff956e46de91abb43f3919f869dc5e1eba3e6
Bug:  873061 ,  873575 , 876531, 876527
Reviewed-on: https://chromium-review.googlesource.com/1174204
Reviewed-by: Victor Costan <pwnall@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585201}
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/chrome/browser/offline_pages/offline_page_request_handler_unittest.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/chrome/browser/offline_pages/offline_page_url_loader.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/chrome/browser/offline_pages/offline_page_url_loader.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/chrome/browser/offline_pages/offline_page_url_loader_request_interceptor.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/chrome/browser/offline_pages/offline_page_url_loader_request_interceptor.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/loader/navigation_loader_interceptor.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/loader/navigation_url_loader_impl_unittest.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_controllee_request_handler.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_controllee_request_handler.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_controllee_request_handler_unittest.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_navigation_loader.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_navigation_loader.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_navigation_loader_unittest.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_request_handler.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_request_handler.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/service_worker/service_worker_request_handler_unittest.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/shared_worker/shared_worker_script_loader.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/shared_worker/shared_worker_script_loader.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/web_package/signed_exchange_request_handler.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/browser/web_package/signed_exchange_request_handler.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/common/single_request_url_loader_factory.cc
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/common/single_request_url_loader_factory.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/public/browser/url_loader_request_interceptor.h
[modify] https://crrev.com/4376512e75b378efdacc9ad1d7af6a8871f92ccf/content/public/common/url_loader_throttle.h
Project Member

Comment 19 by bugdroid1@chromium.org, Aug 23 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/446a85514f577f794d4da755e3ac48fb3a5bc2b2

commit 446a85514f577f794d4da755e3ac48fb3a5bc2b2
Author: Matt Falkenhagen <falken@chromium.org>
Date: Thu Aug 23 00:46:01 2018

service worker: Disable a nav preload test under NetworkService.

ServiceWorkerNavigationPreloadTest.NetworkFallback is flaky when
NetworkService is on, after r585201. Temporarily disabling
to soothe the CQ while I investigate.

Bug:  876911 ,   873575 
Change-Id: I8b5cdd9ae8ac173a2a6debba05219bad7fa0d606
TBR: kinuko
NOTRY: true
Reviewed-on: https://chromium-review.googlesource.com/1186003
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585343}
[modify] https://crrev.com/446a85514f577f794d4da755e3ac48fb3a5bc2b2/testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter
Project Member

Comment 20 by bugdroid1@chromium.org, Aug 28 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc3e7e5ac51c7e608c768ab6c73bcad81a3daee2

commit cc3e7e5ac51c7e608c768ab6c73bcad81a3daee2
Author: Matt Falkenhagen <falken@chromium.org>
Date: Tue Aug 28 06:23:48 2018

service worker: Don't throttle again after network fallback.

Throttles can defer the request and possibly impact performance.
There's evidence of this happening like issue 817909.

Before this CL, service worker network fallback would incur throttling
twice. This CL adds RestartWithFactory to ThrottlingURLLoader to make
network fallback a lightweight internal swap of the destination factory
after throttling already occurred.

Bug:  873575 ,  876983 
Change-Id: Icd44fdf13a27ad1a95e39a6640f7ce0c15cd9184
Reviewed-on: https://chromium-review.googlesource.com/1189250
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586598}
[modify] https://crrev.com/cc3e7e5ac51c7e608c768ab6c73bcad81a3daee2/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/cc3e7e5ac51c7e608c768ab6c73bcad81a3daee2/content/common/throttling_url_loader.cc
[modify] https://crrev.com/cc3e7e5ac51c7e608c768ab6c73bcad81a3daee2/content/common/throttling_url_loader.h

Comment 21 by falken@chromium.org, Aug 28

Labels: M-70
Status: Fixed (was: Started)
This should mostly fixed now. The implemented behavior is:
- Throttles are applied prior to the service worker fetch event (in the navigation case).
- Navigation preload uses the post-throttled request, but it does not itself get re-throttled. It's expected that if the navigation preload response is used via respondWith(), the throttles for the navigation will be able to operate on that response.

I haven't looked at the subresource case, but it sounds like throttles will apply to requests issued by fetch() at least. It's unknown if they need to apply before the fetch event.

Comment 22 by falken@google.com, Jan 18 (5 days ago)

Blocking: 873061
►

Sign in to add a comment