Make Jpeg decoding more fault resistant.

src/images/SkImageDecoder_libjpeg.cpp

Index: src/images/SkImageDecoder_libjpeg.cpp
diff --git a/src/images/SkImageDecoder_libjpeg.cpp b/src/images/SkImageDecoder_libjpeg.cpp
index d50e4eb015689335b3910c97f58104bb41dc14c7..514578d8eb32cee3dbc237f9e254aef73c02de8b 100644
--- a/src/images/SkImageDecoder_libjpeg.cpp
+++ b/src/images/SkImageDecoder_libjpeg.cpp
@@ -284,13 +284,19 @@ static bool skip_src_rows_tile(jpeg_decompress_struct* cinfo,
 
 // This guy exists just to aid in debugging, as it allows debuggers to just
 // set a break-point in one place to see all error exists.
-static bool return_false(const jpeg_decompress_struct& cinfo,
-                         const SkBitmap& bm, const char msg[]) {
-#ifdef SK_DEBUG
+static void error_message(const jpeg_decompress_struct& cinfo,
+                          const SkBitmap& bm, const char msg[]) {
+#if defined(SK_DEBUG) && defined(SK_DEBUG_JPEG_ERRORS)

[scroggo, 2013/09/24 23:01:07]

Like I said in the bug, I think this is a bad idea. This will result in hiding
errors.

     SkDebugf("libjpeg error %d <%s> from %s [%d %d]\n", cinfo.err->msg_code,
              cinfo.err->jpeg_message_table[cinfo.err->msg_code], msg,
              bm.width(), bm.height());
 #endif
+}
+
+
+static bool return_false(const jpeg_decompress_struct& cinfo,
+                         const SkBitmap& bm, const char msg[]) {
+    error_message(cinfo, bm, msg);
     return false;   // must always return false
 }
 
@@ -555,7 +561,8 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
             // if row_count == 0, then we didn't get a scanline, so abort.
             // if we supported partial images, we might return true in this case
             if (0 == row_count) {
-                return return_false(cinfo, *bm, "read_scanlines");
+                error_message(cinfo, *bm, "read_scanlines");

[scroggo, 2013/09/24 23:01:07]

I think this should not print an error message at all, since we're going to
consider this a successful decode.

+                return true;

[scroggo, 2013/09/24 23:01:07]

Since we never initialized the memory, won't this result in garbage on all the
following scanlines?

             }
             if (this->shouldCancelDecode()) {
                 return return_false(cinfo, *bm, "shouldCancelDecode");
@@ -605,7 +612,8 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
         JSAMPLE* rowptr = (JSAMPLE*)srcRow;
         int row_count = jpeg_read_scanlines(&cinfo, &rowptr, 1);
         if (0 == row_count) {
-            return return_false(cinfo, *bm, "read_scanlines");
+            error_message(cinfo, *bm, "read_scanlines");
+            return true;
         }
         if (this->shouldCancelDecode()) {
             return return_false(cinfo, *bm, "shouldCancelDecode");
@@ -787,7 +795,8 @@ bool SkJPEGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {
             // if row_count == 0, then we didn't get a scanline, so abort.
             // if we supported partial images, we might return true in this case
             if (0 == rowCount) {
-                return return_false(*cinfo, bitmap, "read_scanlines");
+                error_message(*cinfo, bitmap, "read_scanlines");
+                return true;
             }
             if (this->shouldCancelDecode()) {
                 return return_false(*cinfo, bitmap, "shouldCancelDecode");
@@ -844,7 +853,8 @@ bool SkJPEGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {
         JSAMPLE* rowptr = (JSAMPLE*)srcRow;
         int row_count = jpeg_read_tile_scanline(cinfo, fImageIndex->huffmanIndex(), &rowptr);
         if (0 == row_count) {
-            return return_false(*cinfo, bitmap, "read_scanlines");
+            error_message(*cinfo, bitmap, "read_scanlines");
+            return true;
         }
         if (this->shouldCancelDecode()) {
             return return_false(*cinfo, bitmap, "shouldCancelDecode");