[webkit-changes] [109779] trunk

Mon, 05 Mar 2012 11:55:11 -0800

Title: [109779] trunk
Revision
109779
Author
commit-qu...@webkit.org
Date
2012-03-05 11:54:58 -0800 (Mon, 05 Mar 2012)

Log Message

Partially loaded JPEGs should have alpha channel
https://bugs.webkit.org/show_bug.cgi?id=78239

Patch by Sami Kyostila <skyos...@chromium.org> on 2012-03-05
Reviewed by Kenneth Russell.

Source/WebCore:

While a JPEG image is loading, the area outside the decoded region
should be fully transparent. Since currently all JPEG frames are marked
as opaque, a renderer respecting this flag will draw the partially
loaded image with garbage outside the valid image region.

Hence, a partially loaded JPEG image should be marked as having an alpha
channel while decoding is in progress. For performance reasons we mark
the image opaque after decoding has finished.

Graphics corruption caused by this bug was recently observed on
Chromium (http://code.google.com/p/chromium/issues/detail?id=113171). A
recent Skia change (r3036) changed SkBitmap::extractSubset() to produce
a bitmap with the same opaqueness flag as the parent. This meant that
the renderer was now seeing an opaque image from the JPEG decoder, and
drawing it appropriately resulted in garbage outside the decoded region.

Test: http/tests/incremental/partial-jpeg.html

* platform/image-decoders/jpeg/JPEGImageDecoder.cpp:
(WebCore::JPEGImageDecoder::outputScanlines):
(WebCore::JPEGImageDecoder::jpegComplete):

LayoutTests:

While a JPEG image is being loaded, the parts which have not been
decoded yet should show whatever is behind the image. This tests
verifies this by displaying a JPEG which never fully completes
loading. This is achieved by serving the JPEG from a PHP script that
strips the end of image marker (ff d9) from the data.

The test image is 32x32 pixels, so compresses to 4x4 JPEG MCU blocks.
The expected result is that the final row of MCU blocks (32x4 pixels)
never finishes loading due to the missing end of image marker and the
indicator div is shown through this area.

* http/tests/incremental/partial-jpeg-expected.png: Added.
* http/tests/incremental/partial-jpeg-expected.txt: Added.
* http/tests/incremental/partial-jpeg.html: Added.
* http/tests/incremental/resources/checkerboard.jpg: Added.
* http/tests/incremental/resources/partial-jpeg.php: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (109778 => 109779)


--- trunk/LayoutTests/ChangeLog	2012-03-05 19:50:05 UTC (rev 109778)
+++ trunk/LayoutTests/ChangeLog	2012-03-05 19:54:58 UTC (rev 109779)
@@ -1,3 +1,27 @@
+2012-03-05  Sami Kyostila  <skyos...@chromium.org>
+
+        Partially loaded JPEGs should have alpha channel
+        https://bugs.webkit.org/show_bug.cgi?id=78239
+
+        Reviewed by Kenneth Russell.
+
+        While a JPEG image is being loaded, the parts which have not been
+        decoded yet should show whatever is behind the image. This tests
+        verifies this by displaying a JPEG which never fully completes
+        loading. This is achieved by serving the JPEG from a PHP script that
+        strips the end of image marker (ff d9) from the data.
+
+        The test image is 32x32 pixels, so compresses to 4x4 JPEG MCU blocks.
+        The expected result is that the final row of MCU blocks (32x4 pixels)
+        never finishes loading due to the missing end of image marker and the
+        indicator div is shown through this area.
+
+        * http/tests/incremental/partial-jpeg-expected.png: Added.
+        * http/tests/incremental/partial-jpeg-expected.txt: Added.
+        * http/tests/incremental/partial-jpeg.html: Added.
+        * http/tests/incremental/resources/checkerboard.jpg: Added.
+        * http/tests/incremental/resources/partial-jpeg.php: Added.
+
 2012-03-05  Adam Klein  <ad...@chromium.org>
 
         Unreviewed test expectations update: widen slowness of jquery/offset.html.

Added: trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.png (0 => 109779)


--- trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.png	                        (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.png	2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,6 @@
+\x89PNG
+
+
+IHDR X')tEXtchecksumd44cebe6673df4032a998cb607740338\\x95\x8F\xEDIDATx\x9C\xED\xDC\xDDi\xC30@ѨdG\xD1m\xDAm\xBCQ\xC6Q\xB0()\x87\x94s\xF0\x93%\xFC\xF3v\xF94\xE6\x9C7:\xAF\xFE\x80\xFFF`\xC4@L`\xC4@L`\xC4@\xEC\xBE[8\x8E\xE3\xF4\xFE\xEB\xF4\xFEZ\xE3\xA9\xFDs~\xFE\xF6mo\xC9 &\xB0b &\xB0b &\xB0b \xB6
+\xAC1\xD6\xE9\xB5\xD68\xBD\x9E\xDD\xE5O\\xC9 &\xB0b &\xB0b &\xB0b \xB6
+\xAC꼫\xDD\xFE+\xE0J&X1\x81X1\x81X1\x81X\xB1\xFBna|\x8F\xF3\x85\xAF\xDD\xFE̓6\xFBo\x8F\xB9{5\xC0[3\xC1\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88m\xCF\xC1\x9AΩ\xF8,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\x98\xC0\x88	,\x80\xD8 \x8Eqsw[\xD1IEND\xAEB`\x82
\ No newline at end of file

Added: trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.txt (0 => 109779)


--- trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/partial-jpeg-expected.txt	2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,9 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x8
+  RenderBlock {HTML} at (0,0) size 800x8
+    RenderBody {BODY} at (8,8) size 784x0
+layer at (8,8) size 32x32
+  RenderBlock (positioned) {DIV} at (8,8) size 32x32 [bgcolor=#008000]
+layer at (8,8) size 32x32
+  RenderImage {IMG} at (8,8) size 32x32

Added: trunk/LayoutTests/http/tests/incremental/partial-jpeg.html (0 => 109779)


--- trunk/LayoutTests/http/tests/incremental/partial-jpeg.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/partial-jpeg.html	2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <script type="text/_javascript_">
+    function forceDisplay() {
+      window.layoutTestController.display();
+      setTimeout(stopLoading, 0);
+    }
+
+    function stopLoading() {
+      window.stop();
+      window.layoutTestController.notifyDone();
+    }
+
+    if (window.layoutTestController) {
+      window.layoutTestController.waitUntilDone();
+      setTimeout(forceDisplay, 0);
+    }
+  </script>
+
+  <style type="text/css">
+    .box {
+      width: 32px;
+      height: 32px;
+      position: absolute;
+    }
+
+    .indicator {
+      background: green;
+    }
+  </style>
+</head>
+
+<body>
+  <!-- The indicator box should be visible through unloaded parts of the image. -->
+  <div class="indicator box"></div>
+  <img class="box" src=""
+</body>
+</html>

Added: trunk/LayoutTests/http/tests/incremental/resources/checkerboard.jpg (0 => 109779)


--- trunk/LayoutTests/http/tests/incremental/resources/checkerboard.jpg	                        (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/resources/checkerboard.jpg	2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,3 @@
+\xFF\xD8\xFF\xE0JFIFHH\xFF\xDBC\xFF\xDBC\xFF\xC0  \xFF\xC4
+\xFF\xC4\xFF\xC4
+\xFF\xC4\xFF\xDA?\xDF\xC30;\xD7\xF0+\x9CA\xE5@;\xD7\xF0+\xA6c\x88 \xA8z\xFEs\x88 \xA8L\xC0\xEF_\xC0\xAEq\x95\xEF_\xC0\xAE\x99\x8E \x80r\xA0\xEB\xF8\xCE \x80r\xA1\xFF\xD9
\ No newline at end of file

Added: trunk/LayoutTests/http/tests/incremental/resources/partial-jpeg.php (0 => 109779)


--- trunk/LayoutTests/http/tests/incremental/resources/partial-jpeg.php	                        (rev 0)
+++ trunk/LayoutTests/http/tests/incremental/resources/partial-jpeg.php	2012-03-05 19:54:58 UTC (rev 109779)
@@ -0,0 +1,10 @@
+<?
+$file = "checkerboard.jpg";
+$size = filesize($file);
+$end_marker_size = 2; // Strip the end marker (ff d9)
+$contents = file_get_contents($file, false, NULL, 0, $size - $end_marker_size);
+header("Content-Type: image/jpeg");
+header("Content-Length: " . $size);
+flush();
+echo $contents;
+?>

Modified: trunk/Source/WebCore/ChangeLog (109778 => 109779)


--- trunk/Source/WebCore/ChangeLog	2012-03-05 19:50:05 UTC (rev 109778)
+++ trunk/Source/WebCore/ChangeLog	2012-03-05 19:54:58 UTC (rev 109779)
@@ -1,3 +1,32 @@
+2012-03-05  Sami Kyostila  <skyos...@chromium.org>
+
+        Partially loaded JPEGs should have alpha channel
+        https://bugs.webkit.org/show_bug.cgi?id=78239
+
+        Reviewed by Kenneth Russell.
+
+        While a JPEG image is loading, the area outside the decoded region
+        should be fully transparent. Since currently all JPEG frames are marked
+        as opaque, a renderer respecting this flag will draw the partially
+        loaded image with garbage outside the valid image region.
+
+        Hence, a partially loaded JPEG image should be marked as having an alpha
+        channel while decoding is in progress. For performance reasons we mark
+        the image opaque after decoding has finished.
+
+        Graphics corruption caused by this bug was recently observed on
+        Chromium (http://code.google.com/p/chromium/issues/detail?id=113171). A
+        recent Skia change (r3036) changed SkBitmap::extractSubset() to produce
+        a bitmap with the same opaqueness flag as the parent. This meant that
+        the renderer was now seeing an opaque image from the JPEG decoder, and
+        drawing it appropriately resulted in garbage outside the decoded region.
+
+        Test: http/tests/incremental/partial-jpeg.html
+
+        * platform/image-decoders/jpeg/JPEGImageDecoder.cpp:
+        (WebCore::JPEGImageDecoder::outputScanlines):
+        (WebCore::JPEGImageDecoder::jpegComplete):
+
 2012-03-05  James Robinson  <jam...@chromium.org>
 
         [chromium] Initialize CCOverdrawCounts struct to zero

Modified: trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp (109778 => 109779)


--- trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp	2012-03-05 19:50:05 UTC (rev 109778)
+++ trunk/Source/WebCore/platform/image-decoders/jpeg/JPEGImageDecoder.cpp	2012-03-05 19:54:58 UTC (rev 109779)
@@ -501,7 +501,9 @@
         if (!buffer.setSize(scaledSize().width(), scaledSize().height()))
             return setFailed();
         buffer.setStatus(ImageFrame::FramePartial);
-        buffer.setHasAlpha(false);
+        // The buffer is transparent outside the decoded area while the image is
+        // loading. The completed image will be marked fully opaque in jpegComplete().
+        buffer.setHasAlpha(true);
         buffer.setColorProfile(m_colorProfile);
 
         // For JPEGs, the frame always fills the entire image.
@@ -569,7 +571,9 @@
 
     // Hand back an appropriately sized buffer, even if the image ended up being
     // empty.
-    m_frameBufferCache[0].setStatus(ImageFrame::FrameComplete);
+    ImageFrame& buffer = m_frameBufferCache[0];
+    buffer.setStatus(ImageFrame::FrameComplete);
+    buffer.setHasAlpha(false);
 }
 
 void JPEGImageDecoder::decode(bool onlySize)

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to