Change the implementations of ResourceLoadStatistics code in NetworkStorageSession to use Async message passing, so that more of the code is autogenerated. This should make test runs more consistent, and should reduce the possibility of bookkeeping errors in the message handling implementations.
<rdar://problem/47433290>
Created attachment 359803 [details] Patch
Created attachment 359805 [details] Patch
Comment on attachment 359805 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359805&action=review > Source/WebKit/NetworkProcess/Classifier/ShouldGrandfatherStatistics.h:40 > +template<> struct EnumTraits<WebKit::ShouldGrandfatherStatistics> { Fun fact: enum classes with bool as the underlying type don't need an EnumTraits because the decoder knows it can only have two values.
Looks like the patch is still out-of-sync with ToT. Uploading a newly-rebased version.
Created attachment 359816 [details] Patch
Created attachment 359844 [details] Patch
Comment on attachment 359844 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359844&action=review > Source/WebKit/NetworkProcess/Classifier/ShouldGrandfatherStatistics.h:40 > +template<> struct EnumTraits<WebKit::ShouldGrandfatherStatistics> { Still not needed. > Source/WebKit/NetworkProcess/NetworkProcess.cpp:604 > + resourceLoadStatistics->dumpResourceLoadStatistics(WTFMove(completionHandler)); It seems like we should call the completion handler in the other cases, too. > Source/WebKit/NetworkProcess/NetworkProcess.cpp:723 > ASSERT_NOT_REACHED(); ditto > Source/WebKit/NetworkProcess/NetworkProcess.cpp:869 > ASSERT_NOT_REACHED(); ditto > Source/WebKit/NetworkProcess/NetworkProcess.cpp:887 > ASSERT_NOT_REACHED(); ditto > Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:409 > +#if ENABLE(RESOURCE_LOAD_STATISTICS) Should we just call the callback with the context if the feature is not enabled?
Created attachment 359906 [details] Patch
Comment on attachment 359906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359906&action=review > Source/WebKit/NetworkProcess/NetworkProcess.cpp:931 > ASSERT_NOT_REACHED(); more completion handler calling
Comment on attachment 359906 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359906&action=review >> Source/WebKit/NetworkProcess/NetworkProcess.cpp:931 >> ASSERT_NOT_REACHED(); > > more completion handler calling Even with ASSERT_NOT_REACHED? I guess that's debug-only.
Created attachment 359915 [details] Patch
Created attachment 359916 [details] Patch
Comment on attachment 359916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359916&action=review > Source/WebKit/NetworkProcess/Classifier/ShouldGrandfatherStatistics.h:34 > + extra space
(In reply to Alex Christensen from comment #14) > Comment on attachment 359916 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=359916&action=review > > > Source/WebKit/NetworkProcess/Classifier/ShouldGrandfatherStatistics.h:34 > > + > > extra space Doh! Fixed.
Committed r240360: <https://trac.webkit.org/changeset/240360>
I found that https://trac.webkit.org/changeset/240360/webkit caused this test http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2FresourceLoadStatistics%2Fcapped-lifetime-for-cookie-set-in-js.html to become a flakey failure on wk2. History: http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2FresourceLoadStatistics%2Fcapped-lifetime-for-cookie-set-in-js.html reproduced with: run-webkit-tests --root testbuild-240360 http/tests/resourceLoadStatistics/capped-lifetime-for-cookie-set-in-js.html --iterations 500 -f r240360 reproduces the failures while r240359 has no failures occur.
I am also very certain that http/tests/resourceLoadStatistics/cap-cache-max-age-for-prevalent-resource.html has become a flaky failure due to https://trac.webkit.org/changeset/240360/webkit Tracking my investigation in https://bugs.webkit.org/show_bug.cgi?id=193752