[Webkit-unassigned] [Bug 179318] Implement "UpdateWorkerState", "fire updatefound event", and actually rely on the real update/install algorithms.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=179318

Chris Dumez <cdumez at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cdumez at apple.com

--- Comment #3 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 326120
  --> https://bugs.webkit.org/attachment.cgi?id=326120
WIP, trying to reconcile that microtask without causing test failures

View in context: https://bugs.webkit.org/attachment.cgi?id=326120&action=review

> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:136
> +    // FIXME: Here is the point in time where we need to create a ServiceWorkerRegistration object

This seems premature, no? What if the registration fails?
In the spec, it is constructed when we resolve the promise (https://w3c.github.io/ServiceWorker/#resolve-job-promise), early in the Install steps.

> Source/WebCore/workers/service/server/SWClientConnection.cpp:200
> +            registration->dispatchEvent(Event::create(eventNames().updatefoundEvent, false, false));

You now fire this event twice?

> Source/WebCore/workers/service/server/SWClientConnection.cpp:219
> +    auto* registrations = m_registrations.get(key);

I believe the registrations are on the container nowadays.

> Source/WebCore/workers/service/server/SWClientConnection.cpp:229
> +        registration->dispatchEvent(Event::create(eventNames().updatefoundEvent, false, false));

Why are we moving more of the event firing logic into SWClientConnection? This does not seem like the right place for this.

> Source/WebCore/workers/service/server/SWServer.cpp:262
> +    registration->fireUpdateFoundEvent();

This is technically part of the install steps, maybe this should be moved to an install function for clarity?

> Source/WebCore/workers/service/server/SWServerRegistration.cpp:98
> +    for (auto& connectionIdentifierWithClients : m_clientRegistrationsByConnection.keys()) {

In my experience, the client registration is not added to m_clientRegistrationsByConnection yet by the time we reach this code, in the case of a registration, so it did not end up firing the event at the new registration object.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171106/e71bdb22/attachment-0001.html>