RESOLVED FIXED 216373
Followup to Bug 215027: address comments to improve APP_BOUND_DOMAINS macro use
https://bugs.webkit.org/show_bug.cgi?id=216373
Summary Followup to Bug 215027: address comments to improve APP_BOUND_DOMAINS macro use
Kate Cheney
Reported 2020-09-10 10:50:18 PDT
This is a followup to Bug 215027 to address Darin's comments to improve the use of the APP_BOUND_DOMAINS macro. <rdar://problem/68645704>
Attachments
Patch (9.69 KB, patch)
2020-09-10 11:59 PDT, Kate Cheney
no flags
Patch (9.91 KB, patch)
2020-09-10 15:05 PDT, Kate Cheney
no flags
Patch for landing (9.10 KB, patch)
2020-09-11 09:40 PDT, Kate Cheney
no flags
Kate Cheney
Comment 1 2020-09-10 11:59:55 PDT
Darin Adler
Comment 2 2020-09-10 12:40:42 PDT
Comment on attachment 408465 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408465&action=review > LayoutTests/platform/wk2/TestExpectations:732 > +# App Bound Domains is for iOS only, so we should skip > +# them here and enable them in the iOS platform specific file. > +http/tests/resourceLoadStatistics/exemptDomains/ [ Skip ] I suggest putting this into TestExpectations, rather than platform/wk2/TestExpectations. Will that work?
Kate Cheney
Comment 3 2020-09-10 13:00:32 PDT
(In reply to Darin Adler from comment #2) > Comment on attachment 408465 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408465&action=review > > > LayoutTests/platform/wk2/TestExpectations:732 > > +# App Bound Domains is for iOS only, so we should skip > > +# them here and enable them in the iOS platform specific file. > > +http/tests/resourceLoadStatistics/exemptDomains/ [ Skip ] > > I suggest putting this into TestExpectations, rather than > platform/wk2/TestExpectations. Will that work? Maybe. All of http/tests/resourceLoadStatistics/ is currently set to 'pass' in LayoutTests/platform/wk2/TestExpectations, which overwrites the 'skip' for http/tests/resourceLoadStatistics/exemptDomains/ in LayoutTests/TestExpectations. I could set http/tests/resourceLoadStatistics/ to 'pass' in LayoutTests/TestExpectations instead of LayoutTests/platform/wk2/TestExpectations but then would have to edit all non-cocoa platform files to skip resourceLoadStatistics/ tests, which seems just as inelegant. Maybe you see a better option though?
Darin Adler
Comment 4 2020-09-10 14:52:09 PDT
(In reply to katherine_cheney from comment #3) > Maybe you see a better option though? No, I guess that means my idea won’t work. Sorry, I could have figured that out if I thought more carefully about your earlier comments.
Kate Cheney
Comment 5 2020-09-10 15:00:05 PDT
(In reply to Darin Adler from comment #4) > (In reply to katherine_cheney from comment #3) > > Maybe you see a better option though? > > No, I guess that means my idea won’t work. Sorry, I could have figured that > out if I thought more carefully about your earlier comments. No problem. Looks like I need to rebase to resolve a conflict in the ios-wk2 TestExpectation file to make the gtk and wincairo bots happy. I'll re-upload and confirm the bots are all green before landing.
Kate Cheney
Comment 6 2020-09-10 15:05:39 PDT
Kate Cheney
Comment 7 2020-09-11 09:40:34 PDT
Created attachment 408538 [details] Patch for landing
EWS
Comment 8 2020-09-11 10:33:31 PDT
Committed r266929: <https://trac.webkit.org/changeset/266929> All reviewed patches have been landed. Closing bug and clearing flags on attachment 408538 [details].
Note You need to log in before you can comment on or make changes to this bug.