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

Issue 715632 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug


Sign in to add a comment

Appcache shouldn't hook into the network layer (and also implement PlzWorker for shared workers)

Project Member Reported by jam@chromium.org, Apr 26 2017

Issue description

Per design doc: https://docs.google.com/document/d/1r3jovmFiSo8Myz6iYutK6MiVddnJ0jv0vnMoTGcU17Q/edit

AppCache should be layered ontop of the network service. i.e. it should use URLLoaderFactory to fetch the cached URLs. PlzNavigate should be able to check with AppCache code whether to use an AppCache URLLoaderFactory if a cache exists, and if so, it should pass a URLLoaderFactory to the renderer for subresources. The end goal should be that we don't need to hook into net/ for AppCache.

===
Update: As part of this bug, we had to implement "PlzWorker" for shared workers, see c39 and below.
 

Comment 1 by jam@chromium.org, Apr 26 2017

Cc: kinuko@chromium.org falken@chromium.org

Comment 2 by ananta@chromium.org, Apr 29 2017

Status: Started (was: Assigned)
The plan is to use the AppCacheRequestHandler class and the AppCacheResponseReader class to handle AppCache requests in the n/w service world. The AppCacheRequestHandler has dependencies on URLRequest which won't work in the n/w service world. First step is to provide an abstraction for the request object and then an abstraction for the AppCacheURLRequestJob. 
Patch here https://codereview.chromium.org/2848493007

Comment 3 by jam@chromium.org, May 2 2017

Blockedon: 712913
Project Member

Comment 4 by bugdroid1@chromium.org, May 4 2017

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

commit 4e8a529c38afa4770486d8033cb37af0d6caf311
Author: ananta <ananta@chromium.org>
Date: Thu May 04 03:07:15 2017

Reduce/Remove URLRequest dependencies from AppCacheRequestHandler

The AppCacheRequestHandler class provides functionality for serving network requests from
the AppCache if applicable. This class is instantiated by the AppCacheInterceptor when a network request
is initiated. Its lifetime depends on the associated URLRequest.

The plan is to reuse this class in the network service world where we won't be intercepting
network requests in the browser process. This effectively means that there won't be any URLRequest as well

To achieve this the proposal is to provide an abstraction for a request called AppCachRequest. This class
will have two subclasses at the moment. AppCacheURLRequest and AppCacheURLLoaderRequest. The AppCacheURLRequest
class will be used by the current network implementation which relies on intercepting n/w requests.

The AppCacheURLLoaderRequest class will be used by the network service mojo implementation.
Next step is to provide an abstraction for the AppCacheURLRequestJob class.

BUG= 715632 
TBR=jam

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

