Extend byte-for-byte update check to all service worker importScripts() resources |
||||||||||||||||||
Issue descriptionTaken from the Mozilla bug tracking the implementation in Firefox (https://bugzilla.mozilla.org/show_bug.cgi?id=1290951): See: https://github.com/slightlyoff/ServiceWorker/issues/839#issuecomment-236256162 Basically this means iterating over every entry in the service worker's associated CacheStorage and performing a byte-for-byte check. This can be done in parallel. Design doc: https://docs.google.com/document/d/1p_2axsp96GfME5lE4Zd-p5ei80-9OD_brFUjLYyN8B0/edit
,
Sep 23 2016
,
Oct 5 2016
,
Nov 2 2016
,
Dec 19 2016
,
Jan 26 2017
"Started" is no longer accurate but it's still high priority. I started looking at this and ended up fixing Issue 485900 , which should set some of the stage for this.
,
Feb 16 2017
,
Mar 30 2017
FWIW, we're close to landing this in FF55. We have a WPT test written in: https://bugzilla.mozilla.org/show_bug.cgi?id=1290951
,
Jul 14 2017
Sorry, didn't get to this in 58 at all... We probably won't get to this until servicification settles down.
,
Mar 14 2018
Note: I'm thinking this is suitable for an intern project.
,
Apr 3 2018
external/wpt/service-workers/service-worker/update-bytecheck.https.html is failing due to this.
,
Apr 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b849563d597779aa87924d39d28e04c012919dfd commit b849563d597779aa87924d39d28e04c012919dfd Author: Kenichi Ishibashi <bashi@chromium.org> Date: Tue Apr 03 08:02:25 2018 Update test expectations for update-bytecheck.https.html This is failing because we don't support byte-for-byte update check to all service worker importScript() resources. Update test expectations to point an appropriate bug entry and remove the entry from S13nSW expectation as this failure isn't specific to S13nSW. Bug: 648295 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo Change-Id: I65dca2a31baa500ae8f1d347af8c77d9e9fd8c29 Reviewed-on: https://chromium-review.googlesource.com/991472 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Commit-Queue: Kenichi Ishibashi <bashi@chromium.org> Cr-Commit-Position: refs/heads/master@{#547643} [modify] https://crrev.com/b849563d597779aa87924d39d28e04c012919dfd/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService [modify] https://crrev.com/b849563d597779aa87924d39d28e04c012919dfd/third_party/WebKit/LayoutTests/TestExpectations
,
May 14 2018
shimazu has a plan here.
,
Aug 1
Let me pass this to my intern. She will work on this as her main project.
,
Aug 21
,
Aug 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fc3522926869bf9cbaca9111eaacebaf412ad038 commit fc3522926869bf9cbaca9111eaacebaf412ad038 Author: momohatt <momohatt@google.com> Date: Wed Aug 29 05:12:36 2018 Add ServiceWorkerImportedScriptUpdateCheck flag This CL tentatively adds ServiceWorkerImportedScriptUpdateCheck flag. It is for the implementation of byte-for-byte update check for service worker importScripts() resources. We are going to implement the feature under this flag and flip it over once it is completed. Bug: 648295 Change-Id: Iabb6140af84382ecd0d07ee28283490c303e8c4d Reviewed-on: https://chromium-review.googlesource.com/1192571 Commit-Queue: Momoko Hattori <momohatt@google.com> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#587017} [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/chrome/browser/about_flags.cc [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/chrome/browser/flag_descriptions.h [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/third_party/blink/common/features.cc [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/third_party/blink/common/service_worker/service_worker_utils.cc [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/third_party/blink/public/common/features.h [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/third_party/blink/public/common/service_worker/service_worker_utils.h [modify] https://crrev.com/fc3522926869bf9cbaca9111eaacebaf412ad038/tools/metrics/histograms/enums.xml
,
Sep 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aec50d02dd761d59e93c4c9ddd45e534959fcfe9 commit aec50d02dd761d59e93c4c9ddd45e534959fcfe9 Author: momohatt <momohatt@google.com> Date: Fri Sep 07 11:33:59 2018 Add pause_when_not_identical option for ServiceWorkerCacheWriter This patch adds pause_when_not_identical option for making ServiceWorkerCacheWriter::MaybeWriteData() return before writing the new body into the storage when the scripts are not identical. This option is necessary for extending byte-for-byte update check of service worker importScripts() resources. Bug: 648295 Change-Id: If33412cd6e63393ad3fdae514b1588a33f17399a Reviewed-on: https://chromium-review.googlesource.com/1198651 Commit-Queue: Momoko Hattori <momohatt@google.com> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#589492} [modify] https://crrev.com/aec50d02dd761d59e93c4c9ddd45e534959fcfe9/content/browser/service_worker/service_worker_cache_writer.cc [modify] https://crrev.com/aec50d02dd761d59e93c4c9ddd45e534959fcfe9/content/browser/service_worker/service_worker_cache_writer.h [modify] https://crrev.com/aec50d02dd761d59e93c4c9ddd45e534959fcfe9/content/browser/service_worker/service_worker_cache_writer_unittest.cc [modify] https://crrev.com/aec50d02dd761d59e93c4c9ddd45e534959fcfe9/content/browser/service_worker/service_worker_new_script_loader.cc [modify] https://crrev.com/aec50d02dd761d59e93c4c9ddd45e534959fcfe9/content/browser/service_worker/service_worker_write_to_cache_job.cc
,
Sep 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a4538ad8d0bac55bb81aed0b8181a6b5ed1048b commit 9a4538ad8d0bac55bb81aed0b8181a6b5ed1048b Author: momohatt <momohatt@google.com> Date: Tue Sep 11 08:13:44 2018 Add ServiceWorkerCacheWriter::Resume() for paused cache writers This method resumes a cache writer which is paused by enabling pause_when_not_identical option. With this option enabled, a cache writer pauses right after it finds any differences with the cached body and the body from network, and calling Resume() to such paused cache writers makes them resume its internal state machine and complete the remaining read and write to the storage. This method takes a callback, but it is triggered only when the remaining read and write finishes asynchronously. Bug: 648295 Change-Id: I1c01804151bff20fc69ba7d0993447f676e98548 Reviewed-on: https://chromium-review.googlesource.com/1214975 Commit-Queue: Momoko Hattori <momohatt@google.com> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#590231} [modify] https://crrev.com/9a4538ad8d0bac55bb81aed0b8181a6b5ed1048b/content/browser/service_worker/service_worker_cache_writer.cc [modify] https://crrev.com/9a4538ad8d0bac55bb81aed0b8181a6b5ed1048b/content/browser/service_worker/service_worker_cache_writer.h [modify] https://crrev.com/9a4538ad8d0bac55bb81aed0b8181a6b5ed1048b/content/browser/service_worker/service_worker_cache_writer_unittest.cc
,
Sep 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f24d40940a76b0ec817d5e3c54ea0718e4ee887a commit f24d40940a76b0ec817d5e3c54ea0718e4ee887a Author: momohatt <momohatt@google.com> Date: Tue Sep 11 10:52:43 2018 Add ServiceWorkerUpdateChecker This patch adds a new class called ServiceWorkerUpdateChecker and makes ServiceWorkerRegisterJob use this class. The new class has a method called Start(), which takes a callback as an argument and triggers it only when the scripts had any changes to update. Currently this method runs the callback unconditionally, but following patches will implement script loading and comparison with the stored resources. Bug: 648295 Change-Id: Iebbe85b6d5e5cdab27c23873b04811626b5abad0 Reviewed-on: https://chromium-review.googlesource.com/1215458 Commit-Queue: Momoko Hattori <momohatt@google.com> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#590260} [modify] https://crrev.com/f24d40940a76b0ec817d5e3c54ea0718e4ee887a/content/browser/BUILD.gn [modify] https://crrev.com/f24d40940a76b0ec817d5e3c54ea0718e4ee887a/content/browser/service_worker/service_worker_register_job.cc [modify] https://crrev.com/f24d40940a76b0ec817d5e3c54ea0718e4ee887a/content/browser/service_worker/service_worker_register_job.h [add] https://crrev.com/f24d40940a76b0ec817d5e3c54ea0718e4ee887a/content/browser/service_worker/service_worker_update_checker.cc [add] https://crrev.com/f24d40940a76b0ec817d5e3c54ea0718e4ee887a/content/browser/service_worker/service_worker_update_checker.h
,
Sep 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/130be1c380e444e53a2e25302f50629af94682a5 commit 130be1c380e444e53a2e25302f50629af94682a5 Author: momohatt <momohatt@google.com> Date: Thu Sep 13 12:18:08 2018 Add ServiceWorkerSingleScriptUpdateChecker This patch adds a new class called ServiceWorkerSingleScriptUpdateChecker, and makes ServiceWorkerUpdateChecker call this class. This new class currently calls UpdateChecker's method unconditionally, but following patches will implement these to SWSingleScriptUpdateChecker: * collect stored scripts which are used in the version to update * load the counterpart of the stored scripts from network * compare the stored scripts and the loaded scripts one by one * when detected any changes, pause the comparison and stop the entire update check Bug: 648295 Change-Id: I23835fa62838b9c1a979e7ae96f296de6b6ff074 Reviewed-on: https://chromium-review.googlesource.com/1220854 Commit-Queue: Momoko Hattori <momohatt@google.com> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#590979} [modify] https://crrev.com/130be1c380e444e53a2e25302f50629af94682a5/content/browser/BUILD.gn [modify] https://crrev.com/130be1c380e444e53a2e25302f50629af94682a5/content/browser/service_worker/service_worker_register_job.cc [add] https://crrev.com/130be1c380e444e53a2e25302f50629af94682a5/content/browser/service_worker/service_worker_single_script_update_checker.cc [add] https://crrev.com/130be1c380e444e53a2e25302f50629af94682a5/content/browser/service_worker/service_worker_single_script_update_checker.h [modify] https://crrev.com/130be1c380e444e53a2e25302f50629af94682a5/content/browser/service_worker/service_worker_update_checker.cc [modify] https://crrev.com/130be1c380e444e53a2e25302f50629af94682a5/content/browser/service_worker/service_worker_update_checker.h
,
Sep 28
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a20c193cadb0b7f574550d833f3f01832a8f35b3 commit a20c193cadb0b7f574550d833f3f01832a8f35b3 Author: momohatt <momohatt@google.com> Date: Fri Sep 28 13:43:17 2018 Implement ServiceWorkerSingleScriptUpdateChecker This patch enables ServiceWorkerSingleScriptUpdateChecker to check if there are any differences between the stored scripts and its counterparts loaded from the network. The comparison is only performed for scripts that are used in the instance of requested SWVersion. This impl doesn't yet allow SWNewScriptLoader to continue the aborted comparison, but future patches will implement: * passing the result of update check to SWNewScriptLoader * record the info about scripts which are already compared and known to be identical Bug: 648295 Change-Id: If89dd415578b1c5809159f196d398447a96a812f Reviewed-on: https://chromium-review.googlesource.com/1224610 Commit-Queue: Momoko Hattori <momohatt@google.com> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Reviewed-by: Ramin Halavati <rhalavati@chromium.org> Cr-Commit-Position: refs/heads/master@{#595072} [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_cache_writer_unittest.cc [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_register_job.cc [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_single_script_update_checker.cc [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_single_script_update_checker.h [add] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_single_script_update_checker_unittest.cc [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_test_utils.cc [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_test_utils.h [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_update_checker.cc [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/browser/service_worker/service_worker_update_checker.h [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/content/test/BUILD.gn [modify] https://crrev.com/a20c193cadb0b7f574550d833f3f01832a8f35b3/tools/traffic_annotation/summary/annotations.xml
,
Oct 12
I'll take it over since her internship was over.
,
Nov 9
,
Nov 12
,
Nov 21
I started to work on this, thanks.
,
Nov 21
Thanks for taking it over :)
,
Jan 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e1bc015ec830751da0e3be1bb823281208cd9a61 commit e1bc015ec830751da0e3be1bb823281208cd9a61 Author: Ting Shao <ting.shao@intel.com> Date: Mon Jan 14 03:49:14 2019 ServiceWorker: Collect update check result of scripts Update check result of each script are collected and stored in ServiceWorkerVersion. Following patches would use the data to continue the update process if some script changed. Bug: 648295 Change-Id: I2c62ab15db1618065199f948e47465530dded870 Reviewed-on: https://chromium-review.googlesource.com/c/1385858 Commit-Queue: Ting Shao <ting.shao@intel.com> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#622379} [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_register_job.cc [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_register_job.h [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_single_script_update_checker.cc [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_single_script_update_checker.h [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_single_script_update_checker_unittest.cc [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_update_checker.cc [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_update_checker.h [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_version.cc [modify] https://crrev.com/e1bc015ec830751da0e3be1bb823281208cd9a61/content/browser/service_worker/service_worker_version.h
,
Jan 17
ting.shao@: We talked about the spec algorithm a while back. I'm trying to improve it here: https://github.com/w3c/ServiceWorker/issues/1374 and https://github.com/w3c/ServiceWorker/pull/1377. Not sure if this work is blocking you.
,
Jan 17
(6 days ago)
falken@: Thanks for the note, I'm not blocked on this issue, I'm implementing the functionality for the compared identical scripts, so please go ahead. And I am also watching that issue.
,
Jan 17
(6 days ago)
Would this bug fix be available in Chrome 72 release? If yes, which would be the beta build to test this change?
,
Jan 21
(3 days ago)
maddyrockz@: I'm sorry that, as there are still more work to be done for this issue, so this bug can't be included in 72, I will update the target field. |
||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||
Comment 1 by sheriffbot@chromium.org
, Sep 20 2016