Bug 238441 – Implement ServiceWorker WindowClient.ancestorOrigins

• Status: RESOLVED
• Resolution: FIXED
• Component: Service Workers

[youenn fablet]

Implement ServiceWorker WindowClient.ancestorOrigins

[youenn fablet]

Created attachment 455895 [details]
Patch

[youenn fablet]

Created attachment 455910 [details]
Patch

[youenn fablet]

Created attachment 455913 [details]
Patch

[Chris Dumez]

Comment on attachment 455913 [details]
Patch

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

r=me but watch out before landing because I think I found a correctness bug.

> Source/WebCore/workers/service/ServiceWorkerClientData.cpp:62
> +    return { identifier, type, frameType, url.isolatedCopy(), lastNavigationWasAppInitiated, isVisible, isFocused, focusOrder, ancestorOrigins };

I bet this should be `crossThreadCopy(ancestorOrigins)`, not `ancestorOrigins`.

> Source/WebCore/workers/service/ServiceWorkerClientData.cpp:80
> +    while (frame) {

nit: I personally think this would look nicer as a for loop (see how it is already implemented in Location::ancestorOrigins()).

> Source/WebCore/workers/service/ServiceWorkerWindowClient.h:46
> +    bool focused() const { return data().isFocused; }

Not a fan of this change as it seems to go against coding style. Booleans without prefixes don't look great.

[youenn fablet]

(In reply to Chris Dumez from comment #4)
> Comment on attachment 455913 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=455913&action=review
> 
> r=me but watch out before landing because I think I found a correctness bug.

Thanks, will fix it.

[youenn fablet]

Created attachment 456108 [details]
Patch for landing

[EWS]

Committed r292105 (249025@main): <https://commits.webkit.org/249025@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456108 [details].

[Radar WebKit Bug Importer]

<rdar://problem/91041206>

[Rob Buis]

Reopening to attach new patch.

[Rob Buis]

Created attachment 456140 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[Rob Buis]

Created attachment 456142 [details]
Patch

[Rob Buis]

Uploaded to wrong bug.