[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/BUILD.gn
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_host.cc
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_host.h
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_interceptor.cc
[add] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_request.cc
[add] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_request.h
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_request_handler_unittest.cc
[add] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_url_loader_request.cc
[add] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_url_loader_request.h
[add] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_url_request.cc
[add] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/browser/appcache/appcache_url_request.h
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/common/appcache_interfaces.cc
[modify] https://crrev.com/4e8a529c38afa4770486d8033cb37af0d6caf311/content/common/appcache_interfaces.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 9 2017

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

commit 46f3e94b444a8fa09c22cdd0b0d3313414feb0c1
Author: ananta <ananta@chromium.org>
Date: Tue May 09 04:56:30 2017

Add an abstraction for a job in the AppCacheRequestHandler class.

The abstraction is in the form of an AppCacheJob base class which has two subclasses.
1. AppCacheURLRequestJob : This is the current URLRequestJob implementation which relies
   on URLRequest interception to serve requests from the cache.

2. AppCacheURLLoaderJob : This will eventually be the job mechanism for the URLLoader based
   AppCache functionality (network service).

The AppCacheJob provides a number of methods like DeliverAppCachedResponse(),
DeliverNetworkResponse(), DeliverErrorResponse(), etc which allow the job to control the content
being served out.

The expectation is that with this abstraction, we should have minimal changes in core AppCache
functionality for the URLLoader work. We will mostly be an AppCache consumer.

BUG= 715632 

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

[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/BUILD.gn
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_interceptor.cc
[add] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_job.cc
[add] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_job.h
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_request.h
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_request_handler_unittest.cc
[add] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_url_loader_job.cc
[add] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_url_request_job.cc
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_url_request_job.h
[modify] https://crrev.com/46f3e94b444a8fa09c22cdd0b0d3313414feb0c1/content/browser/appcache/appcache_url_request_job_unittest.cc

Project Member

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

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

commit e3f159a4458149bd184fcd1f523a5d9d0a6687f7
Author: ananta <ananta@chromium.org>
Date: Thu May 11 23:40:07 2017

Provide skeleton functionality for AppCache handling in the network service.

We have a new class AppCacheNetworkServiceHandler which gets created on the IO thread.
This class provides functionality for checking if a request can be serviced out of the AppCache.
I was looking at AppCacheRequestHandler for this as well. However that class is tied deeply to
the underlying job which gets a touch complicated for lifetime managemnt if the request is not really
being served out the cache.

Added some comments for that class to see if we could remove it once the pieces begin to align.

This class implements the AppCacheStorage::Delegate interface which tells us if a response is available
for a URL. Currently we just trace if a response is found and call the StartURLRequest callback in both cases.

Going forward the plan is to create a URLLoaderFactory for AppCache and pass it back to the callback.
Optionally we could create the loader right there as well. Thanks to jam for pointing that out.

The other changes are to have the original ResourceRequest pointer in the AppCacheURLLoaderRequest class instead
of a copy. We ensure that ownership is transferred correctly.

BUG= 715632 

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

[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/BUILD.gn
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_job.cc
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_job.h
[add] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_network_service_handler.cc
[add] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_network_service_handler.h
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_url_loader_request.cc
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/appcache/appcache_url_loader_request.h
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/e3f159a4458149bd184fcd1f523a5d9d0a6687f7/content/browser/loader/navigation_url_loader_network_service.h

Project Member

Comment 7 by bugdroid1@chromium.org, May 12 2017

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

commit 942998b3db2c227e559abe439b2cc4ce16567673
Author: jam <jam@chromium.org>
Date: Fri May 12 23:11:40 2017

Fix crash in DataUrlNavigationBrowserTest.PDF_FormPost_Block with --enable-network-service.

BUG= 715632 

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

[modify] https://crrev.com/942998b3db2c227e559abe439b2cc4ce16567673/content/browser/loader/navigation_url_loader_network_service.cc

Project Member

Comment 8 by bugdroid1@chromium.org, May 19 2017

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

commit 2e65213dd03a7a0f277c5d16d460ae22d391f33c
Author: ananta <ananta@chromium.org>
Date: Fri May 19 04:08:24 2017

Add a stub implementation of the URLLoaderFactory for AppCache.

The AppCache factory has one instance per Storage partition and is maintained by the
URLLoaderFactoryGetter class.

The AppCache factory maintains a reference to the URLLoaderFactoryGetter instance
and uses it to retrieve the network service to issue unhandled requests to it.

Currently all requests are issued to the network service. Next step is to add functionality
to serve AppCache responses from the AppCache.

We no longer need the AppCacheNetworkServiceHandler class which was added in the last patchset.
Additionally the corresponding methods to initialize the AppCache factory in the AppCacheRequestHandler
class are also no longer needed.

BUG= 715632 

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

[modify] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/BUILD.gn
[delete] https://crrev.com/c4de0303f3d7699d558f8042f946a2825e2d248a/content/browser/appcache/appcache_network_service_handler.cc
[delete] https://crrev.com/c4de0303f3d7699d558f8042f946a2825e2d248a/content/browser/appcache/appcache_network_service_handler.h
[modify] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/appcache/appcache_request_handler.h
[add] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/appcache/appcache_url_loader_factory.cc
[add] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/appcache/appcache_url_loader_factory.h
[modify] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/url_loader_factory_getter.cc
[modify] https://crrev.com/2e65213dd03a7a0f277c5d16d460ae22d391f33c/content/browser/url_loader_factory_getter.h

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

Components: Internals>Network>Service
Project Member

Comment 10 by bugdroid1@chromium.org, Jun 9 2017

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

commit a2c8ec6149d02d7cfe9389d9739236ed06571a6a
Author: ananta <ananta@chromium.org>
Date: Fri Jun 09 23:44:05 2017

Get main frame and subframe AppCache loads to work.

The latest iteration of this patch now has AppCache working for the main frame and
subframe. Subresource loads will be addressed in a later patchset. With this change
the w3schools.com AppCache sample here https://www.w3schools.com/tags/tryit.asp?filename=tryhtml5_html_manifest
works.

This has been integrated with kinuko's patch, i.e. we implement the URLLoaderRequestHandler interface to
validate if the request can be serviced via the AppCache.

The URLLoaderFactory for AppCache is now instantiated as needed.

Changes in this patch include the following:
1. The AppCacheRequestHandler class now implements the URLLoaderRequestHandler interface. The handler then initializes
   the AppCacheURLLoader job which allows us to determine if the request would be serviced via the AppCache. If yes
   we return the URLLoaderFactory for the AppCache in the callback or pass null to the callback.

2. We call the AppCacheRequestHandler::InitializeForNavigationNetworkService() function from
   NavigationURLLoaderNetworkService::URLLoaderRequestController::Start() for AppCache.

3. Use the AppCacheResponseReader and AppCacheResponseInfo classes in AppCacheURLLoaderJob to read data from the AppCache.
   The job invokes methods on the URLLoaderClient when it reads the response/data, etc.
   The data from the AppCache is written to a mojo data pipe and passed to the client.

4. I reverted portions of the previous change to the AppCacheURLLoaderRequest class, which was to hold on to a
   unique_ptr for the ResourceRequest. We now hold on to a copy of the request. The CreateLoaderAndStart() method on the factory gives us a
   copy of the request.

5. Moved some of the methods like IsWaiting/IsDeliveringAppCacheResponse/IsDeliveringNetworkResponse/IsDeliveringErrorResponse/IsCacheEntryNotFound to the
   AppCacheJob base class as these implementations could be shared between the URLRequest and URLLoader versions of the subclasses. I also moved the
   member variables used by these methods to the base class.

6. Add WeakPtrFactory to the AppCacheStorage class and provide a getter to return a weak pointer reference to the instance.

BUG= 715632 

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

[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_job.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_job.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_storage.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_storage.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_storage_impl.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_storage_impl.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_loader_factory.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_loader_factory.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_loader_request.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_loader_request.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_request_job.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/appcache_url_request_job.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/mock_appcache_storage.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/appcache/mock_appcache_storage.h
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/url_loader_factory_getter.cc
[modify] https://crrev.com/a2c8ec6149d02d7cfe9389d9739236ed06571a6a/content/browser/url_loader_factory_getter.h

Project Member

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

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

commit 4c0cb6be7d1846be9fc4c2d6fb2642ead6a3bb1d
Author: ananta <ananta@chromium.org>
Date: Thu Jun 15 04:27:43 2017

Add support for HTTP range requests in the AppCacheURLLoaderImpl class.

The functions in the AppCacheURLRequest class which parsed the HTTP header to initialize
the range information and setup the range response on receiving an AppCache response
have been moved to the AppCacheJob base class. The member variables to track the AppCache
response, the range response and the input range requests have also been moved to the base
class.

Functions moved/added to the AppCacheJob base class are:
1. InitializeRangeRequestInfo(): Initializes the input range request info.
2. SetupRangeResponse : Sets up the range response information.

BUG= 715632 

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

[modify] https://crrev.com/4c0cb6be7d1846be9fc4c2d6fb2642ead6a3bb1d/content/browser/appcache/appcache_job.cc
[modify] https://crrev.com/4c0cb6be7d1846be9fc4c2d6fb2642ead6a3bb1d/content/browser/appcache/appcache_job.h
[modify] https://crrev.com/4c0cb6be7d1846be9fc4c2d6fb2642ead6a3bb1d/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/4c0cb6be7d1846be9fc4c2d6fb2642ead6a3bb1d/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/4c0cb6be7d1846be9fc4c2d6fb2642ead6a3bb1d/content/browser/appcache/appcache_url_request_job.cc
[modify] https://crrev.com/4c0cb6be7d1846be9fc4c2d6fb2642ead6a3bb1d/content/browser/appcache/appcache_url_request_job.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 12 2017

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

commit 2533bff6fceb80f4ac6b582e4ffbf45bfde2a064
Author: ananta <ananta@chromium.org>
Date: Wed Jul 12 04:08:10 2017

Add support for subresource request loads in AppCache for the network service.

This patch builds on the earlier patch where we passed the URLLoaderFactory for AppCache
to the renderer process. The renderer invokes CreateLoaderAndStart which comes to the AppCache
factory. We then instantiate the AppCacheRequestHandler instance here and call into it to handle
the request. The handler creates the AppCacheURLLoaderJob and passes itself to the job. The job controls
the lifetime of the handler.

The AppCacheSubresourceURLFactory class is passed the precreated AppCache host during init. We save
maintain this as a weak reference in the class to protect against the case where the host dies while we are
processing the call from the renderer. Added support to get a weak pointer in the AppCacheHost class.
The AppCacheRequestHandler saves away the host as a weak reference during InitializeForNavigationNetworkService()
and passes to the factory in MaybeCreateSubresourceFactory()

The other changes are to pass the information from CreateLoaderAndStart() to the AppCacheRequestHandler class
and from there to the job. A bug was fixed in the AppCacheURLLoaderJob where we weren't terminating the write correctly
causing the renderer to not consider the load as complete.

With this change normal subresource loads will work. Fallback support is not added yet and will happen in a followup.

BUG= 715632 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_host.cc
[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_host.h
[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_subresource_url_factory.h
[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/2533bff6fceb80f4ac6b582e4ffbf45bfde2a064/content/browser/appcache/appcache_url_loader_job.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 18 2017

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

commit 2ec636380ffa2f4df27e3525a32f7aae6658b58b
Author: ananta <ananta@chromium.org>
Date: Tue Jul 18 22:43:53 2017

Add support for subresource request fallback in AppCache for the network service..

This patch builds on patch https://codereview.chromium.org/2956373002/ for adding fallback
support for subresource requests. This is implemented by passing a proxy URLLoaderClient
interface to the network URL service for subresource requests which are sent to the network.
We intercept the OnReceiveResponse, OnReceiveRedirect and OnComplete calls for supporting
fallback responses in these cases. We could potentially look at moving fallback response
reading to the renderer in a later patchset.

The changes in this patchset are as below:
1. Add support for passing the ResourceResponseHead to the AppCacheURLLoaderJob. This is
   required for supporting fallback responses. The request handler looks at headers in
   the response and the status code to make a call whether a fallback is required.

2. AppCacheRequestHandler::MaybeLoadFallbackForResponse() and MaybeLoadFallbackForRedirect() have
   changed to allow for the fact that they are called in the network service code by the
   AppCacheURLLoaderJob. We don't create new job for delivering the fallback in this case.

3. Added methods in the AppCacheRequest class  to return the URLRequest override and URLLoader
   override respectively.

Next step is to add support for main resource fallback.

BUG= 715632 

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

[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_job.cc
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_request.cc
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_request.h
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_url_loader_request.cc
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_url_loader_request.h
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_url_request.cc
[modify] https://crrev.com/2ec636380ffa2f4df27e3525a32f7aae6658b58b/content/browser/appcache/appcache_url_request.h

Comment 14 by jam@chromium.org, Jul 20 2017

Blockedon: 747130
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 26 2017

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

commit 4a9dd98efab25e78f28b800520c1868aaad2cb1f
Author: ananta <ananta@chromium.org>
Date: Wed Jul 26 18:44:35 2017

The Appcache subresource URL factory needs to inform the URLLoaderClient if there is a failure.

Failure cases include, an invalid AppCacheHost instance, failure to create a request handler, etc.
We already notify the client about the latter case above. Need to do this for an invalid host as well.
This caused the w3schools AppCache test page to spin indefinitely if we pass in an invalid URL.

Additionally on the renderer side we need to reset the cached URLLoaderFactory pointer if a null factory
is passed in the CommitNavigation IPC. This ensures that the request hits the default network loader.

BUG= 715632 
TEST=Covered by browser test. AppCacheNetworkServiceBrowserTest.VerifySubresourceFactoryClearedOnNewNavigation

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

[add] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/browser/appcache/appcache_browsertest.cc
[modify] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/browser/appcache/appcache_subresource_url_factory.h
[modify] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/test/BUILD.gn
[add] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/test/data/appcache/logo.png
[add] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/test/data/appcache/simple_page.manifest
[add] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/test/data/appcache/simple_page_no_manifest.html
[add] https://crrev.com/4a9dd98efab25e78f28b800520c1868aaad2cb1f/content/test/data/appcache/simple_page_with_manifest.html

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 27 2017

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

commit a08d0fc015c42ce95a104c0f4e3de9448b7721e1
Author: mmenke <mmenke@chromium.org>
Date: Thu Jul 27 04:47:20 2017

Revert of The Appcache subresource URL factory needs to inform the URLLoaderClient if there is a failure. (patchset #7 id:120001 of https://codereview.chromium.org/2991443002/ )

Reason for revert:
AppCacheNetworkServiceBrowserTest.VerifySubresourceFactoryClearedOnNewNavigation is very flaky, with a lot of red try runs on the CQ and the builders.

Windows CQ has a 2 hour backlog (Or did earlier), and while this isn't the only reason, it looks like a major contributor.

Original issue's description:
> The Appcache subresource URL factory needs to inform the URLLoaderClient if there is a failure.
>
> Failure cases include, an invalid AppCacheHost instance, failure to create a request handler, etc.
> We already notify the client about the latter case above. Need to do this for an invalid host as well.
> This caused the w3schools AppCache test page to spin indefinitely if we pass in an invalid URL.
>
> Additionally on the renderer side we need to reset the cached URLLoaderFactory pointer if a null factory
> is passed in the CommitNavigation IPC. This ensures that the request hits the default network loader.
>
> BUG= 715632 
> TEST=Covered by browser test. AppCacheNetworkServiceBrowserTest.VerifySubresourceFactoryClearedOnNewNavigation
>
> Review-Url: https://codereview.chromium.org/2991443002
> Cr-Commit-Position: refs/heads/master@{#489710}
> Committed: https://chromium.googlesource.com/chromium/src/+/4a9dd98efab25e78f28b800520c1868aaad2cb1f

TBR=jam@chromium.org,michaeln@chromium.org,ananta@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 715632 

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

[delete] https://crrev.com/e74c254a7b2e33377d52f6b8bef2ea79463c1edb/content/browser/appcache/appcache_browsertest.cc
[modify] https://crrev.com/a08d0fc015c42ce95a104c0f4e3de9448b7721e1/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/a08d0fc015c42ce95a104c0f4e3de9448b7721e1/content/browser/appcache/appcache_subresource_url_factory.h
[modify] https://crrev.com/a08d0fc015c42ce95a104c0f4e3de9448b7721e1/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/a08d0fc015c42ce95a104c0f4e3de9448b7721e1/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/a08d0fc015c42ce95a104c0f4e3de9448b7721e1/content/test/BUILD.gn
[delete] https://crrev.com/e74c254a7b2e33377d52f6b8bef2ea79463c1edb/content/test/data/appcache/logo.png
[delete] https://crrev.com/e74c254a7b2e33377d52f6b8bef2ea79463c1edb/content/test/data/appcache/simple_page.manifest
[delete] https://crrev.com/e74c254a7b2e33377d52f6b8bef2ea79463c1edb/content/test/data/appcache/simple_page_no_manifest.html
[delete] https://crrev.com/e74c254a7b2e33377d52f6b8bef2ea79463c1edb/content/test/data/appcache/simple_page_with_manifest.html

Project Member

Comment 18 by bugdroid1@chromium.org, Jul 27 2017

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

commit aad9e6266f96e08ae80b9af238a9e6121a28c76e
Author: ananta <ananta@chromium.org>
Date: Thu Jul 27 22:41:41 2017

The Appcache subresource URL factory needs to inform the URLLoaderClient if there is a failure.

Relanding this with the browser test fixes.

Failure cases include, an invalid AppCacheHost instance, failure to create a request handler, etc.
We already notify the client about the latter case above. Need to do this for an invalid host as well.
This caused the w3schools AppCache test page to spin indefinitely if we pass in an invalid URL.

Additionally on the renderer side we need to reset the cached URLLoaderFactory pointer if a null factory
is passed in the CommitNavigation IPC. This ensures that the request hits the default network loader.

BUG= 715632 
TEST=Covered by browser test. AppCacheNetworkServiceBrowserTest.VerifySubresourceFactoryClearedOnNewNavigation

This reverts commit a08d0fc015c42ce95a104c0f4e3de9448b7721e1.

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

[add] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/browser/appcache/appcache_browsertest.cc
[modify] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/browser/appcache/appcache_subresource_url_factory.h
[modify] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/test/BUILD.gn
[add] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/test/data/appcache/logo.png
[add] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/test/data/appcache/simple_page.manifest
[add] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/test/data/appcache/simple_page_no_manifest.html
[add] https://crrev.com/aad9e6266f96e08ae80b9af238a9e6121a28c76e/content/test/data/appcache/simple_page_with_manifest.html

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 29 2017

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

commit b9800e5823ff3358a4f90d1a9e72c26a3d742988
Author: ananta <ananta@chromium.org>
Date: Sat Jul 29 18:04:13 2017

Add support for fallback content for the frame. This includes main and subframes.

To achieve this the following changes were done.
1. The URLLoaderRequestHandler which is implemented by the navigation request handlers
   like AppCache/ServiceWorker, etc now has the following method
   MaybeCreateLoaderForResponse(). AppCache returns fallback content for the response
   passed in if applicable.

2. The AppCacheRequestHandler class which implements the URLLoaderRequestHandler interface
   implements the above method. It uses the MaybeLoadFallbackForResponse() function to potentially
   return fallback content. The difference between fallback handling for subresources and the frame
   is a new job is created for the latter. The MaybeCreateJobForFallback() function
   has been updated with this check.

3. The URLLoaderRequestController class which handles top level navigations now provides
   functionality to invoke the registered handlers when it receives a response. This is only done
   if the request was sent to the default network loader. A boolean flag default_loader_used_  is used to
   track this. If this is set we invoke the registered handlers and pass in the bound URLLoaderClient and
   the request to the handlers which enables them to bind to the connection.

4. We don't use the throttling URL loader for this case. This is fine because the main URL would already have been
   checked by the throtting URL loader when it was sent to the network service. Added a function DisconnectClient()
   to the throttling URL loader which disconnects the client from the network service in case we are passing fallback
   content to the client.

BUG= 715632 

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

[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/loader/url_loader_request_handler.cc
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/loader/url_loader_request_handler.h
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/browser/service_worker/service_worker_url_loader_job.cc
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/common/throttling_url_loader.cc
[modify] https://crrev.com/b9800e5823ff3358a4f90d1a9e72c26a3d742988/content/common/throttling_url_loader.h

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 31 2017

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

commit ad498888ddcfe594dbe84e7cf93c341c16d38c08
Author: Matt Falkenhagen <falken@chromium.org>
Date: Mon Jul 31 01:41:03 2017

Gardening: network service: http/tests/appcache/fallback.html

Update expectation from timeout to failure after r490666.

TBR=ananta
NOTRY=true

Bug:  715632 
Change-Id: Ie7932b83bbd0e409da1ffc5d3acfad26c28f3efb
Reviewed-on: https://chromium-review.googlesource.com/592999
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490695}
[modify] https://crrev.com/ad498888ddcfe594dbe84e7cf93c341c16d38c08/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 1 2017

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

commit d42af8f386d8b6316af705bdde51750ebb33bc8b
Author: Anantanarayanan Iyengar <ananta@chromium.org>
Date: Tue Aug 01 23:09:36 2017

Add an abstraction for the URLRequest class which is used by the AppCacheUpdateJob's URLFetcher.

This is to ensure that we can provide functionality to update the AppCache using the URLLoader.
The plan is to implement the URLLoaderClient part of the update job in a subsequent patch.

This patch has the following changes:
1. We have a new class AppCacheUpdateRequestBase which provides the interface which is
   implemented by the URLRequest and URLLoaderClient subclasses. This interface is used
   by the URLFetcher and the AppCacheUpdateJob.

2. Subclasses AppCacheUpdateURLRequest and AppCacheUpdateURLLoaderRequest. These implement
   the URLRequest and URLLoaderClient portion of the update functionality. The URLRequest
   one is fully functional at this point.

3. I have left most of the reading and updating logic unchanged in URLFetcher unchanged.
   Will revisit this when I start working on the URLLoaderClient functionality.

4. The AppCacheUpdateURLRequest subclass maintains a pointer to the URLFetcher base and
   invokes its methods when it receives a response/redirect notification/data, etc.

BUG= 715632 

Change-Id: I16497e41aa5dbfab0c06c1ab0fc114945dd279e8
Reviewed-on: https://chromium-review.googlesource.com/592809
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Ananta Iyengar <ananta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491147}
[modify] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/BUILD.gn
[modify] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_job.cc
[modify] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_job.h
[add] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_request_base.cc
[add] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_request_base.h
[modify] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_url_fetcher.cc
[modify] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_url_fetcher.h
[add] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_url_loader_request.cc
[add] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_url_loader_request.h
[add] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_url_request.cc
[add] https://crrev.com/d42af8f386d8b6316af705bdde51750ebb33bc8b/content/browser/appcache/appcache_update_url_request.h

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 9 2017

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

commit 25dba32e4601e05935debb62fcd95e5123371a58
Author: Anantanarayanan Iyengar <ananta@chromium.org>
Date: Wed Aug 09 03:32:43 2017

Add support for updating the AppCache using the network URL loader.

This patch adds the implementation for the methods of the AppCacheUpdateJob::UpdateURLLoaderRequest
class added in the last iteration.

Following changes:
1. Bring back the helper classes from mojo for facilitating copying between a mojo buffer and net.
   These classes are MojoToNetPendingBuffer and MojoToNetIOBuffer.

2. The UpdateRequestBase::Read function does not take an IOBuffer anymore. Instead the
   subclasses (URLRequest and URLLoader) pass the IOBuffer to AppCacheUpdateJob::URLFetcher::OnReadCompleted()

3. AppCacheUpdateJob::URLFetcher does not maintain an IOBuffer anymore. The buffers are maintained by the
   request subclasses.

4. The AppCacheUpdateJob::UpdateURLRequest::Read implicitly invokes the fetchers  OnReadCompleted() function
   if the read is synchronously completed. It always returns ERR_IO_PENDING though.

5. The AppCacheServiceImpl class now saves away the URLLoaderFactoryGetter class. This
   enables the updater based on URLLoader to retrieve the network service from there.

BUG= 715632 
TEST=Verified by manual testing on the w3schools AppCache test page and http://webdbg.com/test/appcache/
     The entries get setup correctly and the page loads in offline

Change-Id: Ic7c42471589aff33c68de9af6bafd0c881e5b4db
Reviewed-on: https://chromium-review.googlesource.com/599359
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Commit-Queue: Ananta Iyengar <ananta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492812}
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_service_impl.h
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_job.cc
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_request_base.cc
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_request_base.h
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_url_fetcher.cc
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_url_fetcher.h
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_url_loader_request.cc
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_url_loader_request.h
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_url_request.cc
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/appcache/appcache_update_url_request.h
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/browser/storage_partition_impl.cc
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/common/net_adapters.cc
[modify] https://crrev.com/25dba32e4601e05935debb62fcd95e5123371a58/content/common/net_adapters.h

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 9 2017

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

commit 2bdc53cf1430785617f1ee06a5e9753448083399
Author: yzshen <yzshen@chromium.org>
Date: Wed Aug 09 17:41:32 2017

Network service: update layout test filter.

Unexpected Failures:
* http/tests/appcache/manifest-redirect-2.html
* http/tests/appcache/manifest-redirect.html
* http/tests/appcache/resource-redirect-2.html
* http/tests/appcache/resource-redirect.html

These tests regressed by:
https://chromium-review.googlesource.com/599359

BUG= 715632 
TBR=ananta@chromium.org

Change-Id: Iad6dcf52be9077ed2b319f798f71e0268e1d7062
Reviewed-on: https://chromium-review.googlesource.com/608903
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493043}
[modify] https://crrev.com/2bdc53cf1430785617f1ee06a5e9753448083399/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 19 2017

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

commit d125c4607f072df3165580fe3027b28af2045a02
Author: Anantanarayanan Iyengar <ananta@chromium.org>
Date: Sat Aug 19 00:46:56 2017

Add support for running the AppCacheRequestHandlerTests for URLLoader (network service)

Changes in this patch are as below:

1. The AppCacheRequestHandlerTests are now parameterized. They support two parameters at the
   moment. URLREQUEST which runs the tests using the URLRequest code path and URLLOADER
   which runs these tests using the network service code path.

2. Added a new method MaybeCreateSubresourceLoader to the AppCacheRequestHandler class. This is
   called by the AppCacheSubresourceUrlLoaderFactory when it receives requests to load subresources.
   The SubresourceLoadInfo and the URLLoaderFactoryGetter are passed via the CreateJob() function
   to the AppCacheURLLoaderJob.

3. The AppCacheRequesHandler has two static methods SetRunningInTests() and IsRunningInTest(). These
   are called to indicate we are in tests mode. AppCache URLLoader codepaths use these functions to
   bail early.

4. The other changes are in the tests to account for the differences in behavior between the URLLoader
   and the URLRequest codepaths. 

BUG= 715632 

Change-Id: I5b04cf7d2e5c6f17001191a866a383146bcac8bf
Reviewed-on: https://chromium-review.googlesource.com/612210
Commit-Queue: Ananta Iyengar <ananta@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495768}
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_job.cc
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_job.h
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_request_handler_unittest.cc
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_update_url_loader_request.cc
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/d125c4607f072df3165580fe3027b28af2045a02/content/browser/appcache/appcache_url_loader_request.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 23 2017

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

commit aa70bb4fbef832058376e3117442bec494b23126
Author: Anantanarayanan Iyengar <ananta@chromium.org>
Date: Wed Aug 23 01:38:32 2017

Add support for running the AppCacheUpdateJob tests for the network service (URLLoader) code path.

The main changes are as below:
1. The AppCacheUpdateJobTest class now maintains a pointer to the URLLoaderFactoryGetter class.
   This instance is instantiated for the duration of the test.
2. The mock factory is registered as a test factory on the URLLoaderFactoryGetter via the 
   SetNetworkFactoryForTesting() method.

3. Added a function GetResponseForURL() to the test class RetryRequestTestJob. This mimics the logic in the RetryFactory
   function and is invoked by the mock URLLoaderFactory.

4. Added a function ValidateExtraHeaders() to the test class HttpHeadersRequestTestJob. This checks the cache modified
   headers. It is invoked by the factory function and by the mock URLLoaderFactory.

BUG= 715632 
TBR=jam

Change-Id: I6fbb6a3b582c5971cf8a0c2e6bc9acf7670a696c
Reviewed-on: https://chromium-review.googlesource.com/616256
Commit-Queue: Ananta Iyengar <ananta@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496547}
[modify] https://crrev.com/aa70bb4fbef832058376e3117442bec494b23126/content/browser/appcache/appcache_update_job_unittest.cc
[modify] https://crrev.com/aa70bb4fbef832058376e3117442bec494b23126/content/browser/appcache/appcache_update_url_loader_request.cc
[modify] https://crrev.com/aa70bb4fbef832058376e3117442bec494b23126/content/browser/url_loader_factory_getter.cc
[modify] https://crrev.com/aa70bb4fbef832058376e3117442bec494b23126/content/browser/url_loader_factory_getter.h

Blockedon: 759901
Some notes about a refactor I have in mind regarding subresource loading:

The ownership hieraerchy and control flow is tricky, I'm thinking about taking a cut at restructuring along these lines...

AppCacheSubresourceURLFactory creates a SubresourceLoader that provides the URLLoader bindings to the remote client. Once bound, it sticks.

The SubresourceLoader internally delegates to either an AppCacheLoader to read from disk or a regular loader to load from the network. The SubresourceLoader would be the client of it's delegate loader and proxy URLLoader/Client interactions, it would check for fallback handling in  OnRedirect(), OnResponse(), and FollowRedirect() and switch to an AppCacheLoader when needed.

So the division of labor is more factored

SubresourceLoader mediates between client and the (networkloader or appcacheloader)
NetworkLoader just loads from network
AppcacheLoader just loads from disk

Currently the AppcacheLoader itself is wearing those different hats and its tricky to follow.
Project Member

Comment 29 by bugdroid1@chromium.org, Sep 1 2017

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

commit 2130d27a1e177d73ce3be233fd7c25865515ff9b
Author: Anantanarayanan Iyengar <ananta@chromium.org>
Date: Fri Sep 01 18:21:14 2017

Ensure that redirected subresource requests in the AppCache URLLoader can be served out of the cache.

Subresource requests handled by the AppCacheSubresourceURLFactory can be redirected. The redirected
URL could potentially be served out of the AppCache. This patch provides support for the same.

Changes as below:
1. AppCacheURLLoaderJob now maintains a weak pointer to the AppCacheSubresourceURLFactory instance. This
   is passed to the AppCacheRequestHandler::MaybeCreateSubresourceLoader function and from there to the
   job.

2. When the AppCacheURLLoaderJob receives a redirect notification via a call to URLLoaderClient::OnReceiveRedirect(),
   we remember the redirect information. When the caller invokes FollowRedirect(), we try to load the request out
   of the AppCache via a call to the AppCacheRequestHandler::MaybeCreateSubresourceLoader() function.

3. The AppCacheSubresourceURLFactory class supports weak pointers and provides a method Restart() which is called by
   the AppCacheURLLoaderJob when its FollowRedirect() method is called.

4. A new virtual method SetSubresourceFactory has been added to the WebApplicationCacheHostImpl class. 
   This is overridden by the RendererWebApplicationCacheHostImpl class and we set the AppCache
   URLLoaderFactory instance on the RenderFrameImpl instance.

5. RenderFrameImpl now passes its routing_id to the RendererWebApplicationCacheHostImpl class when it creates it
   This is used to lookup the RenderFrameImpl on which the factory is to be set.

6. A virtual function OnSetSubresourceFactory has been added to the AppCacheFrontend interface.
   The proxy in the browser sends the IPC message AppCacheMsg_SetSubresourceFactory to the renderer.
   The implementation in the renderer invokes the SetSubresourceFactory() method on the host where
   the factory is passed off to the RenderFrameImpl.

7. The AppCacheUpdate job passes the subresource factory to the proxy when it receives the APPCACHE_CACHED_EVENT
   event.

The rest of the changes are boilerplate changes.

BUG= 715632 ,  759901 
TEST=Covered by content browser test AppCacheNetworkServiceBrowserTest.VerifyRedirectedSubresourceAppCacheLoad
TBR=wfh

Change-Id: Id73749254d7c81d0c0fded65fb6185b7e913f76f
Reviewed-on: https://chromium-review.googlesource.com/633922
Commit-Queue: Ananta Iyengar <ananta@chromium.org>
Reviewed-by: Ananta Iyengar <ananta@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Michael Nordman <michaeln@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499235}
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_browsertest.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_frontend_proxy.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_frontend_proxy.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_group_unittest.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_host.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_host.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_host_unittest.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_navigation_handle_core.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_navigation_handle_core.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_request_handler_unittest.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_storage_impl_unittest.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_subresource_url_factory.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_unittest.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_update_job.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_update_job_unittest.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/child/appcache/appcache_dispatcher.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/child/appcache/appcache_dispatcher.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/child/appcache/appcache_frontend_impl.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/child/appcache/appcache_frontend_impl.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/child/appcache/web_application_cache_host_impl.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/common/appcache_interfaces.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/common/appcache_messages.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/renderer/render_frame_impl.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/renderer/renderer_webapplicationcachehost_impl.cc
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/renderer/renderer_webapplicationcachehost_impl.h
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/test/data/appcache/simple_page.manifest
[add] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/content/test/data/appcache/simple_page_redirected_resource_manifest.html
[modify] https://crrev.com/2130d27a1e177d73ce3be233fd7c25865515ff9b/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Cc: -michaeln@chromium.org ananta@chromium.org
Owner: michaeln@chromium.org
Project Member

Comment 31 by bugdroid1@chromium.org, Sep 13 2017

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

commit 2431f5f0a4750b85627be6bdde7d2ba61b8176d3
Author: Michael Nordman <michaeln@google.com>
Date: Wed Sep 13 04:58:15 2017

[AppCache + NetworkService] Support retrieval of appcached resources for navigations that redirect to new locations.

Bug:  715632 
Change-Id: I675c1ab44cf53121077f87d4beb1abf496e638a6
Reviewed-on: https://chromium-review.googlesource.com/654143
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501544}
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/appcache/appcache_job.h
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/appcache/appcache_url_loader_request.h
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/appcache/appcache_url_request_job.cc
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/appcache/appcache_url_request_job.h
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/child/web_url_loader_impl.cc
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/public/common/referrer.cc
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/public/common/referrer.h
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/content/public/test/referrer_unittest.cc
[modify] https://crrev.com/2431f5f0a4750b85627be6bdde7d2ba61b8176d3/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 32 by bugdroid1@chromium.org, Oct 24 2017

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

commit 85243a8ce7a2916d0429fe37fe9a5801c590fb13
Author: Michael Nordman <michaeln@google.com>
Date: Tue Oct 24 01:24:50 2017

[AppCache + NetworkService] class SubresoureLoader

Refactor appcache subresource loading to have a clearer division of labor between
the classes. The CL introduces a new SubresoureLoader class that is composed
of logic that was previously spread between the AppCacheSubresourceURLFactory and
AppCacheURLLoaderJob. The new SubresoureLoader juggles between an instance of a networkloader
and an appcacheloader depending on how the resource load will be handled. The AppCacheURLLoaderJob
no longer knows anything about how to conduct network resource loading.

Also modified the AppCacheRequestHandler class to be more consistent with main vs sub resource
loading in its interface. In both cases, the async LoaderCallback technique is used to Maybe
create a loader at the various interception opportunities. Also tried to make it more clear
what those interception opportunities are via naming and a distinct method for each case.

BUG= 715632 

Change-Id: I25fd1d8767b333a21d42c530c258c71729f592b3
Reviewed-on: https://chromium-review.googlesource.com/719702
Commit-Queue: Michael Nordman <michaeln@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510990}
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_job.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_job.h
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_request_handler_unittest.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_subresource_url_factory.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_subresource_url_factory.h
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_update_job.h
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_update_job_unittest.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_url_loader_job.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_url_loader_job.h
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_url_loader_request.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_url_loader_request.h
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_url_request_job.cc
[modify] https://crrev.com/85243a8ce7a2916d0429fe37fe9a5801c590fb13/content/browser/appcache/appcache_url_request_job.h

Project Member

Comment 33 by bugdroid1@chromium.org, Nov 4 2017

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

commit 8757204a797ec6194d5d965f93e875593f2b982d
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Sat Nov 04 07:50:13 2017

NetworkService: change how AppCache (and SW) provides SubresourceLoaderParams

* Allow request handlers to return a subresource loading parameter
  even when the handler itself doesn't handle the main resource loading.
* This change is necessary to handle the situation where the request
  handler (i.e. AppCache or ServiceWorker) falls back to the network
  for the main resource loading but still wants to control the
  subsequent subresource loading.

This shoudn't change the existing behavior *YET* and all the existing
tests should pass.

This doc has a bit more info:
https://docs.google.com/document/d/1dvvksKQYWEnUdOcjkyreaTOxJD9L4Gybszb5mxrSzHk/

Bug:  778898 ,  715640 ,  715632 
Change-Id: Iad645eefa81c60acb03b78aadf17706e4355af67
Reviewed-on: https://chromium-review.googlesource.com/750746
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514037}
[modify] https://crrev.com/8757204a797ec6194d5d965f93e875593f2b982d/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/8757204a797ec6194d5d965f93e875593f2b982d/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/8757204a797ec6194d5d965f93e875593f2b982d/content/browser/loader/navigation_url_loader_network_service.cc
[modify] https://crrev.com/8757204a797ec6194d5d965f93e875593f2b982d/content/browser/loader/url_loader_request_handler.h

Components: -Internals>Network>Service Internals>Services>Network
Apologies, applied the wrong component in bulk.
Components: Internals>Services>Storage
Setting Internals>Services>Storage to all children of issue 611938
Project Member

Comment 36 by bugdroid1@chromium.org, May 17 2018

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

commit 020d4191bdb39fb24d2e28e791aec4d8921efb24
Author: John Abd-El-Malek <jam@chromium.org>
Date: Thu May 17 00:55:28 2018

Try reenabling some AppCache layout tests with network service.

We'll enable to see if it they still flake or not.

Bug:  715632 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I2a305db1120d98b1a1bb632bb4c2e1315a2571b2
Reviewed-on: https://chromium-review.googlesource.com/1063105
Reviewed-by: Victor Costan <pwnall@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559374}
[modify] https://crrev.com/020d4191bdb39fb24d2e28e791aec4d8921efb24/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 37 by dxie@chromium.org, May 22 2018

