Bug 72234 – Test JPEG image decoding RGB pixel endianess

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[noel gordon]

Test JPEG image decoding RGB pixel endianess

[noel gordon]

Created attachment 114872 [details]
Patch

[Kent Tamura]

Comment on attachment 114872 [details]
Patch

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.

[noel gordon]

Created attachment 114910 [details]
Patch

[noel gordon]

(In reply to comment #2)
> (From update of attachment 114872 [details])
> 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.

We shown up to 3 failures should they happen, sufficient? 
 
> > LayoutTests/fast/images/rgb-jpeg-endian-pixels.html:13
> > +    layoutTestController.waitUntilDone(), layoutTestController.dumpAsText();
> 
> No reason to use a comma operator here.

One day layoutTestController will return its |this| context.  Removed.
 
> > 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.

All done, and the others you mentioned.

[Kent Tamura]

Comment on attachment 114910 [details]
Patch

ok

[noel gordon]

Thankyou Tamura-san

[WebKit Review Bot]

Comment on attachment 114910 [details]
Patch

Clearing flags on attachment: 114910

Committed r100220: <http://trac.webkit.org/changeset/100220>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.