Service worker script loading UMA and improvements with NetS13nSW. |
||
Issue descriptionThe legacy path has ServiceWorker.ContextRequestHandlerStatus which is meant to both track error conditions and detect the usage of importScripts() of new scripts after installation, which we want to deprecate. However there is a flaw. When script streaming is on, we don't go through this path, so the deprecated usage is unknown. The analogue would go to ServiceWorkerScriptLoaderFactory, but I'm not sure it's really needed. The error cases are minor and we wouldn't really watch these metrics I think. I propose: - Add a note to ServiceWorker.ContextRequestHandlerStatus that it doesn't include most script loads, because those are handled by installed scripts manager (service worker script streaming). It isn't logged when S13nSW/NetworkService is on, and can be removed after that ships. - Remove the fallback to network at the top of ServiceWorkerScriptLoaderFactory::CreateLoaderAndStart. It can just fast fail. - Record BadMessage as in the TODO for CheckIfScriptRequestIsValid. This factory should only be used for scripts, we shouldn't fallback to network for others. - Add UseCounter on Blink-side to detect usage of the importScripts() path we want to deprecate. It can probably rely on service worker installed script manager totally. That's not complete since new service workers don't get an installed script manager even after installation, but should work for getting an idea of usage.
,
Jul 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6f4394143cd95271b0d65cc3d7f034359fa460cf commit 6f4394143cd95271b0d65cc3d7f034359fa460cf Author: Kenichi Ishibashi <bashi@chromium.org> Date: Thu Jul 05 05:12:41 2018 Remove network fallback in ServiceWorkerScriptLoaderFactory When a request is invalid we can just fail. Bug: 852666 Change-Id: I541b260f13e29966cc40e5a11a486ed40b2d27aa Reviewed-on: https://chromium-review.googlesource.com/1124078 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Commit-Queue: Kenichi Ishibashi <bashi@chromium.org> Cr-Commit-Position: refs/heads/master@{#572707} [modify] https://crrev.com/6f4394143cd95271b0d65cc3d7f034359fa460cf/content/browser/service_worker/service_worker_script_loader_factory.cc
,
Jul 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2838dd71b6be5ec2a6dba100038351bcda2f5eb0 commit 2838dd71b6be5ec2a6dba100038351bcda2f5eb0 Author: Kenichi Ishibashi <bashi@chromium.org> Date: Mon Jul 09 03:53:17 2018 service worker: Add UseCounter for importScripts() after main script installation Currently we allow importScripts() to load a new script after service worker's installation. According to the spec we should throw a TypeError in such case. We want to change the behavior but it seems that some sites depend on the current behavior so changing the behavior is a bit risky at this point. This CL add a UseCounter to track such usage. Bug: 852666 , 719052 Change-Id: I3e5748a7ce037be75f4f2dfa965d8cb34df9b9bc Reviewed-on: https://chromium-review.googlesource.com/1126799 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Commit-Queue: Kenichi Ishibashi <bashi@chromium.org> Cr-Commit-Position: refs/heads/master@{#573209} [modify] https://crrev.com/2838dd71b6be5ec2a6dba100038351bcda2f5eb0/third_party/blink/public/platform/web_feature.mojom [modify] https://crrev.com/2838dd71b6be5ec2a6dba100038351bcda2f5eb0/third_party/blink/renderer/modules/serviceworkers/service_worker_global_scope.cc [modify] https://crrev.com/2838dd71b6be5ec2a6dba100038351bcda2f5eb0/tools/metrics/histograms/enums.xml
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/756edc8318051658df82da1a046ebff888c64216 commit 756edc8318051658df82da1a046ebff888c64216 Author: Kenichi Ishibashi <bashi@chromium.org> Date: Tue Jul 10 00:48:08 2018 service worker: Update description of ContextRequestHandlerStatus UMA Add notes to describe this is semi-deprecated. Bug: 852666 Change-Id: If19554c580ba082187364211451a106d275e9d96 Reviewed-on: https://chromium-review.googlesource.com/1128659 Commit-Queue: Kenichi Ishibashi <bashi@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Cr-Commit-Position: refs/heads/master@{#573563} [modify] https://crrev.com/756edc8318051658df82da1a046ebff888c64216/tools/metrics/histograms/histograms.xml
,
Jul 10
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dd07120e69cae66ec541cee8cf59f96995ebe513 commit dd07120e69cae66ec541cee8cf59f96995ebe513 Author: Matt Falkenhagen <falken@chromium.org> Date: Tue Jul 10 04:58:45 2018 service worker: Minor correction to ContextRequestHandlerStatus UMA description. Script streaming launched in M64 and affected the metric. Bug: 852666 Change-Id: I07ab5b00ae9766574f6b7eb22e0b0d4b67f416ba TBR: isherman Reviewed-on: https://chromium-review.googlesource.com/1130560 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#573618} [modify] https://crrev.com/dd07120e69cae66ec541cee8cf59f96995ebe513/tools/metrics/histograms/histograms.xml |
||
►
Sign in to add a comment |
||
Comment 1 by dxie@google.com
, Jun 19 2018