Labels: Proj-Servicification-Canary OS-Chrome OS-Linux OS-Mac OS-Windows
Owner: cmumford@chromium.org
Status: Assigned (was: Started)
chris, can you help looking at this?
Is there any guidance on what is left to be done? The tests that jam@ re-enabled in #c36 are not flakey.
I suspect that shared worker + appcache integration isn't really done, but I'm not sure how much that matters. It appears we don't really have test coverage. I made some comments to https://docs.google.com/document/d/1pPSOZZlc29r-WBUWIPFqsXf5kjPYNnToUCbHfKzWVIc/edit a while back.
Thx falken@. It appears as though external/wpt/html/browsers/offline/appcache/workers/appcache-worker.https.html is the only remaining AppCache test disabled in src/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService. As you suggest the Worker tests pass, but the SharedWorker tests (in that file) are failing.
Great to hear we have tests!

I can help with Shared Worker support, since I did this for Shared Worker with service worker under NetworkService. It might need some medium-sized architecture changes.

Basically I suspect Shared Worker startup needs to act like NavigationURLLoaderImpl in terms of making an "interceptor" for AppCache and sending the renderer SubresourceLoaderParams. Right now we do this for service worker in SharedWorkerScriptLoader::Start. We need to add similar hooks for AppCache.

