Implement https://w3c.github.io/ServiceWorker/#clients-claim
Created attachment 328118 [details] Patch
Created attachment 328365 [details] Patch
Comment on attachment 328365 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328365&action=review > Source/WebCore/ChangeLog:10 > + Update setting active service woker within ScriptExecutionContext to not do any notification to the storage process. typo: woker > Source/WebCore/workers/service/SWClientConnection.cpp:222 > + client->setActiveServiceWorker(ServiceWorker::getOrCreate(*client, ServiceWorkerData { newController }), ScriptExecutionContext::ShouldNotifyServiceWorkerConnection::No); notifyClientsOfControllerChange() is also called from activate() for example. How does the server know the client's new controlling worker if you use ShouldNotifyServiceWorkerConnection::No here? > Source/WebCore/workers/service/server/SWServer.cpp:371 > + auto controllingRegistration = m_registrations.get(m_runningOrTerminatingWorkers.get(result.iterator->value)->registrationKey()); The spec says to call "Handle Service Worker Client Unload". Where do you do this?
> > Source/WebCore/workers/service/SWClientConnection.cpp:222 > > + client->setActiveServiceWorker(ServiceWorker::getOrCreate(*client, ServiceWorkerData { newController }), ScriptExecutionContext::ShouldNotifyServiceWorkerConnection::No); > > notifyClientsOfControllerChange() is also called from activate() for > example. How does the server know the client's new controlling worker if you > use ShouldNotifyServiceWorkerConnection::No here? I see, it is used for that purpose as well. I think SWServer should set it in StorageProcess without having to wait for WebProcess to set it on its client. > > Source/WebCore/workers/service/server/SWServer.cpp:371 > > + auto controllingRegistration = m_registrations.get(m_runningOrTerminatingWorkers.get(result.iterator->value)->registrationKey()); > > The spec says to call "Handle Service Worker Client Unload". Where do you do > this? In the line just after, when removing client from registration.
(In reply to youenn fablet from comment #4) > > > Source/WebCore/workers/service/SWClientConnection.cpp:222 > > > + client->setActiveServiceWorker(ServiceWorker::getOrCreate(*client, ServiceWorkerData { newController }), ScriptExecutionContext::ShouldNotifyServiceWorkerConnection::No); > > > > notifyClientsOfControllerChange() is also called from activate() for > > example. How does the server know the client's new controlling worker if you > > use ShouldNotifyServiceWorkerConnection::No here? > > I see, it is used for that purpose as well. > I think SWServer should set it in StorageProcess without having to wait for > WebProcess to set it on its client. Sure, but I do not think it is done currently so the patch would regress things. > > > > Source/WebCore/workers/service/server/SWServer.cpp:371 > > > + auto controllingRegistration = m_registrations.get(m_runningOrTerminatingWorkers.get(result.iterator->value)->registrationKey()); > > > > The spec says to call "Handle Service Worker Client Unload". Where do you do > > this? > > In the line just after, when removing client from registration. Actually, client unload should do these too: If registration’s uninstalling flag is set, invoke Try Clear Registration with registration. If registration is not null, invoke Try Activate with registration.
Created attachment 328470 [details] Patch
Comment on attachment 328470 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=328470&action=review r=me > Source/WebCore/workers/service/server/SWServerWorker.cpp:142 > + auto* registration = m_server.getRegistration(m_registrationKey); I think a comment pointing the which part of which algorithm is implemented would be helpful here. > LayoutTests/http/tests/workers/service/serviceworkerclients-claim-worker.js:2 > + if (event.request.url.indexOf("pinkelephant") !== -1) lol
Created attachment 328487 [details] Patch for landing
Thanks for the review. > > Source/WebCore/workers/service/server/SWServerWorker.cpp:142 > > + auto* registration = m_server.getRegistration(m_registrationKey); > > I think a comment pointing the which part of which algorithm is implemented > would be helpful here. Done
Comment on attachment 328487 [details] Patch for landing Rejecting attachment 328487 [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', 328487, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: .webkit.org/git/WebKit 6a810d9..9772276 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 225530 = 6a810d93379060e6a3c4a9b02a287328d9de8c19 r225531 = 977227696fd292541c136dc33a2194e3c3116f3c 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. Full output: http://webkit-queues.webkit.org/results/5505205
Created attachment 328490 [details] Rebasing
Comment on attachment 328490 [details] Rebasing Clearing flags on attachment: 328490 Committed r225537: <https://trac.webkit.org/changeset/225537>
All reviewed patches have been landed. Closing bug.
<rdar://problem/35863924>