Bug 213553 – [ iOS macOS ] http/tests/resourceLoadStatistics/grandfathering-database.html is a rare flaky timeout

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jacob Uphoff]

http/tests/resourceLoadStatistics/grandfathering-database.html

This test has been a rare flaky timeout since it started running on iOS and then shortly after for macOS. The timeouts appear on Release WK2. It times out more often on iOS and internally for macOS.

History:

https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2FresourceLoadStatistics%2Fgrandfathering-database.html&platform=ios&platform=mac&limit=50000

Diff:

@@ -1,5 +1,3 @@
-PASS successfullyParsed is true
+#PID UNRESPONSIVE - WebKitTestRunner (pid 14028)
+FAIL: Timed out waiting for notifyDone to be called
 
-TEST COMPLETE
-PASS Grandfathered cookie was not purged.
-

[Radar WebKit Bug Importer]

<rdar://problem/64696432>

[Jacob Uphoff]

Set expectations here: https://trac.webkit.org/changeset/263452/webkit

[Alexey Proskuryakov]

*** Bug 213552 has been marked as a duplicate of this bug. ***

[Kate Cheney]

Created attachment 404294 [details]
Patch

[Darin Adler]

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?

[Kate Cheney]

(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!

[Kate Cheney]

Created attachment 404731 [details]
Patch for landing

[EWS]

Committed r264609: <https://trac.webkit.org/changeset/264609>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 404731 [details].