We don't send a complete SubresourceLoaderParams currently. We send the factory bundle via mojom::SharedWorkerFactory::CreateSharedWorker. Unfortunately, I think this is too early in the process:
[B] -> [R]: sends CreateSharedWorker with factory bundle
[R] -> [B]: makes request to URLLoaderFactory, goes to SharedWorkerScriptLoaderFactory
[B] -> [R]: browser runs SharedWorkerScriptLoader::Start which possibly assigns service worker controller, sends the response.

The ServiceWorkerController is sent via existing Mojo messages (ServiceWorkerContainer.SetController). The factory bundle + ServiceWorkerController is all service worker needs to work.

For AppCache, we need to make the factory bundle with AppCache as the default factory, and send it after AppCache decides to intercept the request in SharedWorkerScriptLoader.

I think we basically need to add the equivalent of CommitNavigation for CommitSharedWorker, and give it a SubresoruceLoaderParams. So the flow would be more like:
[B] -> [R]: sends CreateSharedWorker
[R] -> [B]: makes request to URLLoaderFactory, goes to SharedWorkerScriptLoaderFactory
[B] -> [R]: browser runs SharedWorkerScriptLoader::Start which possibly assigns service worker controller or AppCache, sends the response. Makes a factory bundle as appropriate. Sends CommitSharedWorker with the controller + factory bundle. Renderer has to wait for both the URLRequest response and CommitSharedWorker (?) before proceeding.

