Implement real post 'install' event steps of the Install algorithm (steps 14+)
Created attachment 326274 [details] WIP patch Of our tests: 1 timeout 1 assert (know how to resolve that) others pass. EWS for WPT while I continue hacking away.
Comment on attachment 326274 [details] WIP patch Attachment 326274 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5141575 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/synced-state.https.html imported/w3c/web-platform-tests/service-workers/service-worker/getregistrations.https.html imported/w3c/web-platform-tests/service-workers/service-worker/ServiceWorkerGlobalScope/postmessage.https.html imported/w3c/web-platform-tests/service-workers/service-worker/multiple-update.https.html imported/w3c/web-platform-tests/service-workers/service-worker/state.https.html imported/w3c/web-platform-tests/service-workers/service-worker/claim-affect-other-registration.https.html imported/w3c/web-platform-tests/service-workers/service-worker/waiting.https.html imported/w3c/web-platform-tests/service-workers/service-worker/oninstall-script-error.https.html imported/w3c/web-platform-tests/service-workers/service-worker/update.https.html http/tests/workers/service/service-worker-clear.html imported/w3c/web-platform-tests/service-workers/service-worker/onactivate-script-error.https.html imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting.https.html imported/w3c/web-platform-tests/service-workers/service-worker/postmessage-from-waiting-serviceworker.https.html http/tests/workers/service/basic-install-event.html imported/w3c/web-platform-tests/service-workers/service-worker/active.https.html imported/w3c/web-platform-tests/service-workers/service-worker/clients-matchall-exact-controller.https.html imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html imported/w3c/web-platform-tests/service-workers/service-worker/activate-event-after-install-state-change.https.html imported/w3c/web-platform-tests/service-workers/service-worker/install-event-type.https.html imported/w3c/web-platform-tests/service-workers/service-worker/claim-using-registration.https.html imported/w3c/web-platform-tests/service-workers/service-worker/register-wait-forever-in-install-worker.https.html
Created attachment 326282 [details] Archive of layout-test-results from ews104 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 326274 [details] WIP patch Attachment 326274 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5141220 Number of test failures exceeded the failure limit.
Created attachment 326284 [details] Archive of layout-test-results from ews126 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews126 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 326274 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=326274&action=review > Source/WebCore/workers/service/server/SWServerJobQueue.cpp:159 > + registration->updateRegistrationState(ServiceWorkerRegistrationState::Waiting, installing); We never register the client registrations with the server, do we? I do not see WebSWClientConnection::addServiceWorkerRegistrationInServer() being called. If so, then those update*State() functions only update the server-side objects, not the client ones.
Comment on attachment 326274 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=326274&action=review >> Source/WebCore/workers/service/server/SWServerJobQueue.cpp:159 >> + registration->updateRegistrationState(ServiceWorkerRegistrationState::Waiting, installing); > > We never register the client registrations with the server, do we? I do not see WebSWClientConnection::addServiceWorkerRegistrationInServer() being called. If so, then those update*State() functions only update the server-side objects, not the client ones. I had to do something like this for it to work: 452void ServiceWorkerContainer::addRegistration(ServiceWorkerRegistration& registration) 453{ 454 m_swConnection->addServiceWorkerRegistrationInServer(registration.data().key, registration.identifier()); 455 m_registrations.add(registration.data().key, ®istration); 456} 457 458void ServiceWorkerContainer::removeRegistration(ServiceWorkerRegistration& registration) 459{ 460 m_swConnection->removeServiceWorkerRegistrationInServer(registration.data().key, registration.identifier()); 461 m_registrations.remove(registration.data().key); 462} See my patch at https://bugs.webkit.org/show_bug.cgi?id=179396
Created attachment 326344 [details] Another WIP
Comment on attachment 326344 [details] Another WIP Attachment 326344 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5151531 Number of test failures exceeded the failure limit.
Created attachment 326360 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 326344 [details] Another WIP Attachment 326344 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5151661 Number of test failures exceeded the failure limit.
Created attachment 326367 [details] Archive of layout-test-results from ews124 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews124 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 326382 [details] WIP
Created attachment 326395 [details] WIP
Created attachment 326407 [details] New WIP
Created attachment 326428 [details] New WIP
Created attachment 326430 [details] Not pretty but seems to pass the tests
Comment on attachment 326430 [details] Not pretty but seems to pass the tests View in context: https://bugs.webkit.org/attachment.cgi?id=326430&action=review > Source/WebCore/workers/service/ServiceWorkerRegistration.cpp:64 > + context.setActiveServiceWorker(getNewestWorker()); I moved this here from ServiceWorkerContainer::jobResolvedWithRegistration() to keep one test (imported/w3c/web-platform-tests/service-workers/service-worker/claim-affect-other-registration.https.html) passing. The test relies on serviceWorker.controller which is currently support via this hack. > Source/WebCore/workers/service/server/SWServerJobQueue.cpp:120 > + registration.updateRegistrationState(job, ServiceWorkerRegistrationState::Installing, worker, false); The 'false' here and on the next line is a hack to make sure these 2 tasks get executed *before* the promise is resolved below. It does so by NOT using a micro task. The rest of this patch seems fine but this part is terrible and we need to find a better way. At least, we now know what was going on. > Source/WebCore/workers/service/server/SWServerJobQueue.cpp:177 > + auto* waiting = registration->waitingWorker(); These changes below are probably not needed but it seems much cleaner than keeping the fake PostInstall Microtask in the WebProcess.
Comment on attachment 326430 [details] Not pretty but seems to pass the tests Attachment 326430 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5159298 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/foreign-fetch-cors.https.html imported/w3c/web-platform-tests/fetch/api/abort/serviceworker-intercepted.https.html
Created attachment 326432 [details] Archive of layout-test-results from ews125 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.6
Created attachment 326468 [details] Alternate approach
(In reply to Chris Dumez from comment #21) > Created attachment 326468 [details] > Alternate approach Ok, will clean up this last patch and get it ready for review today.
(In reply to Chris Dumez from comment #22) > (In reply to Chris Dumez from comment #21) > > Created attachment 326468 [details] > > Alternate approach > > Ok, will clean up this last patch and get it ready for review today. Ping me, will r+ asap
Created attachment 326484 [details] WIP Patch
Created attachment 326492 [details] Patch
Comment on attachment 326492 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=326492&action=review > Source/WebCore/ChangeLog:9 > + Implement real post 'install' event steps of the Install algorithm (steps 14+) > + https://bugs.webkit.org/show_bug.cgi?id=179401 > + > + Reviewed by NOBODY (OOPS!). > + > + Implement real post 'install' event steps of the Install algorithm (steps 14+): > + - https://w3c.github.io/ServiceWorker/#installation-algorithm This is weird duplication. > Source/WebKit/ChangeLog:9 > + Implement real post 'install' event steps of the Install algorithm (steps 14+) > + https://bugs.webkit.org/show_bug.cgi?id=179401 > + > + Reviewed by NOBODY (OOPS!). > + > + Implement real post 'install' event steps of the Install algorithm (steps 14+): > + - https://w3c.github.io/ServiceWorker/#installation-algorithm This is weird duplication
Created attachment 326499 [details] Patch
Committed r224652: <https://trac.webkit.org/changeset/224652>
<rdar://problem/35562200>
*** Bug 179395 has been marked as a duplicate of this bug. ***