[webkit-reviews] review granted: [Bug 225010] PCM: debug mode should send the second report on a 10 second delay after the first : [Attachment 426968] Patch

[bugzilla-daemon at webkit.org]

John Wilander <wilander at apple.com> has granted katherine_cheney at apple.com's
request for review:
Bug 225010: PCM: debug mode should send the second report on a 10 second delay
after the first
https://bugs.webkit.org/show_bug.cgi?id=225010

Attachment 426968: Patch

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




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

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

r=me with small comments. I think it's fine to land this without full EWS
feedback since it only affects the opt-in debug mode setting. It could
theoretically break regression tests but that is very unlikely.

> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:401
> +		       // mode is enabled, this should be every 10 seconds for
easy testing.

I would avoid stating the 10 seconds since it's a constant that may change,
making the comment obsolete. You can say "… this should be much shorter for
easy testing."

> Source/WebKit/NetworkProcess/PrivateClickMeasurementManager.cpp:413
> +		   // Or, if debug mode is enabled, we should send the second
report on a 10 second delay for easy testing.

Ditto. I suggest "… we should send the second report on a much shorter delay
for easy testing."