Support container.getRegistration() / getRegistrations() inside service workers.
Created attachment 328367 [details] WIP Patch
Created attachment 328373 [details] WIP Patch
Created attachment 328374 [details] WIP Patch
Created attachment 328379 [details] WIP Patch
Comment on attachment 328379 [details] WIP Patch Attachment 328379 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/5491816 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https.html imported/w3c/web-platform-tests/service-workers/service-worker/activation.https.html
Created attachment 328389 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 328379 [details] WIP Patch Attachment 328379 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/5491833 New failing tests: imported/w3c/web-platform-tests/service-workers/service-worker/skip-waiting-without-using-registration.https.html imported/w3c/web-platform-tests/service-workers/service-worker/activation.https.html
Created attachment 328391 [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
Created attachment 328395 [details] Patch
Created attachment 328396 [details] Patch
Created attachment 328397 [details] Patch
Comment on attachment 328397 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328397&action=review > LayoutTests/imported/w3c/ChangeLog:16 > + a cleanup step to unregister. The test now passes all checks. PR: https://github.com/w3c/web-platform-tests/pull/8559
Comment on attachment 328397 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328397&action=review > Source/WebCore/dom/ScriptExecutionContext.cpp:588 > +void ScriptExecutionContext::postTaskTo(const DocumentOrWorkerIdentifier& contextIdentifier, WTF::Function<void(ScriptExecutionContext&)>&& task) I am always wondering whether to use Identifier or const Identifier& in such cases. I have difficulties keeping it consistent. Since it is a Variant, it is better to pass it by ref? > Source/WebCore/dom/ScriptExecutionContext.cpp:592 > + switchOn(contextIdentifier, [&](DocumentIdentifier identifier) { space between ] and ( is apparently our style. > Source/WebCore/dom/ScriptExecutionContext.cpp:596 > + document->postTask([task = WTFMove(task)](ScriptExecutionContext& scope) { auto& here and in other various places below ? > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:232 > + Ref<SecurityOrigin> topOrigin = context->topOrigin(); This may end up topOrigin to be destroyed on the main thread. Shouldn't we isolate it? Ditto below probably. > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:320 > + } You can probably write it as: auto registrations = WTF::map(WTFMove(registrations), [this] (auto&& registrationData) { return ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(registrationData)); }); > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:545 > + m_pendingGetRegistrationsRequests.clear(); I would try to use one map that keeps DeferredPromise instead of typed DOMPromiseDeferred. > Source/WebKit/WebProcess/Storage/WebSWClientConnection.cpp:140 > + ASSERT(isMainThread()); We do not do that often when receiving IPC messages. Should we try to do that more aggressively? Or should these assertions be moved to the lambdas instead?
Comment on attachment 328397 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328397&action=review >> Source/WebCore/dom/ScriptExecutionContext.cpp:588 >> +void ScriptExecutionContext::postTaskTo(const DocumentOrWorkerIdentifier& contextIdentifier, WTF::Function<void(ScriptExecutionContext&)>&& task) > > I am always wondering whether to use Identifier or const Identifier& in such cases. > I have difficulties keeping it consistent. Since it is a Variant, it is better to pass it by ref? I am not sure in the case of a Variant so I figured better safe than sorry. >> Source/WebCore/dom/ScriptExecutionContext.cpp:592 >> + switchOn(contextIdentifier, [&](DocumentIdentifier identifier) { > > space between ] and ( is apparently our style. Uh? ok. >> Source/WebCore/dom/ScriptExecutionContext.cpp:596 >> + document->postTask([task = WTFMove(task)](ScriptExecutionContext& scope) { > > auto& here and in other various places below ? ok. >> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:232 >> + Ref<SecurityOrigin> topOrigin = context->topOrigin(); > > This may end up topOrigin to be destroyed on the main thread. Shouldn't we isolate it? > Ditto below probably. Will isolateCopy(). >> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:320 >> + } > > You can probably write it as: > auto registrations = WTF::map(WTFMove(registrations), [this] (auto&& registrationData) { > return ServiceWorkerRegistration::getOrCreate(*scriptExecutionContext(), *this, WTFMove(registrationData)); > }); Ok, wasn't familiar with WTF::map() >> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:545 >> + m_pendingGetRegistrationsRequests.clear(); > > I would try to use one map that keeps DeferredPromise instead of typed DOMPromiseDeferred. Will look into this.
Created attachment 328409 [details] Patch
Comment on attachment 328409 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328409&action=review > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:292 > + Ref<SecurityOrigin> topOrigin = context->topOrigin(); No need for this variable and refing topOrigin probably as well.
Created attachment 328411 [details] Patch
Comment on attachment 328411 [details] Patch Rejecting attachment 328411 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 328411, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: ..99b2274 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 225506 = 6dd634539ab447fd1507bbe274df2c73431ab7b7 r225507 = 99b227420d9a3b88cc44f866efee9ec3cb1c6da1 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Total errors found: 0 in 0 files Full output: http://webkit-queues.webkit.org/results/5494450
Comment on attachment 328411 [details] Patch Clearing flags on attachment: 328411 Committed r225513: <https://trac.webkit.org/changeset/225513>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35846638>