Report error on failure to create SkCodec

Update NewFromStream to report an error on failure to create an
SkCodec, so that a client can distinguish between
- not enough data
- invalid data

In Chromium, this will allow blink::ImageDecoder to call SetFailed if
the stream is invalid early and we never create an SkCodec. Without
this, ImageDecoder will keep trying to create an SkCodec when it
receives more data.

Change-Id: I4f505c56d91c982be36a828fd0f7db17b1596588
Reviewed-on: https://skia-review.googlesource.com/22642
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Reviewed-by: Chris Blume <cblume@chromium.org>

1 files changed, 39 insertions, 44 deletions

diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 3e224c13a9..70397f656f 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -67,16 +67,16 @@ bool SkBmpCodec::IsBmp(const void* buffer, size_t bytesRead) {
  * Creates a bmp decoder
  * Reads enough of the stream to determine the image format
  */
-SkCodec* SkBmpCodec::NewFromStream(SkStream* stream) {
-    return SkBmpCodec::NewFromStream(stream, false);
+SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, Result* result) {
+    return SkBmpCodec::NewFromStream(stream, result, false);
 }
 
 /*
  * Creates a bmp decoder for a bmp embedded in ico
  * Reads enough of the stream to determine the image format
  */
-SkCodec* SkBmpCodec::NewFromIco(SkStream* stream) {
-    return SkBmpCodec::NewFromStream(stream, true);
+SkCodec* SkBmpCodec::NewFromIco(SkStream* stream, Result* result) {
+    return SkBmpCodec::NewFromStream(stream, result, true);
 }
 
 // Header size constants
@@ -132,14 +132,7 @@ static BmpHeaderType get_header_type(size_t infoBytes) {
     }
 }
 
-/*
- * Read enough of the stream to initialize the SkBmpCodec. Returns a bool
- * representing success or failure. If it returned true, and codecOut was
- * not nullptr, it will be set to a new SkBmpCodec.
- * If codecOut is set to a new SkCodec, it will take ownership of the stream.
- * Otherwise, the stream will not be deleted.
- */
-bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
+SkCodec::Result SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
         std::unique_ptr<SkCodec>* codecOut) {
     // The total bytes in the bmp file
     // We only need to use this value for RLE decoding, so we will only
@@ -157,14 +150,14 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
         if (stream->read(hBuffer, kBmpHeaderBytesPlusFour) !=
                 kBmpHeaderBytesPlusFour) {
             SkCodecPrintf("Error: unable to read first bitmap header.\n");
-            return false;
+            return kIncompleteInput;
         }
 
         totalBytes = get_int(hBuffer, 2);
         offset = get_int(hBuffer, 10);
         if (offset < kBmpHeaderBytes + kBmpOS2V1Bytes) {
             SkCodecPrintf("Error: invalid starting location for pixel data\n");
-            return false;
+            return kInvalidInput;
         }
 
         // The size of the second (info) header in bytes
@@ -173,7 +166,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
         infoBytes = get_int(hBuffer, 14);
         if (infoBytes < kBmpOS2V1Bytes) {
             SkCodecPrintf("Error: invalid second header size.\n");
-            return false;
+            return kInvalidInput;
         }
     } else {
         // This value is only used by RLE compression.  Bmp in Ico files do not
@@ -190,19 +183,19 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
         uint8_t hBuffer[4];
         if (stream->read(hBuffer, 4) != 4) {
             SkCodecPrintf("Error: unable to read size of second bitmap header.\n");
-            return false;
+            return kIncompleteInput;
         }
         infoBytes = get_int(hBuffer, 0);
         if (infoBytes < kBmpOS2V1Bytes) {
             SkCodecPrintf("Error: invalid second header size.\n");
-            return false;
+            return kInvalidInput;
         }
     }
 
     // Determine image information depending on second header format
     const BmpHeaderType headerType = get_header_type(infoBytes);
     if (kUnknown_BmpHeaderType == headerType) {
-        return false;
+        return kInvalidInput;
     }
 
     // We already read the first four bytes of the info header to get the size
@@ -212,7 +205,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
     std::unique_ptr<uint8_t[]> iBuffer(new uint8_t[infoBytesRemaining]);
     if (stream->read(iBuffer.get(), infoBytesRemaining) != infoBytesRemaining) {
         SkCodecPrintf("Error: unable to read second bitmap header.\n");
-        return false;
+        return kIncompleteInput;
     }
 
     // The number of bits used per pixel in the pixel data
@@ -268,7 +261,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
         case kUnknown_BmpHeaderType:
             // We'll exit above in this case.
             SkASSERT(false);
-            return false;
+            return kInvalidInput;
     }
 
     // Check for valid dimensions from header
@@ -287,7 +280,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
     constexpr int kMaxDim = 1 << 16;
     if (width <= 0 || height <= 0 || width >= kMaxDim || height >= kMaxDim) {
         SkCodecPrintf("Error: invalid bitmap dimensions.\n");
-        return false;
+        return kInvalidInput;
     }
 
     // Create mask struct
@@ -337,7 +330,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                     uint8_t buffer[kBmpMaskBytes];
                     if (stream->read(buffer, kBmpMaskBytes) != kBmpMaskBytes) {
                         SkCodecPrintf("Error: unable to read bit inputMasks.\n");
-                        return false;
+                        return kIncompleteInput;
                     }
                     maskBytes = kBmpMaskBytes;
                     inputMasks.red = get_int(buffer, 0);
@@ -384,10 +377,10 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                     //       in chromium.  I have not come across a test case
                     //       that uses this format.
                     SkCodecPrintf("Error: huffman format unsupported.\n");