Sorry this is a bit handwavy. WDYT?
Components: Blink>Storage>AppCache
I think... we can also start ScriptLoader earlier in the browser process and get it resolved before sending the CreeSharedWorker with the detached loader client end points and right subresource loader factories, like PlzNavigate does. ?
Also chatted with falken@. I tried to write a rough sketch of how this could possibly work (while I haven't looked into every detail).  The gist of change is that we can probably just start loading scripts in the browser process rather than wait for the renderer to ask (a la PlzNavigate, maybe we can call this PlzWorkerLoad :)).

Anyways this is the sketch: 
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit#

Hope this helps.
Thanks for the doc!! We should totally call this something like PlzWorker.
Blockedon: 858975
Owner: kinuko@chromium.org
kinuko@, can you help assign this bug?
Owner: nhiroki@chromium.org
Hiroki is taking a look into the SharedWorker + AppCache (and SW) fix, thanks Hiroki!
Status: Started (was: Assigned)
WIP CL: https://chromium-review.googlesource.com/c/chromium/src/+/1132837
Project Member

Comment 50 by bugdroid1@chromium.org, Jul 23

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

commit d712272227a19dffdf1f97d46d6c2522ff246085
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Mon Jul 23 07:20:22 2018

PlzWorker: Pass scoped_refptr<ChromeAppCacheService> to SharedWorkerServiceImpl

This is a preparation CL for PlzWorker for shared workers.

SharedWorkerServiceImpl needs to access ChromeAppCacheService to create a
NavigationLoaderInterceptor for AppCache. Subsequent CLs will actually use it.

DesignDoc:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Change-Id: Ib2e8e8d2ae9adb2bc4851fb985eb121dc4ad2917
Reviewed-on: https://chromium-review.googlesource.com/1146405
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577117}
[modify] https://crrev.com/d712272227a19dffdf1f97d46d6c2522ff246085/content/browser/shared_worker/shared_worker_host_unittest.cc
[modify] https://crrev.com/d712272227a19dffdf1f97d46d6c2522ff246085/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/d712272227a19dffdf1f97d46d6c2522ff246085/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/d712272227a19dffdf1f97d46d6c2522ff246085/content/browser/storage_partition_impl.cc

