The following layout test is flaky on iOS Release WK2 imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https.html Probable cause: The first failure visible on the dashboard blames https://trac.webkit.org/changeset/226275/webkit Flakiness Dashboard: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fservice-workers%2Fservice-worker%2Fupdate-after-navigation-fetch-event.https.html --- /Volumes/Data/slave/ios-simulator-11-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https-expected.txt +++ /Volumes/Data/slave/ios-simulator-11-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https-actual.txt @@ -1,3 +1,6 @@ -PASS Update should be triggered after a navigation fetch event. +Harness Error (TIMEOUT), message = null + +TIMEOUT Update should be triggered after a navigation fetch event. Test timed out +
<rdar://problem/36443428>
Marked test as flaky in https://trac.webkit.org/changeset/227179/webkit.
Looking at the flakiness dashboard, it is almost all green. There is exactly one failure and it is a TEXT one, not a TIMEOUT: --- /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https-expected.txt +++ /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https-actual.txt @@ -1,3 +1,3 @@ -PASS Update should be triggered after a navigation fetch event. +FAIL Update should be triggered after a navigation fetch event. promise_test: Unhandled rejection with value: object "Error: wait_for_state must be passed a ServiceWorker"
(In reply to Chris Dumez from comment #3) > Looking at the flakiness dashboard, it is almost all green. There is exactly > one failure and it is a TEXT one, not a TIMEOUT: > --- > /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test- > results/imported/w3c/web-platform-tests/service-workers/service-worker/ > update-after-navigation-fetch-event.https-expected.txt > +++ > /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test- > results/imported/w3c/web-platform-tests/service-workers/service-worker/ > update-after-navigation-fetch-event.https-actual.txt > @@ -1,3 +1,3 @@ > > -PASS Update should be triggered after a navigation fetch event. > +FAIL Update should be triggered after a navigation fetch event. > promise_test: Unhandled rejection with value: object "Error: wait_for_state > must be passed a ServiceWorker" The test looks like: return service_worker_unregister_and_register(t, parsed_url, scope) .then(function(r) { registration = r; return wait_for_state(t, registration.installing, 'activated'); }) wait_for_state() is given null so registration.installing is null. I believe this means that the worker is already active. This can happen when registering a service worker that is already registered. The test attempts to unregister *then* register. However, because of the "Try Clear Registration" algorithm, the registration might be kept, simply marked as uninstalling.
(In reply to Chris Dumez from comment #4) > (In reply to Chris Dumez from comment #3) > > Looking at the flakiness dashboard, it is almost all green. There is exactly > > one failure and it is a TEXT one, not a TIMEOUT: > > --- > > /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test- > > results/imported/w3c/web-platform-tests/service-workers/service-worker/ > > update-after-navigation-fetch-event.https-expected.txt > > +++ > > /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test- > > results/imported/w3c/web-platform-tests/service-workers/service-worker/ > > update-after-navigation-fetch-event.https-actual.txt > > @@ -1,3 +1,3 @@ > > > > -PASS Update should be triggered after a navigation fetch event. > > +FAIL Update should be triggered after a navigation fetch event. > > promise_test: Unhandled rejection with value: object "Error: wait_for_state > > must be passed a ServiceWorker" > > The test looks like: > return service_worker_unregister_and_register(t, parsed_url, scope) > .then(function(r) { > registration = r; > return wait_for_state(t, registration.installing, 'activated'); > }) > > wait_for_state() is given null so registration.installing is null. I believe > this means that the worker is already active. This can happen when > registering a service worker that is already registered. The test attempts > to unregister *then* register. However, because of the "Try Clear > Registration" algorithm, the registration might be kept, simply marked as > uninstalling. Interestingly, this service worker is indeed used by several tests: LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/multiple-update.https.html: var script = 'resources/update-nocookie-worker.py'; LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https.html: var script = 'resources/update-nocookie-worker.py'; LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/update-after-oneday.https.html: var script = 'resources/update-nocookie-worker.py';
(In reply to Chris Dumez from comment #5) > (In reply to Chris Dumez from comment #4) > > (In reply to Chris Dumez from comment #3) > > > Looking at the flakiness dashboard, it is almost all green. There is exactly > > > one failure and it is a TEXT one, not a TIMEOUT: > > > --- > > > /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test- > > > results/imported/w3c/web-platform-tests/service-workers/service-worker/ > > > update-after-navigation-fetch-event.https-expected.txt > > > +++ > > > /Volumes/Data/slave/ios-simulator-11-debug-tests-wk2/build/layout-test- > > > results/imported/w3c/web-platform-tests/service-workers/service-worker/ > > > update-after-navigation-fetch-event.https-actual.txt > > > @@ -1,3 +1,3 @@ > > > > > > -PASS Update should be triggered after a navigation fetch event. > > > +FAIL Update should be triggered after a navigation fetch event. > > > promise_test: Unhandled rejection with value: object "Error: wait_for_state > > > must be passed a ServiceWorker" > > > > The test looks like: > > return service_worker_unregister_and_register(t, parsed_url, scope) > > .then(function(r) { > > registration = r; > > return wait_for_state(t, registration.installing, 'activated'); > > }) > > > > wait_for_state() is given null so registration.installing is null. I believe > > this means that the worker is already active. This can happen when > > registering a service worker that is already registered. The test attempts > > to unregister *then* register. However, because of the "Try Clear > > Registration" algorithm, the registration might be kept, simply marked as > > uninstalling. > > Interestingly, this service worker is indeed used by several tests: > LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/ > multiple-update.https.html: var script = > 'resources/update-nocookie-worker.py'; > LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/ > update-after-navigation-fetch-event.https.html: var script = > 'resources/update-nocookie-worker.py'; > LayoutTests/imported/w3c/web-platform-tests/service-workers/service-worker/ > update-after-oneday.https.html: var script = > 'resources/update-nocookie-worker.py'; Looks like I may have been wrong that the issue is with tryClear not calling clear in some cases. I was able to reproduce the test failure without having a register job reusing an existing registration.
Created attachment 341134 [details] Patch
Created attachment 341145 [details] Patch
Comment on attachment 341145 [details] Patch Attachment 341145 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7782552 New failing tests: accessibility/smart-invert-reference.html
Created attachment 341153 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 341153 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 failure is unrelated.
Comment on attachment 341145 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=341145&action=review > Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp:73 > +void DeferredPromise::whenSettled(std::function<void()>&& callback) This is more or less the same code as DOMPromise::whenSettled. Could we try to share it? > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:453 > + }); whenSettled call is not needed if shouldNotifyWhenResolved == ShouldNotifyWhenResolved::No. Maybe notifyWhenResolvedIfNeeded should be an optional<std::function>
Created attachment 341202 [details] Patch
Comment on attachment 341202 [details] Patch Clearing flags on attachment: 341202 Committed r232156: <https://trac.webkit.org/changeset/232156>
All reviewed patches have been landed. Closing bug.