Bug 176082 - Implement all of "Register" right up until where the script is fetched
Summary: Implement all of "Register" right up until where the script is fetched
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-29 16:51 PDT by Brady Eidson
Modified: 2017-08-30 12:40 PDT (History)
3 users (show)

See Also:


Attachments
EWS run (54.62 KB, patch)
2017-08-29 20:32 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (63.38 KB, patch)
2017-08-30 09:32 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff
Patch (63.38 KB, patch)
2017-08-30 10:58 PDT, Brady Eidson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>