S13nServiceWorker: external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html crashes |
||
Issue descriptionS13nServiceWorker: external/wpt/service-workers/service-worker/import-scripts-resource-map.https.html crashes
,
Feb 22 2018
I vaguely remember that we've planned to dynamically update the list of installed scripts and ask for the scripts to the installed script manager. But making it a URLLoader seems simpler since we may be able to have the logic to determine whether it's installed or not on the browser side.
,
Feb 22 2018
Thanks. I'm working on a patch to refactor ServiceWorkerInstalledScriptSender::Sender into its own class. That does the work of reading from disk and providing data pipes. With that I think we can make a new URLLoader class, which only gets used for this duplicate case. It should be very lightweight: all it'll do is take the data pipes from Sender and give them to URLLoaderClient.
,
Feb 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a39b8ba5d25b20356438380c80ed95dce90f75e3 commit a39b8ba5d25b20356438380c80ed95dce90f75e3 Author: Matt Falkenhagen <falken@chromium.org> Date: Fri Feb 23 07:51:30 2018 service worker: Extract ServiceWorkerInstalledScriptsSender::Sender into its own class. This will be useful to reuse for the linked bug: it's a class that takes a ServiceWorkerResponseReader and gives back data pipes that will be useful for URLLoader. This patch is mostly mechanical. Other than just moving the code, it adds Client interface and renames some functions in ServiceWorkerInstalledScriptsSender to use that interface. It also does minor renaming like |owner_| -> |client_|. R=shimazu Bug: 814583 Change-Id: I236eb8aeb4b7a79ee8584a413260fdbe3f629dbf Reviewed-on: https://chromium-review.googlesource.com/930107 Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Commit-Queue: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#538724} [modify] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/BUILD.gn [add] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/service_worker/service_worker_installed_script_reader.cc [add] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/service_worker/service_worker_installed_script_reader.h [modify] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/service_worker/service_worker_installed_scripts_sender.cc [modify] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/service_worker/service_worker_installed_scripts_sender.h [modify] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/service_worker/service_worker_installed_scripts_sender_unittest.cc [modify] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/service_worker/service_worker_metrics.cc [modify] https://crrev.com/a39b8ba5d25b20356438380c80ed95dce90f75e3/content/browser/service_worker/service_worker_metrics.h
,
Feb 27 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/10fde2da6b806828bea35eed97d05d69c26053b3 commit 10fde2da6b806828bea35eed97d05d69c26053b3 Author: Matt Falkenhagen <falken@chromium.org> Date: Tue Feb 27 17:17:56 2018 S13nServiceWorker: Allow importScript() twice for non-installed workers. An installing service worker can call importScripts('dupe.js'); importScripts('dupe.js');. The first call should install the script, and the second call should read the installed script. This CL implements that. We detect if the script is already installed, and if so we use a simple URLLoader that reads the installed script. This fixes WPT test import-scripts-resource-map.https.html. Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I30c7532cedffaad3d62b12239ab230a0708bb49c Bug: 814583 Reviewed-on: https://chromium-review.googlesource.com/934024 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#539474} [modify] https://crrev.com/10fde2da6b806828bea35eed97d05d69c26053b3/content/browser/BUILD.gn [add] https://crrev.com/10fde2da6b806828bea35eed97d05d69c26053b3/content/browser/service_worker/service_worker_installed_script_loader.cc [add] https://crrev.com/10fde2da6b806828bea35eed97d05d69c26053b3/content/browser/service_worker/service_worker_installed_script_loader.h [modify] https://crrev.com/10fde2da6b806828bea35eed97d05d69c26053b3/content/browser/service_worker/service_worker_script_url_loader_factory.cc [modify] https://crrev.com/10fde2da6b806828bea35eed97d05d69c26053b3/content/browser/service_worker/service_worker_script_url_loader_factory.h [modify] https://crrev.com/10fde2da6b806828bea35eed97d05d69c26053b3/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
,
Feb 27 2018
,
Mar 8 2018
Issue 771527 has been merged into this issue. |
||
►
Sign in to add a comment |
||
Comment 1 by falken@chromium.org
, Feb 22 2018This is testing installing a service worker that does: importScripts('dupe.js'); importScripts('dupe.js'); // again The first importScript should be stored, and the second importScript should just read the stored one instead of going to network. We have some comments in SWScriptURLLoaderFactory about this: // TODO: Make sure we come here only for new / unknown scripts // once script streaming manager in the renderer side stops sending // resource requests for the known script URLs, i.e. add DCHECK for // version->script_cache_map()->LookupResourceId(url) == // kInvalidServiceWorkerResourceId. // // Currently this could be false for the installing worker that imports // the same script twice (e.g. importScripts('dupe.js'); // importScripts('dupe.js');). So what we want to do is read the installed script and send it to the renderer as a URLLoader. I think what we need to do is factor something out of ServiceWorkerInstalledScriptSender that can be given a ServiceWorkerResponseReader, and writes the metadata/body to a given data pipe. Or maybe make it a URLLoader. Did we have any plans in this space?