[webkit-reviews] review denied: [Bug 96239] Add partial load test for JPEG images : [Attachment 163052] Patch

[bugzilla-daemon at webkit.org]

Yuta Kitamura <yutak at chromium.org> has denied noel gordon
<noel.gordon at gmail.com>'s request for review:
Bug 96239: Add partial load test for JPEG images
https://bugs.webkit.org/show_bug.cgi?id=96239

Attachment 163052: Patch
https://bugs.webkit.org/attachment.cgi?id=163052&action=review

------- Additional Comments from Yuta Kitamura <yutak at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=163052&action=review


Could you elaborate on the motivation of this test? Were there any regressions
in the past? Do you have any reference to the specification? Are there any
other tests that cover this behavior?

And I'm a little concerned about the license of the picture.

r- for a style issue mentioned in in-line comments.

> LayoutTests/http/tests/images/jpeg-partial-load.html:20
> +    if (window.testRunner) testRunner.notifyDone();

We usually don't fold "if" expression into one line.

See
<http://www.webkit.org/coding/coding-style.html#linebreaking-multiple-statement
s>.

> LayoutTests/http/tests/images/jpeg-partial-load.html:26
> +    setTimeout(testDone, 500);

The use of setTimeout() like this can be the source of test flakiness, so it's
recommended to use other reliable notification method whenever possible. I'm
not sure if there's a workaround in this case, though.