New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 724322 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug
Proj-Servicification

Blocking:
issue 598073
issue 724323



Sign in to add a comment

Add method to support chaining of URLLoader creation for the network service

Project Member Reported by jam@chromium.org, May 18 2017

Issue description

(Kinuko: added a separate bug for this to track the effort)
(this is from comment #132 in parent bug)


We (jam@, scottmg@, ananta@ and kinuko@) discussed how URLLoader{Factory,} could be set up for a main resource for navigation, and here's a rough note:

Some basic facts:
- For main resources it's possible that, say, a request goes to SW -> network -> (redirect) -> SW
- The lifetime of the navigation request / NavigationURLLoader and that of URLLoader can be different, as a single request may talk to multiple URLLoader(Factory)
- We can't tell which URLLoader(Factory) will serve the final request beforehand, and resolution could happen asyncly
- In redirects we'll need to restart the process, and some redirect contexts / states will need to be kept around across redirect legs

High-level possible design:
- When we start a request we initialize a chain of URLLoaderFactory in NavigationURLLoader
- Each feature's URLLoaderFactory may forward the request to the next URLLoaderFactory in the chain if it finds it can't handle it
- In redirect case the request will start from the beginning of the chain
- Each feature may keep track the redirect/restart state during the request (e.g. in their own request_handlers in SW and AppCache cases, at least initially)
- We could have URLLoaderFactoryManager or something to keep/manage all these factories and states during the lifetime of the navigation request
 

Comment 1 by jam@chromium.org, May 18 2017

Blocking: 724323

Comment 2 by kinuko@chromium.org, May 22 2017

Status: Started (was: Assigned)

Comment 3 by kinuko@chromium.org, May 23 2017

(Many meetings today, wasn't able to make it ready for review yet but will up something in a few days)

Comment 4 by yzshen@chromium.org, May 24 2017

Components: Internals>Network>Service
Cc: michaeln@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, May 29 2017

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

commit 697329710bb599e46ae7dc72dfc5f88c4abc4545
Author: kinuko <kinuko@chromium.org>
Date: Mon May 29 08:50:07 2017

Network service: Implement URLLoader chaining for interceptors

BUG= 724322 

Review-Url: https://codereview.chromium.org/2897063002
Cr-Commit-Position: refs/heads/master@{#475314}

[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/BUILD.gn
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/loader/navigation_url_loader_network_service.h
[add] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/loader/url_loader_request_handler.h
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_controllee_request_handler.cc
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_controllee_request_handler.h
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_request_handler.cc
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_request_handler.h
[add] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_response_type.h
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_url_job_wrapper.cc
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_url_job_wrapper.h
[modify] https://crrev.com/697329710bb599e46ae7dc72dfc5f88c4abc4545/content/browser/service_worker/service_worker_url_request_job.h

Comment 7 by kinuko@chromium.org, May 29 2017

Status: Fixed (was: Started)

Comment 8 by jam@chromium.org, Jun 1 2017

Kinuko: as part of this can we remove the URLLoaderFactory and just have URLLoaderRequestHandler create a URLLoader directly?
Yeah let me file a new bug, I agree that factory thing feels confusing in the new code.
Filed:  issue 729503 
Project Member

Comment 11 by bugdroid1@chromium.org, Jun 6 2017

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.

Sign in to add a comment