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

Issue 759901 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocking:
issue 715632



Sign in to add a comment

The AppCache network service implementation needs to pass the subresource URL factory to the renderer when the cache is updated.

Project Member Reported by ananta@chromium.org, Aug 29 2017

Issue description

Currently we pass the AppCache subresource URL factory to the renderer when the
navigation is committed. This works well for the case when the AppCache for
the frame is downloaded and available. However for the case when the AppCache
for the frame is not yet available, we need to pass the subresource factory 
along with or before the AppCache update event to the renderer (RenderFrameImpl)

A number of AppCache layout tests fail due to this.

Examples below. Please note that this list is not comprehensive.
http/tests/appcache/simple.html [ Timeout ]
http/tests/appcache/top-frame-3.html [ Crash Timeout ]
http/tests/appcache/video.html [ Failure Timeout ]
http/tests/appcache/whitelist-wildcard.html [ Failure ]



 
Project Member

Comment 1 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
Owner: ananta@chromium.org
Status: Fixed (was: Available)

Comment 3 by laforge@google.com, Nov 7 2017

Components: Internals>Network>Service

Comment 4 by laforge@google.com, Nov 7 2017

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

Comment 5 by laforge@google.com, Nov 8 2017

Components: Internals>Services>Storage
Setting Internals>Services>Storage to all children of issue 611938

Sign in to add a comment