Bug 238441

Summary: Implement ServiceWorker WindowClient.ancestorOrigins
Product: WebKit Reporter: youenn fablet <youennf>
Component: Service WorkersAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, benjamin, cdumez, changseok, clopez, cmarcelo, esprehn+autocc, ews-watchlist, fred.wang, glenn, kondapallykalyan, nham, pdr, rbuis, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch for landing
none
Patch
none
Patch none

youenn fablet
Reported 2022-03-28 02:06:50 PDT
Implement ServiceWorker WindowClient.ancestorOrigins
Attachments
Patch (15.81 KB, patch)
2022-03-28 03:40 PDT, youenn fablet
ews-feeder: commit-queue-
Patch (17.71 KB, patch)
2022-03-28 06:03 PDT, youenn fablet
no flags
Patch (19.61 KB, patch)
2022-03-28 06:59 PDT, youenn fablet
no flags
Patch for landing (22.59 KB, patch)
2022-03-30 03:05 PDT, youenn fablet
no flags
Patch (5.08 KB, patch)
2022-03-30 10:24 PDT, Rob Buis
no flags
Patch (5.83 KB, patch)
2022-03-30 10:27 PDT, Rob Buis
no flags
youenn fablet
Comment 1 2022-03-28 03:40:27 PDT
youenn fablet
Comment 2 2022-03-28 06:03:43 PDT
youenn fablet
Comment 3 2022-03-28 06:59:30 PDT
Chris Dumez
Comment 4 2022-03-29 08:51:14 PDT
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
Comment 5 2022-03-30 00:49:07 PDT
(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
Comment 6 2022-03-30 03:05:18 PDT
Created attachment 456108 [details] Patch for landing
EWS
Comment 7 2022-03-30 06:29:14 PDT
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
Comment 8 2022-03-30 06:30:22 PDT
Rob Buis
Comment 9 2022-03-30 10:24:39 PDT
Reopening to attach new patch.
Rob Buis
Comment 10 2022-03-30 10:24:42 PDT
EWS Watchlist
Comment 11 2022-03-30 10:26:27 PDT
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
Comment 12 2022-03-30 10:27:57 PDT
Rob Buis
Comment 13 2022-03-30 10:30:45 PDT
Uploaded to wrong bug.
Note You need to log in before you can comment on or make changes to this bug.