Summary: | Layout Test http/tests/storageAccess/request-and-grant-access-then-detach-should-not-have-access.html is flaky | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryan Haddad <ryanhaddad> | ||||||||
Component: | New Bugs | Assignee: | John Wilander <wilander> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, cdumez, commit-queue, jlewis3, ryanhaddad, sroberts, tsavell, webkit-bug-importer, wilander | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=183616 | ||||||||||
Attachments: |
|
Description
Ryan Haddad
2018-01-12 10:59:53 PST
*** Bug 181605 has been marked as a duplicate of this bug. *** Marked as flaky: https://trac.webkit.org/changeset/227574/webkit Created attachment 332832 [details]
Patch
Comment on attachment 332832 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=332832&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:91 > +- (void)_hasStorageAccessEntry:(NSString *)host completionHandler:(void (^)(BOOL))completionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); Give this a WKWebView* from which you get the pageID instead of host. Have the return value be an NSDictionary<_WKFrameHandle*, NSString*> to get the storage access entries for this WKWebView. > Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:180 > + boolean hasStorageAccessEntry(DOMString hostName); What if we had this return a promise so we don't need to add more synchronous stuff? I think the promise should give you an array of partitions instead of just a bool. (In reply to Alex Christensen from comment #5) > Comment on attachment 332832 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=332832&action=review > > > Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:91 > > +- (void)_hasStorageAccessEntry:(NSString *)host completionHandler:(void (^)(BOOL))completionHandler WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA)); > > Give this a WKWebView* from which you get the pageID instead of host. Have > the return value be an NSDictionary<_WKFrameHandle*, NSString*> to get the > storage access entries for this WKWebView. > > > Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:180 > > + boolean hasStorageAccessEntry(DOMString hostName); > > What if we had this return a promise so we don't need to add more > synchronous stuff? > I think the promise should give you an array of partitions instead of just a > bool. Note that I do not believe we support promises in TestRunner IDL (which uses its own bindings generator). We do support callbacks though. Created attachment 333024 [details]
Patch
Comment on attachment 333024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333024&action=review > Tools/WebKitTestRunner/TestController.cpp:2674 > +// auto* dataStore = WKContextGetWebsiteDataStore(platformContext()); Please don't land this commented-out code. Created attachment 333098 [details]
Patch for landing
(In reply to Alex Christensen from comment #8) > Comment on attachment 333024 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333024&action=review > > > Tools/WebKitTestRunner/TestController.cpp:2674 > > +// auto* dataStore = WKContextGetWebsiteDataStore(platformContext()); > > Please don't land this commented-out code. It was intended as a friendly gesture to the Linux maintainers. Will remove. Thanks for the review! Comment on attachment 333098 [details] Patch for landing Clearing flags on attachment: 333098 Committed r228109: <https://trac.webkit.org/changeset/228109> All reviewed patches have been landed. Closing bug. The commit doesn't seem to have fixed the flakiness of the test: https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r228129%20(2753)/results.html https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/2753 I will add additional logging to the test output to see if we can understand why the bots see these failures. Test is still flaky, also being flaky on Mojave WK2 and Debug. https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2FstorageAccess%2Frequest-and-grant-access-then-detach-should-not-have-access.html |