Project Member

Comment 51 by bugdroid1@chromium.org, Jul 26

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

commit ff43e447c74327fdf6fb59ae214bf2ab7124469a
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Jul 26 07:06:46 2018

PlzWorker: Create an AppCache interceptor for shared workers

This is one of CLs for PlzWorker for shared workers. In this CL, responses
served by the interceptor are actually not used yet and tests are still failing.
The subsequent CLs will wire up the interceptor up to resource loaders.

DesignDoc:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Change-Id: Iba11b4c0bd598d8ca3c445196f21855c46b1069e
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1146417
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578225}
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/appcache/appcache_request_handler.h
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/loader/navigation_url_loader_impl.cc
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/shared_worker/shared_worker_host.h
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/shared_worker/shared_worker_script_loader.cc
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/shared_worker/shared_worker_script_loader.h
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/shared_worker/shared_worker_script_loader_factory.cc
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/shared_worker/shared_worker_script_loader_factory.h
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/ff43e447c74327fdf6fb59ae214bf2ab7124469a/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 52 by bugdroid1@chromium.org, Jul 27

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

commit c00da2bed66a00aecd8610c6e6ef98992f7f1570
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Fri Jul 27 06:09:21 2018

PlzWorker: Teach a precreated AppCacheHost's ID to SharedWorkerWebApplicationCacheHostImpl

On PlzWorker, AppCacheHost is created before starting worker script loading from
a renderer process. This CL teaches the precreated AppCacheHost's ID from the
browser process to the renderer process in order to associate worker script
loading with the host.

This fixes DCHECK failures on appcache-worker.https.html caused by host
mismatch, the test still times out though.

DesignDoc:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I16c17447bbb0f4b154c5a7e9393afdea2d0e29d6
Reviewed-on: https://chromium-review.googlesource.com/1150941
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578547}
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/browser/shared_worker/mock_shared_worker.cc
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/browser/shared_worker/mock_shared_worker.h
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/common/shared_worker/shared_worker_factory.mojom
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/renderer/shared_worker/embedded_shared_worker_stub.cc
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/renderer/shared_worker/embedded_shared_worker_stub.h
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/renderer/shared_worker/shared_worker_factory_impl.cc
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/content/renderer/shared_worker/shared_worker_factory_impl.h
[modify] https://crrev.com/c00da2bed66a00aecd8610c6e6ef98992f7f1570/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 53 by bugdroid1@chromium.org, Jul 27

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

commit 66add30d798d2966544d688408f36cf9ef883437
Author: Yoichi Osato <yoichio@chromium.org>
Date: Fri Jul 27 06:21:10 2018

Revert "PlzWorker: Teach a precreated AppCacheHost's ID to SharedWorkerWebApplicationCacheHostImpl"

This reverts commit c00da2bed66a00aecd8610c6e6ef98992f7f1570.

Reason for revert: Compile failed on linux x64
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20x64/70577

Original change's description:
> PlzWorker: Teach a precreated AppCacheHost's ID to SharedWorkerWebApplicationCacheHostImpl
> 
> On PlzWorker, AppCacheHost is created before starting worker script loading from
> a renderer process. This CL teaches the precreated AppCacheHost's ID from the
> browser process to the renderer process in order to associate worker script
> loading with the host.
> 
> This fixes DCHECK failures on appcache-worker.https.html caused by host
> mismatch, the test still times out though.
> 
> DesignDoc:
> https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing
> 
> Bug:  715632 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I16c17447bbb0f4b154c5a7e9393afdea2d0e29d6
> Reviewed-on: https://chromium-review.googlesource.com/1150941
> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578547}

TBR=falken@chromium.org,kinuko@chromium.org,nhiroki@chromium.org

Change-Id: I2911bb7adaf8ac6be3dde96e5908a0e80e7ebc2e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  715632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1152590
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578548}
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/browser/shared_worker/mock_shared_worker.cc
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/browser/shared_worker/mock_shared_worker.h
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/common/shared_worker/shared_worker_factory.mojom
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/renderer/shared_worker/embedded_shared_worker_stub.cc
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/renderer/shared_worker/embedded_shared_worker_stub.h
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/renderer/shared_worker/shared_worker_factory_impl.cc
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/content/renderer/shared_worker/shared_worker_factory_impl.h
[modify] https://crrev.com/66add30d798d2966544d688408f36cf9ef883437/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 54 by bugdroid1@chromium.org, Jul 27

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

commit 7202790f6ee81e928f73cca4aac24682785ba042
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Fri Jul 27 07:08:56 2018

Reland "PlzWorker: Teach a precreated AppCacheHost's ID to SharedWorkerWebApplicationCacheHostImpl"

This reverts commit 66add30d798d2966544d688408f36cf9ef883437.

Reason for revert:
Looks like the build failure was caused by the flaky build system, not the original CL:
https://bugs.chromium.org/p/chromium/issues/detail?id=853069&desc=2

Original change's description:
> Revert "PlzWorker: Teach a precreated AppCacheHost's ID to SharedWorkerWebApplicationCacheHostImpl"
> 
> This reverts commit c00da2bed66a00aecd8610c6e6ef98992f7f1570.
> 
> Reason for revert: Compile failed on linux x64
> https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20x64/70577
> 
> Original change's description:
> > PlzWorker: Teach a precreated AppCacheHost's ID to SharedWorkerWebApplicationCacheHostImpl
> > 
> > On PlzWorker, AppCacheHost is created before starting worker script loading from
> > a renderer process. This CL teaches the precreated AppCacheHost's ID from the
> > browser process to the renderer process in order to associate worker script
> > loading with the host.
> > 
> > This fixes DCHECK failures on appcache-worker.https.html caused by host
> > mismatch, the test still times out though.
> > 
> > DesignDoc:
> > https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing
> > 
> > Bug:  715632 
> > Cq-Include-Trybots: luci.chromium.try:linux_mojo
> > Change-Id: I16c17447bbb0f4b154c5a7e9393afdea2d0e29d6
> > Reviewed-on: https://chromium-review.googlesource.com/1150941
> > Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
> > Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> > Reviewed-by: Matt Falkenhagen <falken@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#578547}
> 
> TBR=falken@chromium.org,kinuko@chromium.org,nhiroki@chromium.org
> 
> Change-Id: I2911bb7adaf8ac6be3dde96e5908a0e80e7ebc2e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  715632 
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Reviewed-on: https://chromium-review.googlesource.com/1152590
> Reviewed-by: Yoichi Osato <yoichio@chromium.org>
> Commit-Queue: Yoichi Osato <yoichio@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578548}

TBR=falken@chromium.org,kinuko@chromium.org,nhiroki@chromium.org,yoichio@chromium.org

Change-Id: I5921c0865fe6b4a3c216061c33c39002e293f69f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  715632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1152591
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578555}
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/browser/shared_worker/mock_shared_worker.cc
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/browser/shared_worker/mock_shared_worker.h
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/common/shared_worker/shared_worker_factory.mojom
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/renderer/shared_worker/embedded_shared_worker_stub.cc
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/renderer/shared_worker/embedded_shared_worker_stub.h
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/renderer/shared_worker/shared_worker_factory_impl.cc
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/content/renderer/shared_worker/shared_worker_factory_impl.h
[modify] https://crrev.com/7202790f6ee81e928f73cca4aac24682785ba042/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Labels: -Proj-Servicification-Canary Proj-Servicification
Labels: Hotlist-KnownIssue
Project Member

Comment 57 by bugdroid1@chromium.org, Aug 29

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

commit 74946487b66775f2ce6416736eadaf6056c2d6ca
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Aug 29 10:55:27 2018

SharedWorker: Give more specific names to function arguments for cleanup

For a while, we need to keep 3 shared worker script loading paths: classic,
S13nServiceWorker, and NetworkService. To clarify which function arguments are
used for which loading path, we should give more specific names to them.

Bug:  715632 
Change-Id: Ic566b8bc8d79424d1ac497048455cfb0b0fcbee2
Reviewed-on: https://chromium-review.googlesource.com/1194807
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587069}
[modify] https://crrev.com/74946487b66775f2ce6416736eadaf6056c2d6ca/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/74946487b66775f2ce6416736eadaf6056c2d6ca/content/browser/shared_worker/shared_worker_host.h
[modify] https://crrev.com/74946487b66775f2ce6416736eadaf6056c2d6ca/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/74946487b66775f2ce6416736eadaf6056c2d6ca/content/browser/shared_worker/shared_worker_service_impl.h

