Summary: | [ iOS macOS ] http/tests/resourceLoadStatistics/grandfathering-database.html is a rare flaky timeout | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jacob Uphoff <jacob_uphoff> | ||||||
Component: | New Bugs | Assignee: | Kate Cheney <katherine_cheney> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, cdumez, darin, katherine_cheney, webkit-bot-watchers-bugzilla, webkit-bug-importer, wilander | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Jacob Uphoff
2020-06-24 05:02:52 PDT
Set expectations here: https://trac.webkit.org/changeset/263452/webkit *** Bug 213552 has been marked as a duplicate of this bug. *** Created attachment 404294 [details]
Patch
Comment on attachment 404294 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404294&action=review > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:2088 > + bool notifyPagesWhenDataRecordsWereModified = !!callback; > + // Setting a callback implies we expect to receive callbacks. So register for them. > + setStatisticsNotifyPagesWhenDataRecordsWereScanned(notifyPagesWhenDataRecordsWereModified); I understand turning this on if we were passed a callback. I don’t fully understand turning it off if we were passed null. Are we the one and only client? Does this need to be reset between tests? (In reply to Darin Adler from comment #5) > Comment on attachment 404294 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=404294&action=review > > > Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp:2088 > > + bool notifyPagesWhenDataRecordsWereModified = !!callback; > > + // Setting a callback implies we expect to receive callbacks. So register for them. > > + setStatisticsNotifyPagesWhenDataRecordsWereScanned(notifyPagesWhenDataRecordsWereModified); > > I understand turning this on if we were passed a callback. > > I don’t fully understand turning it off if we were passed null. Are we the > one and only client? Does this need to be reset between tests? Yes, this is reset between tests in WKWebsiteDataStoreStatisticsResetToConsistentState. This function doesn't make sense without a callback, but thinking more about it, I agree turning it off when passed null does not make sense. I think the better option is only calling TestRunner::setStatisticsNotifyPagesWhenDataRecordsWereScanned() if the callback isn't null, and doing nothing if it is null. I'll change that before landing. Thanks for the comments! Created attachment 404731 [details]
Patch for landing
Committed r264609: <https://trac.webkit.org/changeset/264609> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404731 [details]. |