Bug 72234

Created attachment 114872 [details] Patch

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.

Created attachment 114910 [details] Patch

(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.

Comment on attachment 114910 [details] Patch ok

Thankyou Tamura-san

Comment on attachment 114910 [details] Patch Clearing flags on attachment: 114910 Committed r100220: <http://trac.webkit.org/changeset/100220>

All reviewed patches have been landed. Closing bug.