Project Member

Comment 58 by bugdroid1@chromium.org, Aug 30

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

commit 08c3c6d448315329c97a9bd07508427400b8cebd
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Aug 30 08:53:24 2018

SharedWorker: Consolidate default loader factory creation for cleanup

Default loader factories for shared workers have been created in different
places depending on the configurations of S13nServiceWorker and NetworkService.
This has been impaired code readability.

To improve it, this CL consolidates default loader factory creation.

Bug:  715632 
Change-Id: Ic06cd6ab4d9f0c3ab40b45f8f8c94d26bf5b7b15
Reviewed-on: https://chromium-review.googlesource.com/1195171
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587489}
[modify] https://crrev.com/08c3c6d448315329c97a9bd07508427400b8cebd/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/08c3c6d448315329c97a9bd07508427400b8cebd/content/browser/shared_worker/shared_worker_host_unittest.cc
[modify] https://crrev.com/08c3c6d448315329c97a9bd07508427400b8cebd/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/08c3c6d448315329c97a9bd07508427400b8cebd/content/browser/shared_worker/shared_worker_service_impl.h

Project Member

Comment 59 by bugdroid1@chromium.org, Aug 31

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

commit 9b94b57844cccdddfb480a44e38f1f48274fc8af
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Fri Aug 31 05:04:57 2018

SharedWorker: Give more specific names to function arguments for cleanup (2)

This is a follow-up CL for
https://chromium-review.googlesource.com/c/chromium/src/+/1194807

For a while, we need to keep 3 shared worker script loading paths: classic,
S13nServiceWorker, and NetworkService. To clarify which function arguments are
used for which loading path, we should give more specific names to them.

Bug:  715632 
Change-Id: I5c075cd72f89d7ca761861db02be0b9146cc215e
TBR: kinuko@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/1198663
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587947}
[modify] https://crrev.com/9b94b57844cccdddfb480a44e38f1f48274fc8af/content/browser/shared_worker/mock_shared_worker.cc
[modify] https://crrev.com/9b94b57844cccdddfb480a44e38f1f48274fc8af/content/browser/shared_worker/mock_shared_worker.h
[modify] https://crrev.com/9b94b57844cccdddfb480a44e38f1f48274fc8af/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/9b94b57844cccdddfb480a44e38f1f48274fc8af/content/common/shared_worker/shared_worker_factory.mojom
[modify] https://crrev.com/9b94b57844cccdddfb480a44e38f1f48274fc8af/content/renderer/shared_worker/embedded_shared_worker_stub.cc
[modify] https://crrev.com/9b94b57844cccdddfb480a44e38f1f48274fc8af/content/renderer/shared_worker/embedded_shared_worker_stub.h

Blocking: 680046
Project Member

Comment 61 by bugdroid1@chromium.org, Sep 4

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

commit a29d903d84c2041e806fa67fd35a4fbe8d31813f
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Sep 04 12:44:53 2018

PlzWorker: Initiate top-level worker script loading for Shared Workers from the browser process

This CL adds SharedWorkerScriptFetcher that initiates a top-level worker script
loading for shared workers from the browser process. Once the fetcher receives a
response head, that unbounds the URL loader and its client, and passes them to
the renderer process with the StartWorker message. In the renderer process,
WorkerClassicScriptLoader in Blink makes a resource request as usual, and
ResourceDispatcher in Content serves it using the passed URLLoader etc.

In addition, this CL makes Shared Workers run with AppCache on NetworkService,
AppCache's fallback case is not implemented yet though. Subsequent CLs will
implement the case.

See the design doc for more details:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Change-Id: I7aafd2224dec42a89a274a9b1820ae9775535aca
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1154743
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588495}
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/BUILD.gn
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/mock_shared_worker.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/mock_shared_worker.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host_unittest.cc
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_fetcher.cc
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_fetcher.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader_factory.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader_factory.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/common/shared_worker/shared_worker_factory.mojom
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/loader/web_url_loader_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/service_worker/service_worker_network_provider.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/service_worker/service_worker_network_provider.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/embedded_shared_worker_stub.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/embedded_shared_worker_stub.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/shared_worker_factory_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/shared_worker_factory_impl.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/external/wpt/fetch/sec-metadata/sharedworker.tentative.https.sub-expected.txt
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/http/tests/fetch/chromium/data-saver-expected.txt
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/blink/public/mojom/BUILD.gn
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/blink/public/mojom/shared_worker/shared_worker_main_script_load_params.mojom
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/tools/traffic_annotation/summary/annotations.xml

Project Member

Comment 62 by bugdroid1@chromium.org, Sep 4

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

commit a29d903d84c2041e806fa67fd35a4fbe8d31813f
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Sep 04 12:44:53 2018

PlzWorker: Initiate top-level worker script loading for Shared Workers from the browser process

This CL adds SharedWorkerScriptFetcher that initiates a top-level worker script
loading for shared workers from the browser process. Once the fetcher receives a
response head, that unbounds the URL loader and its client, and passes them to
the renderer process with the StartWorker message. In the renderer process,
WorkerClassicScriptLoader in Blink makes a resource request as usual, and
ResourceDispatcher in Content serves it using the passed URLLoader etc.

In addition, this CL makes Shared Workers run with AppCache on NetworkService,
AppCache's fallback case is not implemented yet though. Subsequent CLs will
implement the case.

See the design doc for more details:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Change-Id: I7aafd2224dec42a89a274a9b1820ae9775535aca
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1154743
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588495}
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/BUILD.gn
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/mock_shared_worker.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/mock_shared_worker.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host_unittest.cc
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_fetcher.cc
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_fetcher.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader_factory.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader_factory.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/common/shared_worker/shared_worker_factory.mojom
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/loader/web_url_loader_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/service_worker/service_worker_network_provider.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/service_worker/service_worker_network_provider.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/embedded_shared_worker_stub.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/embedded_shared_worker_stub.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/shared_worker_factory_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/shared_worker_factory_impl.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/external/wpt/fetch/sec-metadata/sharedworker.tentative.https.sub-expected.txt
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/http/tests/fetch/chromium/data-saver-expected.txt
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/blink/public/mojom/BUILD.gn
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/blink/public/mojom/shared_worker/shared_worker_main_script_load_params.mojom
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/tools/traffic_annotation/summary/annotations.xml

Project Member

Comment 63 by bugdroid1@chromium.org, Sep 4

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

commit a29d903d84c2041e806fa67fd35a4fbe8d31813f
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Sep 04 12:44:53 2018

PlzWorker: Initiate top-level worker script loading for Shared Workers from the browser process

This CL adds SharedWorkerScriptFetcher that initiates a top-level worker script
loading for shared workers from the browser process. Once the fetcher receives a
response head, that unbounds the URL loader and its client, and passes them to
the renderer process with the StartWorker message. In the renderer process,
WorkerClassicScriptLoader in Blink makes a resource request as usual, and
ResourceDispatcher in Content serves it using the passed URLLoader etc.

In addition, this CL makes Shared Workers run with AppCache on NetworkService,
AppCache's fallback case is not implemented yet though. Subsequent CLs will
implement the case.

See the design doc for more details:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Change-Id: I7aafd2224dec42a89a274a9b1820ae9775535aca
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1154743
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588495}
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/BUILD.gn
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/mock_shared_worker.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/mock_shared_worker.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_host_unittest.cc
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_fetcher.cc
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_fetcher.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader_factory.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_script_loader_factory.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/common/shared_worker/shared_worker_factory.mojom
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/loader/resource_dispatcher.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/loader/web_url_loader_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/service_worker/service_worker_network_provider.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/service_worker/service_worker_network_provider.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/embedded_shared_worker_stub.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/embedded_shared_worker_stub.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/shared_worker_factory_impl.cc
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/content/renderer/shared_worker/shared_worker_factory_impl.h
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/external/wpt/fetch/sec-metadata/sharedworker.tentative.https.sub-expected.txt
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/http/tests/fetch/chromium/data-saver-expected.txt
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/blink/public/mojom/BUILD.gn
[add] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/third_party/blink/public/mojom/shared_worker/shared_worker_main_script_load_params.mojom
[modify] https://crrev.com/a29d903d84c2041e806fa67fd35a4fbe8d31813f/tools/traffic_annotation/summary/annotations.xml

Project Member

Comment 64 by bugdroid1@chromium.org, Sep 6

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

commit a294826b7179bf14015e5f2be285d021af1299ad
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Sep 06 09:54:06 2018

PlzWorker: Increment the reference count for the process keep alive early

This CL makes SharedWorkerServiceImpl increment the reference count for the
renderer process keep alive before asynchronously creating a script loader.

Before this CL, the reference count is incremented after the script loader is
created. This crashes a test that forcibly terminates the shared worker during
loader creation because the termination tries to decrement the refcount and hits
the DCHECK to make sure the refcount is non-zero.

Bug:  715632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I322a5da1215da67e027005afb0544c3b95090113
Reviewed-on: https://chromium-review.googlesource.com/1205976
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589122}
[modify] https://crrev.com/a294826b7179bf14015e5f2be285d021af1299ad/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/a294826b7179bf14015e5f2be285d021af1299ad/content/browser/shared_worker/shared_worker_host_unittest.cc
[modify] https://crrev.com/a294826b7179bf14015e5f2be285d021af1299ad/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/a294826b7179bf14015e5f2be285d021af1299ad/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/a294826b7179bf14015e5f2be285d021af1299ad/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 65 by bugdroid1@chromium.org, Sep 6

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

