Summary: | Storage Access API: Enable per-page storage access scope and align test cases | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | John Wilander <wilander> | ||||||||||
Component: | WebKit Misc. | Assignee: | John Wilander <wilander> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bfulgham, cdumez, esprehn+autocc, ews-watchlist, kangil.han, tsavell, 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=249382 | ||||||||||||
Attachments: |
|
Description
John Wilander
2020-09-28 19:28:32 PDT
Created attachment 409955 [details]
Patch
Comment on attachment 409955 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409955&action=review > Source/WebCore/ChangeLog:21 > + Changed the default setting for m_storageAccessScope. "to per-page." > Source/WebCore/page/Settings.yaml:705 > + initial: true Does this setting only exist to support test cases? I wonder if we should have some place to collect these kinds of settings (not relevant to this patch today, just an idea). Created attachment 410184 [details]
Patch for landing
Created attachment 410185 [details]
Patch for landing
(In reply to Brent Fulgham from comment #3) > Comment on attachment 409955 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=409955&action=review > > > Source/WebCore/ChangeLog:21 > > + Changed the default setting for m_storageAccessScope. > > "to per-page." Fixed. > > Source/WebCore/page/Settings.yaml:705 > > + initial: true > > Does this setting only exist to support test cases? I wonder if we should > have some place to collect these kinds of settings (not relevant to this > patch today, just an idea). It is only for test cases, yes. Also, thanks for the review, Brent! Committed r267817: <https://trac.webkit.org/changeset/267817> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410185 [details]. It looks like the changes in https://trac.webkit.org/changeset/267817/webkit broke these two tests http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-without-user-interaction.html http/tests/storageAccess/request-storage-access-cross-origin-sandboxed-iframe-without-allow-token.html History: https://results.webkit.org/?suite=layout-tests&suite=layout-tests&test=http%2Ftests%2FstorageAccess%2Frequest-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-without-user-interaction.html&test=http%2Ftests%2FstorageAccess%2Frequest-storage-access-cross-origin-sandboxed-iframe-without-allow-token.html Diffs --- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-without-user-interaction-expected.txt +++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/storageAccess/request-and-grant-access-cross-origin-sandboxed-iframe-from-prevalent-domain-without-user-interaction-actual.txt @@ -3,8 +3,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS Storage access was denied. document.cookie == , cookies seen server-side == "No cookies" +FAIL FAIL Storage access was granted and requestStorageAccessResolved was not granted but should not have been granted. document.cookie == PASS successfullyParsed is true +Some tests failed. TEST COMPLETE --- /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/storageAccess/request-storage-access-cross-origin-sandboxed-iframe-without-allow-token-expected.txt +++ /Volumes/Data/slave/catalina-release-tests-wk2/build/layout-test-results/http/tests/storageAccess/request-storage-access-cross-origin-sandboxed-iframe-without-allow-token-actual.txt @@ -3,8 +3,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS Storage access was denied. document.cookie == , cookies seen server-side == "No cookies" +FAIL Storage access was granted and requestStorageAccessResolved was not granted but should not have been granted. document.cookie == PASS successfullyParsed is true +Some tests failed. TEST COMPLETE All tests passed on my system and the previous patch was all green on EWS. You can revert my patch if you want. I’ll investigate. I can reproduce the failures when not running with the -f flag. Reverted r267817 for reason: Broke two tests on Mac Committed r267897: <https://trac.webkit.org/changeset/267897> Created attachment 410500 [details]
Patch
Comment on attachment 410500 [details]
Patch
Brent, I fixed the two failing tests by adding WebPage::clearPageLevelStorageAccess() and calling it in WebProcess::clearResourceLoadStatistics(). Your name is still as reviewer in the change log. If it looks good to you, please cq+. Thanks!
Committed r267973: <https://trac.webkit.org/changeset/267973> All reviewed patches have been landed. Closing bug and clearing flags on attachment 410500 [details]. |