Bug 176082

Summary: Implement all of "Register" right up until where the script is fetched
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=174541
Attachments:
Description Flags
EWS run
none
Patch
none
Patch none

Description Brady Eidson 2017-08-29 16:51:22 PDT
Implement all of "Register" right up until where the script is fetched

This includes tests for all the exceptions that can occur along the way.
Comment 1 Brady Eidson 2017-08-29 20:32:59 PDT
Created attachment 319326 [details]
EWS run
Comment 2 Brady Eidson 2017-08-30 09:32:09 PDT
Created attachment 319372 [details]
Patch
Comment 3 Andy Estes 2017-08-30 10:35:23 PDT
Comment on attachment 319372 [details]
Patch

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

> Source/WebCore/workers/service/ServiceWorkerContainer.cpp:170
> +    ScopeGuard guard([this, job = &job] {

Why not just capture &job?

> Source/WebCore/workers/service/ServiceWorkerRegistration.cpp:37
> +{
> +}

Do you need to call suspendIfNeeded() here now that this is an ActiveDOMObject?
Comment 4 Brady Eidson 2017-08-30 10:41:23 PDT
(In reply to Andy Estes from comment #3)
> Comment on attachment 319372 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319372&action=review
> 
> > Source/WebCore/workers/service/ServiceWorkerContainer.cpp:170
> > +    ScopeGuard guard([this, job = &job] {
> 
> Why not just capture &job?

Oh, no reason. (Leftovers from a previous iteration of this function)

> > Source/WebCore/workers/service/ServiceWorkerRegistration.cpp:37
> > +{
> > +}
> 
> Do you need to call suspendIfNeeded() here now that this is an
> ActiveDOMObject?

The answer is no, but simply because there is still no code path that actually ends up in properly resolving the promise with a new registration.

Once there is such a code path, I sure will!
Comment 5 Brady Eidson 2017-08-30 10:58:13 PDT
Created attachment 319379 [details]
Patch
Comment 6 WebKit Commit Bot 2017-08-30 12:40:39 PDT
Comment on attachment 319379 [details]
Patch

Clearing flags on attachment: 319379

Committed r221392: <http://trac.webkit.org/changeset/221392>
Comment 7 WebKit Commit Bot 2017-08-30 12:40:40 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2017-08-30 12:40:57 PDT
<rdar://problem/34169273>