[webkit-reviews] review granted: [Bug 212445] Minimum user interaction time in ResourceLoadStatistics should handle the case of -1 : [Attachment 400406] Patch

[bugzilla-daemon at webkit.org]

John Wilander <wilander at apple.com> has granted katherine_cheney at apple.com's
request for review:
Bug 212445: Minimum user interaction time in ResourceLoadStatistics should
handle the case of -1
https://bugs.webkit.org/show_bug.cgi?id=212445

Attachment 400406: Patch

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




--- Comment #4 from John Wilander <wilander at apple.com> ---
Comment on attachment 400406
  --> https://bugs.webkit.org/attachment.cgi?id=400406
Patch

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

Looks good! See comment on change log.

>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:2544
> +    if (statistic.mostRecentUserInteractionTime.secondsSinceEpoch().value()
<= 0)

Good that you include 0 here.

>
Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsDatabaseStore.cpp
:2596
> +    if (!parameters().isRunningTest && now - oldestUserInteraction <
parameters().minimumTimeBetweenDataRecordsRemoval)

��

> LayoutTests/ChangeLog:7
> +	   Reviewed by NOBODY (OOPS!).

Please add a comment on that the case of data deletion are already tested and
that these new tests only need to check for the absence of deletion. Also add a
comment on which test case(s) started failing once you corrected the issue, if
any.