New issue
Advanced search Search tips

Issue 898755 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

NavigationLoaderInterceptors should be owned by SharedWorkerScriptFetcher, not SharedWorkerScriptLoader

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

Issue description

Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 25

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

commit e1aa6756c8c5030a6888fb470e3116b875f23bbf
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Oct 25 09:30:55 2018

SharedWorker: Remove an unused param from SharedWorkerScriptFetcher::CreateAndStart()

Bug: 898755
Change-Id: Ic2383ec4f845ab86e0487e4f859fa3f02efa8b1f
Reviewed-on: https://chromium-review.googlesource.com/c/1298531
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602653}
[modify] https://crrev.com/e1aa6756c8c5030a6888fb470e3116b875f23bbf/content/browser/shared_worker/shared_worker_script_loader_factory.cc
[modify] https://crrev.com/e1aa6756c8c5030a6888fb470e3116b875f23bbf/content/browser/shared_worker/shared_worker_script_loader_factory.h
[modify] https://crrev.com/e1aa6756c8c5030a6888fb470e3116b875f23bbf/content/browser/shared_worker/shared_worker_service_impl.cc

Copied horo@'s comment from the link on the reporter's comment:

> I think this complexity is coming from the wired design of SharedWorkerScriptLoader and SharedWorkerScriptFetcher and the interceptors.
> 
> In navigation_url_loader_impl.cc, NavigationLoaderInterceptor::MaybeCreateLoader() is called before creating a ThrottlingURLLoader.
But while loading the shared worker script, NavigationLoaderInterceptor::MaybeCreateLoader() is called after creating a ThrottlingURLLoader.
I think SharedWorkerScriptFetcher should have the interceptors, and NavigationLoaderInterceptor::MaybeCreateLoader() should be called at SharedWorkerScriptFetcher::Start() and after redirection.
And also, NavigationLoaderInterceptor::MaybeCreateLoaderForResponse() should be called at  SharedWorkerScriptFetcher::OnReceiveResponse().
I tried this in:
https://chromium-review.googlesource.com/c/chromium/src/+/1304113

It turned out that this cleanup doesn't work until NetworkService is enabled. This is because when ServicificationServiceWorker is on but NetworkService is off, SharedWorkerScriptLoader is directly invoked via mojo connection without SharedWorkerScriptFetcher, so the interceptor handling needs to be in SharedWorkerScriptLoader.

I'll try this cleanup again after NetworkService is enabled by default.

Sign in to add a comment