[webkit-reviews] review granted: [Bug 235325] Add a data URL resource loader performance test : [Attachment 449412] Patch

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Peng Liu
<peng.liu6 at apple.com>'s request for review:
Bug 235325: Add a data URL resource loader performance test
https://bugs.webkit.org/show_bug.cgi?id=235325

Attachment 449412: Patch

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




--- Comment #2 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 449412
  --> https://bugs.webkit.org/attachment.cgi?id=449412
Patch

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

> LayoutTests/media/video-src-blob-perf.html:19
> +			   failTest("Loading a data URL takes too long time.");

It might be helpful to log the multiple when the test fails, to help diagnose
failures in case the test is flaky in some situations.

> LayoutTests/media/video-src-blob-perf.html:43
> +		   video1.src = url;
> +		   start1 = Date.now();

It might be more accurate to set the source of both elements after the data url
has been created.

> LayoutTests/media/video-src-blob-perf.html:60
> +		   setTimeout(() => failTest("Cannot complete the test in 5
seconds."), 5000);

Even normal tests sometimes take more than five seconds on a heavily loaded
bot, so I would increase the timeout significantly (10 sec?)