commit 84e2037b6180c6cd20081efbe29ba8c94eee0a4e
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Sep 06 13:23:48 2018

[jumbo] Unduplicate duplicated IsShuttingDown

Quick fix to unbreak jumbo builds. In
https://chromium-review.googlesource.com/c/chromium/src/+/1205976
a copy of IsShuttingDown was added and this patch changes the code
to instead use the original version.

TBR=kinuko@chromium.org,nhiroki@chromium.org

Bug:  715632 
Change-Id: I2451fcaa009ec6403827cdb05cd3b47112c4681b
Reviewed-on: https://chromium-review.googlesource.com/1209703
Reviewed-by: Daniel Bratell <bratell@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#589148}
[modify] https://crrev.com/84e2037b6180c6cd20081efbe29ba8c94eee0a4e/content/browser/shared_worker/shared_worker_host.cc
[modify] https://crrev.com/84e2037b6180c6cd20081efbe29ba8c94eee0a4e/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/84e2037b6180c6cd20081efbe29ba8c94eee0a4e/content/browser/shared_worker/shared_worker_service_impl.h

Project Member

Comment 66 by bugdroid1@chromium.org, Sep 10

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

commit 4787ce4345024c8258ee281c94b4b9253fdd30bc
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Mon Sep 10 06:46:24 2018

PlzWorker: Add the Save-Data header to browser-initiated requests for shared worker scripts

This CL adds the Save-Data header to browser-initiated requests for shared
worker's main script. This has been tested by LayoutTests, but it's no longer
available after NetworkService is enabled because LayoutTest's internal API to
enable the data saver feature doesn't take effect on the preferences in the
browser process. Therefore, this CL adds browser tests instead.

Design doc of the PlzWorker:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I1390205530742dba73057f6a866af8db158afe54
Reviewed-on: https://chromium-review.googlesource.com/1209004
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589839}
[modify] https://crrev.com/4787ce4345024c8258ee281c94b4b9253fdd30bc/chrome/browser/data_saver/data_saver_browsertest.cc
[modify] https://crrev.com/4787ce4345024c8258ee281c94b4b9253fdd30bc/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/4787ce4345024c8258ee281c94b4b9253fdd30bc/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/4787ce4345024c8258ee281c94b4b9253fdd30bc/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
[modify] https://crrev.com/4787ce4345024c8258ee281c94b4b9253fdd30bc/third_party/WebKit/LayoutTests/http/tests/fetch/chromium/data-saver.html
[delete] https://crrev.com/a4de8da116585357c123d71e5f54d1103824c6df/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/http/tests/fetch/chromium/data-saver-expected.txt

Project Member

Comment 67 by bugdroid1@chromium.org, Sep 13

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

commit 2a243e5727173f8ef92ac80282ece9fc12ec16a0
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Sep 13 11:20:09 2018

PlzWorker: Handle failures on browser-initiated script loading for shared workers

This CL handles a failure on shared worker's main script loading initiated from
the browser process (PlzWorker). When the failure happens, shared worker startup
in the renderer process fails and dispatches an ErrorEvent on the SharedWorker
JS object.

Before this CL, such failures are just ignored and a layout test times out.

Design doc of the PlzWorker:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I9168ad2fadcf27697d6e07489716b0e92a5b17c0
Reviewed-on: https://chromium-review.googlesource.com/1214906
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590970}
[modify] https://crrev.com/2a243e5727173f8ef92ac80282ece9fc12ec16a0/content/browser/shared_worker/shared_worker_script_fetcher.cc
[modify] https://crrev.com/2a243e5727173f8ef92ac80282ece9fc12ec16a0/content/browser/shared_worker/shared_worker_script_fetcher.h
[modify] https://crrev.com/2a243e5727173f8ef92ac80282ece9fc12ec16a0/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/2a243e5727173f8ef92ac80282ece9fc12ec16a0/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/2a243e5727173f8ef92ac80282ece9fc12ec16a0/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 68 by bugdroid1@chromium.org, Sep 25

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

commit 50eb41e90b6e917f2db2ead4a1a32e45414db091
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Tue Sep 25 06:02:14 2018

PlzWorker: Add the Sec-Metadata header to browser-initiated requests for shared worker scripts

This CL adss the Sec-Metadata header to browser-initiated requests for shared
worker's main script. Note that the browser-initiated request (a.k.a PlzWorker)
is enabled only when the NetworkService is enabled.

Design doc of the PlzWorker:
https://docs.google.com/document/d/1Jtn33bvqkqWxq6K7HIA4uU6HLWPTmOD7vFviacfTmhM/edit?usp=sharing

Bug:  715632 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I6ecc10122bdcf36aa4a0b8d7daddb0b3475e1dff
Reviewed-on: https://chromium-review.googlesource.com/1212247
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593832}
[modify] https://crrev.com/50eb41e90b6e917f2db2ead4a1a32e45414db091/content/browser/shared_worker/shared_worker_service_impl.cc
[modify] https://crrev.com/50eb41e90b6e917f2db2ead4a1a32e45414db091/content/browser/shared_worker/shared_worker_service_impl.h
[modify] https://crrev.com/50eb41e90b6e917f2db2ead4a1a32e45414db091/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
[delete] https://crrev.com/6a4948b18880d9e70a9b4746e3cb62c1cc6a8154/third_party/WebKit/LayoutTests/virtual/outofblink-cors-ns/external/wpt/fetch/sec-metadata/sharedworker.tentative.https.sub-expected.txt

Project Member

Comment 69 by bugdroid1@chromium.org, Oct 3

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

commit da9aac88a80c910036d941c26ea5188eb68324fb
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Oct 03 04:23:41 2018

PlzWorker: Support AppCache's fallback case for shared workers

This CL makes SharedWorkerScriptFetcher ask network interceptors if they want
to handle a response served by the default network loader. This is necessary for
supporting AppCache's fallback.

A possible confusing point of this CL is
SharedWorkerScriptFetcher::OnReceiveResponse() can be called twice. First, it's
called when the interceptor or the default network loader serves the shared
worker's main script. Then, it's called again when the interceptor wants to
serve an alternative response for the response served by the default network
loader.

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I93b45fa1f2e42422c0e85d3a67471c891dc5992a
Bug:  715632 
Reviewed-on: https://chromium-review.googlesource.com/c/1224611
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596122}
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/content/browser/appcache/appcache_request_handler.cc
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/content/browser/shared_worker/shared_worker_script_fetcher.cc
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/content/browser/shared_worker/shared_worker_script_fetcher.h
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/content/browser/shared_worker/shared_worker_script_loader.cc
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/content/browser/shared_worker/shared_worker_script_loader.h
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/content/browser/shared_worker/shared_worker_script_loader_factory.cc
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/content/browser/shared_worker/shared_worker_script_loader_factory.h
[modify] https://crrev.com/da9aac88a80c910036d941c26ea5188eb68324fb/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 70 by bugdroid1@chromium.org, Oct 4

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

commit beae6e4c704d6fcd515260edc5fac173f825549c
Author: Matt Falkenhagen <falken@chromium.org>
Date: Thu Oct 04 22:34:11 2018

shared worker: Fix some code around default factory.

Shared worker's factory bundle's default factory can now be AppCache
rather than network, since support for shared worker + AppCache was
added in  issue 715632 .

Use CloneWithoutDefaultFactory() to make the service worker fallback
factory accordingly, as is done for frames in
RenderFrameImpl::BuildServiceWorkerNetworkProviderForNavigation and
dedicated workers in RenderFrameImpl::CreateWorkerFetchContext.

No test added since we don't have a feature that can be set as the
default factory and be used in conjunction with service worker. The
point of the change is to fix the comments and make it consistent with
the other callsites.

Bug:  715632 
Change-Id: Ie1ee1cae5423fd81933dea599e6d724a12bf51f7
Reviewed-on: https://chromium-review.googlesource.com/c/1260883
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596890}
[modify] https://crrev.com/beae6e4c704d6fcd515260edc5fac173f825549c/content/renderer/shared_worker/embedded_shared_worker_stub.cc

nhiroki: can this be marked fixed now?
I finished SharedWorkers and AppCache integration on the network service in M71. I'm not really sure there're remaining issues on AppCache on the network service.

kinuko@: Do you know remaining issues? At least, it looks like there're no failing tests:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/FlagExpectations/enable-features%3DNetworkService?q=enable-features%3DNetworkService&sq=package:chromium&dr
Status: Fixed (was: Started)
I can't really recall off-hand. I'd like to just close this and see if anything else's still broken from the wild.
Labels: M-71

Comment 75 by falken@chromium.org, Yesterday (42 hours ago)

Description: Show this description

Comment 76 by falken@chromium.org, Yesterday (42 hours ago)

Summary: Appcache shouldn't hook into the network layer (and also implement PlzWorker for shared workers) (was: Appcache shouldn't hook into the network layer)

Sign in to add a comment