Implement "Soft Update" algorithm: - https://w3c.github.io/ServiceWorker/#soft-update-algorithm
<rdar://problem/36163461>
Created attachment 330036 [details] Patch
Created attachment 330040 [details] Patch
Comment on attachment 330040 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=330040&action=review > Source/WebCore/workers/service/SWClientConnection.cpp:132 > + failedFetchingScript(jobDataIdentifier, registrationKey, ResourceError { errorDomainWebKitInternal, 0, URL(), ASCIILiteral("Failed to fetch service worker script") }); I don't know whether this makeScopeExit is worth it here: - There is only one early return - We could probably have different error messages for the two failure cases. > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:195 > if (!context || !context->sessionID().isValid()) { Given updateRegistration is only called by ServiceWorkerRegistration, we probably do not need to check that context is null since this is done at the call sites. We could also move that if statement and maybe the next one to the two call sites. > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:368 > context->postTask([job = makeRef(job), exception](ScriptExecutionContext&) { Looking at the code, it seems we take a job ref both to go to main thread and to go to worker thread. Given it carries a Ref<DeferredPromise>, I do not think this is actually safe, since we may destroy the promise in the main thread. We should probably rework this. For instance m_scheduledJobs should not have refs of jobs that are created in the worker thread. > Source/WebCore/workers/service/context/SWContextManager.cpp:131 > + serviceWorker->thread().runLoop().postTask([task = WTFMove(task)] (auto& context) { Are we sure postTask will actually always execute the task? I would guess that if we are trying to terminate the worker, adding a task in the queue will not make the task be executed. Maybe the m_workerMap check prevents that case. If so, we could assert that the queue is not being killed. > Source/WebCore/workers/service/context/ServiceWorkerFetch.cpp:141 > + if (isNonSubresourceRequest || (registration.lastUpdateTime() && (WallTime::now() - registration.lastUpdateTime()) > 86400_s)) We probably have this now - lastUpdateTime > 86400 in various places. Would it be useful to have a helper function for this on the registration?
Created attachment 330138 [details] Patch
Comment on attachment 330138 [details] Patch Clearing flags on attachment: 330138 Committed r226274: <https://trac.webkit.org/changeset/226274>
All reviewed patches have been landed. Closing bug.
After this patch the test imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html has become very flaky on WK2. It is most common on macOS: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fservice-workers%2Fservice-worker%2Fregister-same-scope-different-script-url.https.html https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r226294%20(1953)/results.html https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/1953 diff: --- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https-actual.txt @@ -1,7 +1,9 @@ + +Harness Error (TIMEOUT), message = null PASS Register different scripts concurrently PASS Register then register new script URL PASS Register then register new script URL that 404s FAIL Register then register new script that does not install assert_unreached: unexpected rejection: assert_equals: on redundant, installing should be null expected null but got object "[object ServiceWorker]" Reached unreachable code -PASS Register same-scope new script url effect on controller +TIMEOUT Register same-scope new script url effect on controller Test timed out
Created new bug for above flakiness: https://bugs.webkit.org/show_bug.cgi?id=181166