New issue
Advanced search Search tips

Issue 831998 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 715640



Sign in to add a comment

S13nSW: http/tests/serviceworker/chromium.update-served-from-cache.html is failing

Project Member Reported by bashi@chromium.org, Apr 12 2018

Issue description

I think this is failing because we haven't implemented bypassing HTTP cache in S13nSW code path yet.

Probably we need to do something similar to ServiceWorkerContextRequestHandler::MaybeCreateJobImpl() in ServiceWorkerScriptLoaderFactory or ServiceWorker{New,Installed}ScriptLoader?

https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_context_request_handler.cc?l=223&rcl=6391d19a2bbc914ea208d566e85f31420ea65e55

 

Comment 1 by bashi@chromium.org, Apr 12 2018

Blocking: 715640
Cc: falken@chromium.org kinuko@chromium.org
Labels: -Pri-3 Pri-2
falken@, kinuko@: Do you have thoughts on:
- Where should we check ServiceWorkerUpdateViaCache mode
- How to tell URLLoader to bypass cache

?

Comment 2 by kinuko@google.com, Apr 12 2018

For the latter q: you can use the same flag in resource request when you create a URLLoader.

https://cs.chromium.org/chromium/src/services/network/public/cpp/resource_request.h?dr=CSs&q=load_flags+file:resource_request&l=73
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 13 2018

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

commit 860f4fd3c055b3c0cf66e29cd19a203c8863425a
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Fri Apr 13 08:54:25 2018

S13nSW: Check updateViaCache and bypass HTTP cache if needed

ShouldBypassCacheDueToUpdateViaCache() is moved to ServiceWorkerUtils
so that we can use the function in S13nSW code path.

This fixes test failures in
- http/tests/serviceworker/chromium.update-served-from-cache.html
- external/wpt/service-workers/service-worker/registration-updateviacache.https.html

Bug:  831998 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Iff73827268f37b4fa2aa5826ff153bd92b3a3c92
Reviewed-on: https://chromium-review.googlesource.com/1011527
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550556}
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/browser/service_worker/service_worker_context_request_handler.cc
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/browser/service_worker/service_worker_new_script_loader.cc
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/browser/service_worker/service_worker_new_script_loader.h
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/common/service_worker/service_worker_utils.cc
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/common/service_worker/service_worker_utils.h
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 4 by bashi@chromium.org, Apr 16 2018

Status: Fixed (was: Started)
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/860f4fd3c055b3c0cf66e29cd19a203c8863425a

commit 860f4fd3c055b3c0cf66e29cd19a203c8863425a
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Fri Apr 13 08:54:25 2018

S13nSW: Check updateViaCache and bypass HTTP cache if needed

ShouldBypassCacheDueToUpdateViaCache() is moved to ServiceWorkerUtils
so that we can use the function in S13nSW code path.

This fixes test failures in
- http/tests/serviceworker/chromium.update-served-from-cache.html
- external/wpt/service-workers/service-worker/registration-updateviacache.https.html

Bug:  831998 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Iff73827268f37b4fa2aa5826ff153bd92b3a3c92
Reviewed-on: https://chromium-review.googlesource.com/1011527
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550556}
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/browser/service_worker/service_worker_context_request_handler.cc
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/browser/service_worker/service_worker_new_script_loader.cc
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/browser/service_worker/service_worker_new_script_loader.h
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/common/service_worker/service_worker_utils.cc
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/content/common/service_worker/service_worker_utils.h
[modify] https://crrev.com/860f4fd3c055b3c0cf66e29cd19a203c8863425a/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Sign in to add a comment