[webkit-reviews] review granted: [Bug 175090] Resource Load Statistics: Report user interaction immediately, but only when needed : [Attachment 317038] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted John Wilander <wilander at apple.com>'s
request for review:
Bug 175090: Resource Load Statistics: Report user interaction immediately, but
only when needed
https://bugs.webkit.org/show_bug.cgi?id=175090

Attachment 317038: Patch

https://bugs.webkit.org/attachment.cgi?id=317038&action=review




--- Comment #10 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 317038
  --> https://bugs.webkit.org/attachment.cgi?id=317038
Patch

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

r=me with comments. Also, please take a look at the iOS failure.

> Source/WebCore/loader/ResourceLoadObserver.cpp:284
> +    notificationTimerFired();

I think this would look better if we called it notifyObserver(). Maybe the call
to m_notificationTimer.stop() could be moved inside notifyObserver() too.

> Source/WebCore/loader/ResourceLoadObserver.cpp:337
> +    m_lastReportedUserInteractionMap.clear();

I think it would be nice to stop m_notificationTimer too.

>
LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-in-cross
-origin-sub-frame.html:28
>      if (internals) {

Should drop the curly brackets.

>
LayoutTests/http/tests/loading/resourceLoadStatistics/user-interaction-only-rep
orted-once-within-short-period-of-time.html:57
> +	   if (internals) {

Should drop the curly brackets.