Promise returned by `skipWaiting` is resolved late
Reported by
m...@mikepennisi.com,
May 23 2017
|
|||||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/58.0.3029.110 Chrome/58.0.3029.110 Safari/537.36
Steps to reproduce the problem:
1. Register a service worker with an empty script
2. Wait for that worker to become active
3. Register a service worker with the following script:
```
oninstall = function() {
console.log('install');
setTimeout(function() {
self.skipWaiting()
.then(function() {
console.log('skipWaiting resolved');
});
}, 0);
};
onactivate = function() { console.log('activate'); };
```
What is the expected behavior?
String values are printed to the developer's console in the following sequence:
"install", "skipWaiting resolved", "activate"
What went wrong?
String values are usually\* printed to the developer's console in the following
sequence: "install", "activate", "skipWaiting resolved"
Did this work before? N/A
Does this work in other browsers? N/A
Chrome version: 58.0.3029.110 Channel: stable
OS Version:
Flash Version:
> 1. Let promise be a new promise.
> 2. Run the following substeps in parallel:
> 1. Set service worker's skip waiting flag.
> 2. Invoke Try Activate with service worker's containing service worker
> registration.
> 3. Resolve promise with undefined.
> 3. Return promise.
source:
https://w3c.github.io/ServiceWorker/#service-worker-global-scope-skipwaiting
(retrieved 2017-05-23)
Even in cases where "Try Activate" synchronously runs "Activate" (as in the
example above), the "Update Worker State" algorithm modifies the worker in a
new task ("Queue a task to run these substeps: [...]"). The promise returned by
the `skipWaiting` function is resolved prior to this, and any fulfillment
handlers are scheduled as microtasks and likewise executed before the state
change.
This behavior is only observable if `skipWaiting` is invoked after the
`install` event has been fully processed. The demonstration above achieves this
using the `setTimeout` API for simplicity's sake, but this is more likely to
actually occur when `skipWaiting` is invoked in the handler for a functional
event such as `onmessage`.
\* - The bug is somewhat inconsistent; in rare cases, the expected behavior can be observed.
,
May 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a03921af62dfc391b10999380a25e2baf2bf22bc commit a03921af62dfc391b10999380a25e2baf2bf22bc Author: mike <mike@mikepennisi.com> Date: Mon May 29 17:01:26 2017 Upstream service wrkr "skipWaiting" tests to WPT **skip-waiting-installed** This test exists in both the Web Platform Tests project and in the Chromium project, but the two versions differ slightly. Update the Web Platform Tests version with improvements available in the Chromium version: - Remove unnecessary `<script>` include - Discourage worker termination by invoking `event.waitUntil` - Refactor promise management to avoid timing out in response to assertion failures - Introduce a "use strict" directive The Chromium version contains an additional assertion that is faulty. Extend the Web Platform Tests version with a corrected form of this assertion. Persist the Chromium version in its current state in order to preserve test coverage, but annotate that version with references to a new Chromium issue which tracks the resolution of the underlying bug. **skip-waiting-using-registration** This test exists in equivalent forms in both the Web Platform Tests project and in the Chromium project. Update the Web Platform Tests version: - Remove unnecessary `<script>` include - Refactor promise management to avoid timing out in response to assertion failures - Increase prevision of test "cleanup" logic - Introduce a "use strict" directive Remove the Chromium version of the test. **skip-waiting-without-client** This test exists in equivalent forms in both the Web Platform Tests project and in the Chromium project. Remove an unnecessary `<script>` include from the Web Platform Tests version. Remove the Chromium version of the test. **skip-waiting-without-using-registration** This test exists in equivalent forms in both the Web Platform Tests project and in the Chromium project. Update the Web Platform Tests version: - Remove unnecessary `<script>` include - Increase prevision of test "cleanup" logic - Introduce a "use strict" directive Remove the Chromium version of the test. **skip-waiting** This test exists in equivalent forms in both the Web Platform Tests project and in the Chromium project. Update the Web Platform Tests version: - Remove unnecessary `<script>` include - Increase prevision of test "cleanup" logic - Introduce a "use strict" directive Remove the Chromium version of the test. --- Remove the `skip-waiting-worker.js` "resource" file from the Chromium project as it is no longer referenced by any tests. BUG= 688116 , 725616 R=falken@chromium.org Review-Url: https://codereview.chromium.org/2900183002 Cr-Commit-Position: refs/heads/master@{#475361} [modify] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/skip-waiting-installed-worker.js [add] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting-installed.https-expected.txt [modify] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting-installed.https.html [modify] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting-using-registration.https.html [modify] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting-without-client.https.html [modify] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting-without-using-registration.https.html [modify] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting.https.html [rename] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.skip-waiting-installed.html [modify] https://crrev.com/a03921af62dfc391b10999380a25e2baf2bf22bc/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/skip-waiting-installed-worker.js [delete] https://crrev.com/ae70e708c0a58bf54e190956cb9a560bb5036974/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/skip-waiting-worker.js [delete] https://crrev.com/ae70e708c0a58bf54e190956cb9a560bb5036974/third_party/WebKit/LayoutTests/http/tests/serviceworker/skip-waiting-using-registration.html [delete] https://crrev.com/ae70e708c0a58bf54e190956cb9a560bb5036974/third_party/WebKit/LayoutTests/http/tests/serviceworker/skip-waiting-without-client.html [delete] https://crrev.com/ae70e708c0a58bf54e190956cb9a560bb5036974/third_party/WebKit/LayoutTests/http/tests/serviceworker/skip-waiting-without-using-registration.html [delete] https://crrev.com/ae70e708c0a58bf54e190956cb9a560bb5036974/third_party/WebKit/LayoutTests/http/tests/serviceworker/skip-waiting.html
,
Jul 17 2017
I am investigating this issue, and found the test case may has problem, see my question in github: https://github.com/w3c/web-platform-tests/issues/6548. If the owner confirm the test issue, we can start review https://chromium-review.googlesource.com/c/571612/. And I also found the root cause of the bug "`skipWaiting` is resolved late", However, to fix the bug, it's maybe better after the mojofacation work done.
,
Jul 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a32f0c8cada092a820574aaa71df8477a5fa448d commit a32f0c8cada092a820574aaa71df8477a5fa448d Author: xzhan96 <xiaofeng.zhang@intel.com> Date: Wed Jul 19 09:06:24 2017 Fix the issue of skip-waiting-installed.https.html According to the current spec, skipWaiting() promise should be resolved earlier even in the case that the service worker state is "installed", and then continues the activate in parallel. The test case violate that. The background is in https://github.com/w3c/ServiceWorker/issues/1015. BUG= 725616 Change-Id: Ic09b4d404ea41138c8bda8d3ced07094a7ebc6ab Reviewed-on: https://chromium-review.googlesource.com/571612 Commit-Queue: Xiaofeng Zhang <xiaofeng.zhang@intel.com> Reviewed-by: Makoto Shimazu <shimazu@chromium.org> Cr-Commit-Position: refs/heads/master@{#487797} [modify] https://crrev.com/a32f0c8cada092a820574aaa71df8477a5fa448d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting-installed.https.html
,
Jul 25 2017
,
Aug 3 2017
Looks like skip-waiting-installed.https.html is fixed. Is anything left to do here?
,
Aug 3 2017
falken@, my previous fix is just for the test case's issue, the issue will happens only after we fix the issue of "skipWaiting is resolved late", this is a chromium implementation issue. I knows how to fix the issue of "skipWaiting is resolved late", but I am worrying if there is IPC and mojo order problem.
,
Aug 3 2017
The issue was caused by DidSkipWaiting would not be called when service worker status is INSTALLED, see my fix https://chromium-review.googlesource.com/c/599570. However, I am thinking if it has the order issue between ServiceWorkerMsg_DidSkipWaiting(IPC) and activate mojo event? That will case the test result flaky. The similar case is crbug.com/745327 .
,
Aug 17 2017
,
Aug 17 2017
,
Sep 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da03debaf12b660bde0a18a4fbd631b03b7673fb commit da03debaf12b660bde0a18a4fbd631b03b7673fb Author: xzhan96 <xiaofeng.zhang@intel.com> Date: Mon Sep 04 16:55:43 2017 Correct the skip-waiting-installed.html behavior According to the latest spec, the skipWaiting promise should be resolved after 'activate' event is dispatched. This change is from https://github.com/w3c/ServiceWorker/pull/1065. Tracing through the spec, skipWaiting() enters "Try Activate". "Try Activate" invokes "Activate". "Activate" blocks until the final step: "13. Run the Update Worker State algorithm passing registration’s active worker and activated as the arguments." "Update Worker State" queues a task to set ServiceWorker#state to 'activated'. But in step 10, we have dispatched the 'activate' event. Therefore the order should be: 1. 'activate' event handler runs 2. skipWaiting() promise resolves 3. ServiceWorker#state is set to 'activated' So we correct the test case here and delete all the wrong expected files. BUG= 725616 Change-Id: Id0765988c7cdf48f39bb73ccb3fc0cce6ea60949 Reviewed-on: https://chromium-review.googlesource.com/646244 Commit-Queue: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Cr-Commit-Position: refs/heads/master@{#499513} [modify] https://crrev.com/da03debaf12b660bde0a18a4fbd631b03b7673fb/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/skip-waiting-installed-worker.js [delete] https://crrev.com/e385f3369ac6d64ad242ae58abbf471ea29a6668/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/skip-waiting-installed.https-expected.txt [delete] https://crrev.com/e385f3369ac6d64ad242ae58abbf471ea29a6668/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium.skip-waiting-installed.html [delete] https://crrev.com/e385f3369ac6d64ad242ae58abbf471ea29a6668/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/skip-waiting-installed-worker.js [delete] https://crrev.com/e385f3369ac6d64ad242ae58abbf471ea29a6668/third_party/WebKit/LayoutTests/platform/linux/virtual/mojo-blobs/external/wpt/service-workers/service-worker/skip-waiting-installed.https-expected.txt [delete] https://crrev.com/e385f3369ac6d64ad242ae58abbf471ea29a6668/third_party/WebKit/LayoutTests/platform/linux/virtual/service-worker-script-streaming/external/wpt/service-workers/service-worker/skip-waiting-installed.https-expected.txt [delete] https://crrev.com/e385f3369ac6d64ad242ae58abbf471ea29a6668/third_party/WebKit/LayoutTests/platform/mac/virtual/mojo-blobs/external/wpt/service-workers/service-worker/skip-waiting-installed.https-expected.txt [delete] https://crrev.com/e385f3369ac6d64ad242ae58abbf471ea29a6668/third_party/WebKit/LayoutTests/platform/mac/virtual/service-worker-script-streaming/external/wpt/service-workers/service-worker/skip-waiting-installed.https-expected.txt
,
Sep 21 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by falken@chromium.org
, May 24 2017Labels: -OS-Linux -Pri-2 OS-All Pri-3
Status: Available (was: Unconfirmed)