<html>
    <head>
      <base href="https://bugs.webkit.org/">
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Implement "UpdateWorkerState", "fire updatefound event", and actually rely on the real update/install algorithms."
   href="https://bugs.webkit.org/show_bug.cgi?id=179318#c4">Comment # 4</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - Implement "UpdateWorkerState", "fire updatefound event", and actually rely on the real update/install algorithms."
   href="https://bugs.webkit.org/show_bug.cgi?id=179318">bug 179318</a>
              from <span class="vcard"><a class="email" href="mailto:beidson@apple.com" title="Brady Eidson <beidson@apple.com>"> <span class="fn">Brady Eidson</span></a>
</span></b>
        <pre>(In reply to Chris Dumez from <a href="show_bug.cgi?id=179318#c3">comment #3</a>)
<span class="quote">> Comment on <span class=""><a href="attachment.cgi?id=326120&action=diff" name="attach_326120" title="WIP, trying to reconcile that microtask without causing test failures">attachment 326120</a> <a href="attachment.cgi?id=326120&action=edit" title="WIP, trying to reconcile that microtask without causing test failures">[details]</a></span>
> WIP, trying to reconcile that microtask without causing test failures

> View in context:
> <a href="https://bugs.webkit.org/attachment.cgi?id=326120&action=review">https://bugs.webkit.org/attachment.cgi?id=326120&action=review</a>

> > 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
> (<a href="https://w3c.github.io/ServiceWorker/#resolve-job-promise">https://w3c.github.io/ServiceWorker/#resolve-job-promise</a>), early in the
> Install steps.</span >

That's not true.

It's created at step 6. of the register algorithm by invoking Set Registration.

<a href="https://w3c.github.io/ServiceWorker/#register-algorithm">https://w3c.github.io/ServiceWorker/#register-algorithm</a>

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

> You now fire this event twice?</span >

Why do you say that? (If this patch happens to fire it twice, that's because I'm trying to refactor out one of them.

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

> I believe the registrations are on the container nowadays.</span >

After I started this Friday, yes - As you can see from the patch not applying it's simply out of date.

<span class="quote">> 
> > 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.</span >

SWClientConnection isn't firing the event. It's telling the registration to.

<span class="quote">> 
> > 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?</span >

Maybe. Once I rebase to trunk will take a look.

<span class="quote">> 
> > 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.</span >

Right, and that's what I'm fixing above.</pre>
        </div>
      </p>


      <hr>
      <span>You are receiving this mail because:</span>

      <ul>
          <li>You are the assignee for the bug.</li>
      </ul>
    </body>
</html>