-                    return false;
+                    return kUnimplemented;
                 default:
                    SkCodecPrintf("Error: invalid bmp bit masks header.\n");
-                   return false;
+                   return kInvalidInput;
             }
             break;
         case kJpeg_BmpCompressionMethod:
@@ -401,16 +394,16 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
             //       It is unsupported in the previous version and
             //       in chromium.  I think it is used mostly for printers.
             SkCodecPrintf("Error: compression format not supported.\n");
-            return false;
+            return kUnimplemented;
         case kCMYK_BmpCompressionMethod:
         case kCMYK8BitRLE_BmpCompressionMethod:
         case kCMYK4BitRLE_BmpCompressionMethod:
             // TODO: Same as above.
             SkCodecPrintf("Error: CMYK not supported for bitmap decoding.\n");
-            return false;
+            return kUnimplemented;
         default:
             SkCodecPrintf("Error: invalid format for bitmap decoding.\n");
-            return false;
+            return kInvalidInput;
     }
     iBuffer.reset();
 
@@ -421,7 +414,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
         //                 Seems like we can just assume that the offset is zero and try to decode?
         //                 Maybe we don't want to try to decode corrupt images?
         SkCodecPrintf("Error: pixel data offset less than header size.\n");
-        return false;
+        return kInvalidInput;
     }
 
 
@@ -474,7 +467,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                     break;
                 default:
                     SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
-                    return false;
+                    return kInvalidInput;
             }
 
             if (codecOut) {
@@ -486,16 +479,17 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                 codecOut->reset(new SkBmpStandardCodec(width, height, info, stream, bitsPerPixel,
                                                        numColors, bytesPerColor, offset - bytesRead,
                                                        rowOrder, isOpaque, inIco));
-                return static_cast<SkBmpStandardCodec*>(codecOut->get())->didCreateSrcBuffer();
+                return static_cast<SkBmpStandardCodec*>(codecOut->get())->didCreateSrcBuffer()
+                        ? kSuccess : kInvalidInput;
             }
-            return true;
+            return kSuccess;
         }
 
         case kBitMask_BmpInputFormat: {
             // Bmp-in-Ico must be standard mode
             if (inIco) {
                 SkCodecPrintf("Error: Icos may not use bit mask format.\n");
-                return false;
+                return kInvalidInput;
             }
 
             switch (bitsPerPixel) {
@@ -505,7 +499,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                     break;
                 default:
                     SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
-                    return false;
+                    return kInvalidInput;
             }
 
             // Skip to the start of the pixel array.
@@ -513,7 +507,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
             // in bit mask mode.
             if (stream->skip(offset - bytesRead) != offset - bytesRead) {
                 SkCodecPrintf("Error: unable to skip to image data.\n");
-                return false;
+                return kIncompleteInput;
             }
 
             if (codecOut) {
@@ -521,7 +515,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                 std::unique_ptr<SkMasks> masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
                 if (nullptr == masks) {
                     SkCodecPrintf("Error: invalid input masks.\n");
-                    return false;
+                    return kInvalidInput;
                 }
 
                 // Masked bmps are not a great fit for SkEncodedInfo, since they have
@@ -540,9 +534,10 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                 const SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
                 codecOut->reset(new SkBmpMaskCodec(width, height, info, stream, bitsPerPixel,
                                                    masks.release(), rowOrder));
-                return static_cast<SkBmpMaskCodec*>(codecOut->get())->didCreateSrcBuffer();
+                return static_cast<SkBmpMaskCodec*>(codecOut->get())->didCreateSrcBuffer()
+                        ? kSuccess : kInvalidInput;
             }
-            return true;
+            return kSuccess;
         }
 
         case kRLE_BmpInputFormat: {
@@ -552,7 +547,7 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
             // Check for a valid number of total bytes when in RLE mode
             if (totalBytes <= offset) {
                 SkCodecPrintf("Error: RLE requires valid input size.\n");
-                return false;
+                return kInvalidInput;
             }
 
             // Bmp-in-Ico must be standard mode
@@ -572,11 +567,11 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
                                                   numColors, bytesPerColor, offset - bytesRead,
                                                   rowOrder));
             }
-            return true;
+            return kSuccess;
         }
         default:
             SkASSERT(false);
-            return false;
+            return kInvalidInput;
     }
 }
 
@@ -584,16 +579,16 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco,
  * Creates a bmp decoder
  * Reads enough of the stream to determine the image format
  */
-SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, bool inIco) {
+SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, Result* result, bool inIco) {
     std::unique_ptr<SkStream> streamDeleter(stream);
     std::unique_ptr<SkCodec> codec;
-    bool success = ReadHeader(stream, inIco, &codec);
+    *result = ReadHeader(stream, inIco, &codec);
     if (codec) {
         // codec has taken ownership of stream, so we do not need to
         // delete it.
         streamDeleter.release();
     }
-    return success ? codec.release() : nullptr;
+    return kSuccess == *result ? codec.release() : nullptr;
 }
 
 SkBmpCodec::SkBmpCodec(int width, int height, const SkEncodedInfo& info, SkStream* stream,
@@ -606,7 +601,7 @@ SkBmpCodec::SkBmpCodec(int width, int height, const SkEncodedInfo& info, SkStrea
 {}
 
 bool SkBmpCodec::onRewind() {
-    return SkBmpCodec::ReadHeader(this->stream(), this->inIco(), nullptr);
+    return SkBmpCodec::ReadHeader(this->stream(), this->inIco(), nullptr) == kSuccess;
 }
 
 int32_t SkBmpCodec::getDstRow(int32_t y, int32_t height) const {