Bug 172730

Summary: Make ResourceLoadStatistics testing more reliable
Product: WebKit Reporter: Brent Fulgham <bfulgham>
Component: Tools / TestsAssignee: Brent Fulgham <bfulgham>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, ap, bfulgham, jlewis3, lforschler, ryanhaddad, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch aestes: review+

Description Brent Fulgham 2017-05-30 14:03:00 PDT
The tests for ResourceLoadStatistics are inherently flaky, because they expose the algorithm to various inputs, then immediately attempts to read back state even though the statistics are being processed in a background queue.

The test harness needs to be revised to work more like the scrolling tests, where a callback is registered to be called when the work is complete.
Comment 1 Brent Fulgham 2017-05-30 14:03:26 PDT
<rdar://problem/32028373>
Comment 2 Brent Fulgham 2017-05-30 22:37:49 PDT
Created attachment 311574 [details]
Patch
Comment 3 Andy Estes 2017-05-31 09:21:28 PDT
Comment on attachment 311574 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311574&action=review

> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:146
> +            RunLoop::main().dispatch([this, protectedThis = makeRef(*this)] () mutable {
> +                WebProcessProxy::notifyPageStatisticsAndDataRecordsProcessed();
> +            });

It's not obvious why `this` has to be captured here since the lambda calls a static function.
Comment 4 Brent Fulgham 2017-05-31 09:30:38 PDT
Comment on attachment 311574 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311574&action=review

>> Source/WebKit2/UIProcess/WebResourceLoadStatisticsStore.cpp:146
>> +            });
> 
> It's not obvious why `this` has to be captured here since the lambda calls a static function.

Good point. I'll remove that and double-check before landing.
Comment 5 Brent Fulgham 2017-05-31 09:58:55 PDT
Committed r217606: <http://trac.webkit.org/changeset/217606>