NetS13nServiceWorker: delay SW update until after page load |
|||||||||||
Issue descriptionCurrently we are controlling soft update timing by SWControlleeRequestHandler, but after NetS13nServiceWorker SWControlleeRequestHandler is created only for navigation request. That would have some effect to performance.
,
Jun 25 2018
It'd be nice to do this in a way that's more generic and doesn't need special code to observe resource requests. 1. Can the browser process in //content know when a page load reached a milestone like FMP? 2. Could the service worker just report when its idle? (i.e,. reuse RequestTermination) The problem is it might never get updated, if that's too lenient. BTW: I think Firefox uses DOMContentLoaded to trigger update.
,
Jun 27 2018
,
Jun 27 2018
,
Jun 27 2018
Thinking more about this... The cases to consider are: 1. Normal service worker. 2. No-fetch service worker. 3. Page load failed. Ideally we can trigger update for all 3 cases at a time when the page is not busy. Approaches: - Network quiescence: It might not work well for a normal service worker getting things from CacheStorage - Fetch event (or any event in the SW) quiescence : It might not work well for a no-fetch service worker. Ideally there's somewhere in Blink or //content where we can just say "page is not busy now". Maybe something like main thread activity quiescence? But I guess that may never happen. Maybe some heuristic mixed with a hard timeout would work?
,
Jun 27 2018
Also need to consider: 4. Shared worker clients.
,
Jun 27 2018
I guess we can make ServiceWorkerProviderContext aware of how many requests are being made, and trigger update after request quiescence. That would work for no-fetch service workers too. It has to be duplicated for shared worker + pages as SWNetworkProvider would be the one talking to SWPC. For handling page load failure, it'd probably have to be a point in SWControlleeRequestHandler (or ServiceWorkerNavigationLoader and SharedWorkerScriptLoader). Another case is: 5. Redirects during main resource request. I think we don't need to worry too much about that. It might be able to triggered similarly to page load failure.
,
Jun 27 2018
(As I wrote in the chat) Network quiescence is detected in Blink and count requests that hit CacheStorage too. It's detected by idleness_detector and this can be relatively easily ported to worker cases too (but not sure if requests from workers matter too much) If something like DOMContentLoaded (which fires relatively early btw) can be okay just using DidFinishLoad/DidFailLoad might be an option too, it's notified from blink to content::WebContents (can be observable via WebContentsObserver)
,
Jul 3
I turned my attention to some other things (crashers, regressions). Resetting this to available.
,
Jul 3
,
Jul 18
I'm picking this up again. Seems ideal if we can use the network quiescence detector built into Blink, and also handle cases 3-5 mentioned above.
,
Jul 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/34989fad45b307c82280c6941e3b4a5819a5517b commit 34989fad45b307c82280c6941e3b4a5819a5517b Author: Matt Falkenhagen <falken@chromium.org> Date: Fri Jul 20 04:07:27 2018 NetS13nServiceWorker: Delay soft update until network quiescence after a navigation. This more closely matches the legacy code path. We wait for network quiet as an optimization, as the update may adversely impact page load performance. The design is: - During the main resource request, mark the provider host as needing to update its controller service worker. - After navigation, the renderer sends the provider host a HintToUpdateServiceWorker IPC on network quiet. When the host receives this message, it schedules an update task on the relevant ServiceWorkerVersion, which triggers after 1 second. - If the HintToUpdateServiceWorker IPC was never received and the host is being destroyed while marked as needing to update, it schedules the update task then on the ServiceWorkerVersion as above. This CL handles the major case. Future work can be to expand this to shared workers and intermediate service workers in the redirect chain. A unit test is added. There is also an existing WPT: update-after-navigation-fetch-event.https.html See also design doc: https://docs.google.com/document/d/1f0oVW9d04qTLRX9e0qJ9gQW6bQIgN-_fiAFgmn6UcpI/edit?usp=sharing Bug: 853635 Change-Id: I7196d4e8b84312504851bf04defb3d8cfae291c6 Reviewed-on: https://chromium-review.googlesource.com/1143061 Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#576773} [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/browser/service_worker/embedded_worker_instance_unittest.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/browser/service_worker/service_worker_controllee_request_handler.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/browser/service_worker/service_worker_controllee_request_handler.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/browser/service_worker/service_worker_provider_host.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/browser/service_worker/service_worker_provider_host.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/browser/service_worker/service_worker_provider_host_unittest.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/browser/service_worker/service_worker_version.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/common/service_worker/service_worker_container.mojom [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/renderer/service_worker/service_worker_network_provider.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/renderer/service_worker/service_worker_network_provider.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/renderer/service_worker/service_worker_provider_context.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/renderer/service_worker/service_worker_provider_context.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/renderer/service_worker/service_worker_provider_context_unittest.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/third_party/blink/public/platform/modules/service_worker/web_service_worker_network_provider.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/third_party/blink/renderer/core/loader/frame_fetch_context.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/third_party/blink/renderer/core/loader/frame_fetch_context.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/third_party/blink/renderer/platform/loader/fetch/fetch_context.h [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc [modify] https://crrev.com/34989fad45b307c82280c6941e3b4a5819a5517b/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
,
Jul 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7a2c319459f35920f9d30941b7100c37fe414411 commit 7a2c319459f35920f9d30941b7100c37fe414411 Author: Matt Falkenhagen <falken@chromium.org> Date: Mon Jul 23 06:23:08 2018 S13nServiceWorker: Refine scheduling updates after navigation. (1) Account for multiple main resource requests at the same time, by adding a ref count of expected hints. (2) Also update the intermediate service workers in redirect chains. This both conforms to the spec and is needed to implement (1) sensibly, since we don't know which service workers are intermediate and which are final when incrementing the ref count. Bug: 853635 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: If4212e12c9e2084c8f1b34c76e80b094053a207f Reviewed-on: https://chromium-review.googlesource.com/1144905 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#577113} [modify] https://crrev.com/7a2c319459f35920f9d30941b7100c37fe414411/content/browser/service_worker/service_worker_controllee_request_handler.cc [modify] https://crrev.com/7a2c319459f35920f9d30941b7100c37fe414411/content/browser/service_worker/service_worker_provider_host.cc [modify] https://crrev.com/7a2c319459f35920f9d30941b7100c37fe414411/content/browser/service_worker/service_worker_provider_host.h [modify] https://crrev.com/7a2c319459f35920f9d30941b7100c37fe414411/content/browser/service_worker/service_worker_provider_host_unittest.cc [modify] https://crrev.com/7a2c319459f35920f9d30941b7100c37fe414411/content/browser/service_worker/service_worker_version.cc [modify] https://crrev.com/7a2c319459f35920f9d30941b7100c37fe414411/content/browser/service_worker/service_worker_version.h [modify] https://crrev.com/7a2c319459f35920f9d30941b7100c37fe414411/third_party/WebKit/LayoutTests/TestExpectations
,
Jul 30
Request merge to 69. These patches are part of the ServiceWorkerServicification experiment which we aim to start in 69 Beta soon. c#12 and c#13 look healthy on Canary and these are expected to have impact on performance, so I'd like to get them in 69.
,
Jul 31
Your change meets the bar and is auto-approved for M69. Please go ahead and merge the CL to branch 3497 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68dcf00c3fe939888efa8365c4b9a2239f494848 commit 68dcf00c3fe939888efa8365c4b9a2239f494848 Author: Matt Falkenhagen <falken@chromium.org> Date: Tue Jul 31 02:16:05 2018 M69: NetS13nServiceWorker: Delay soft update until network quiescence after a navigation. This more closely matches the legacy code path. We wait for network quiet as an optimization, as the update may adversely impact page load performance. The design is: - During the main resource request, mark the provider host as needing to update its controller service worker. - After navigation, the renderer sends the provider host a HintToUpdateServiceWorker IPC on network quiet. When the host receives this message, it schedules an update task on the relevant ServiceWorkerVersion, which triggers after 1 second. - If the HintToUpdateServiceWorker IPC was never received and the host is being destroyed while marked as needing to update, it schedules the update task then on the ServiceWorkerVersion as above. This CL handles the major case. Future work can be to expand this to shared workers and intermediate service workers in the redirect chain. A unit test is added. There is also an existing WPT: update-after-navigation-fetch-event.https.html See also design doc: https://docs.google.com/document/d/1f0oVW9d04qTLRX9e0qJ9gQW6bQIgN-_fiAFgmn6UcpI/edit?usp=sharing Bug: 853635 Change-Id: I7196d4e8b84312504851bf04defb3d8cfae291c6 Reviewed-on: https://chromium-review.googlesource.com/1143061 Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#576773}(cherry picked from commit 34989fad45b307c82280c6941e3b4a5819a5517b) Reviewed-on: https://chromium-review.googlesource.com/1156084 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#259} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/browser/service_worker/embedded_worker_instance_unittest.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/browser/service_worker/service_worker_controllee_request_handler.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/browser/service_worker/service_worker_controllee_request_handler.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/browser/service_worker/service_worker_provider_host.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/browser/service_worker/service_worker_provider_host.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/browser/service_worker/service_worker_provider_host_unittest.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/browser/service_worker/service_worker_version.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/common/service_worker/service_worker_container.mojom [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/renderer/service_worker/service_worker_network_provider.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/renderer/service_worker/service_worker_network_provider.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/renderer/service_worker/service_worker_provider_context.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/renderer/service_worker/service_worker_provider_context.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/renderer/service_worker/service_worker_provider_context_unittest.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/content/renderer/service_worker/service_worker_subresource_loader_unittest.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/third_party/blink/public/platform/modules/service_worker/web_service_worker_network_provider.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/third_party/blink/renderer/core/loader/frame_fetch_context.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/third_party/blink/renderer/core/loader/frame_fetch_context.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/third_party/blink/renderer/platform/loader/fetch/fetch_context.h [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc [modify] https://crrev.com/68dcf00c3fe939888efa8365c4b9a2239f494848/third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h
,
Jul 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/06c2c028339f831abe1a5d7f6c2348248194cd76 commit 06c2c028339f831abe1a5d7f6c2348248194cd76 Author: Matt Falkenhagen <falken@chromium.org> Date: Tue Jul 31 03:22:09 2018 M69: S13nServiceWorker: Refine scheduling updates after navigation. (1) Account for multiple main resource requests at the same time, by adding a ref count of expected hints. (2) Also update the intermediate service workers in redirect chains. This both conforms to the spec and is needed to implement (1) sensibly, since we don't know which service workers are intermediate and which are final when incrementing the ref count. TBR=falken@chromium.org (cherry picked from commit 7a2c319459f35920f9d30941b7100c37fe414411) Bug: 853635 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: If4212e12c9e2084c8f1b34c76e80b094053a207f Reviewed-on: https://chromium-review.googlesource.com/1144905 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#577113} Reviewed-on: https://chromium-review.googlesource.com/1156104 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/branch-heads/3497@{#260} Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753} [modify] https://crrev.com/06c2c028339f831abe1a5d7f6c2348248194cd76/content/browser/service_worker/service_worker_controllee_request_handler.cc [modify] https://crrev.com/06c2c028339f831abe1a5d7f6c2348248194cd76/content/browser/service_worker/service_worker_provider_host.cc [modify] https://crrev.com/06c2c028339f831abe1a5d7f6c2348248194cd76/content/browser/service_worker/service_worker_provider_host.h [modify] https://crrev.com/06c2c028339f831abe1a5d7f6c2348248194cd76/content/browser/service_worker/service_worker_provider_host_unittest.cc [modify] https://crrev.com/06c2c028339f831abe1a5d7f6c2348248194cd76/content/browser/service_worker/service_worker_version.cc [modify] https://crrev.com/06c2c028339f831abe1a5d7f6c2348248194cd76/content/browser/service_worker/service_worker_version.h
,
Aug 1
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by dxie@google.com
, Jun 19 2018