[webkit-reviews] review denied: [Bug 72234] Test JPEG image decoding RGB pixel endianess : [Attachment 114872] Patch

[bugzilla-daemon at webkit.org]

Kent Tamura <tkent at chromium.org> has denied noel gordon
<noel.gordon at gmail.com>'s request for review:
Bug 72234: Test JPEG image decoding RGB pixel endianess
https://bugs.webkit.org/show_bug.cgi?id=72234

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

------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=114872&action=review


> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:9
> +<img src="resources/rgb-jpeg-red.jpg"   onload="test(this, [255,0,0,255])">
> +<img src="resources/rgb-jpeg-green.jpg" onload="test(this, [0,255,0,255])">
> +<img src="resources/rgb-jpeg-blue.jpg"  onload="test(this, [0,0,255,255])">
> +
> +<pre id="log">PASS</pre>

Because there are three tests, we had better show three 'PASS' in the test
result.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:13
> +    layoutTestController.waitUntilDone(), layoutTestController.dumpAsText();


No reason to use a comma operator here.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:24
> +	   r += data[i + 0], g += data[i + 1], b += data[i + 2], a += data[i +
3];

ditto.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:26
> +    return [ r / size, g / size, b / size, a / size ];

nit: spaces after ']' and before ']' are not needed.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:36
> +    if (tolerance = tolerance || 0, delta > tolerance)

The tolerance argument is always specified.  We don't need "tolerance =
tolerance || 0".
Also, there is no reason to use a comma operator here.

> LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:60
> +	   if (testImage(image, expect), ++loadedImages < 3)

No reason to use a comma operator here.