Get rid of leaks in SkCodec::NewFromStream.

SkCodec::NewFromStream claims to delete the passed in SkStream on
failure. This allows the caller to pass an SkStream to the function
and not worry about deleting it depending on the return value.

Most of our SkCodecs did not honor this contract though. Update them
to delete the stream on failure. Further, update SkCodec::NewFromStream
to delete the stream if it did not match any subclass, and delete the
SkCodec if we decided to return NULL because it was too big.

Add a test which tests streams which represent the beginnings of
supported format types but do not contain enough data to create an
SkCodec. The interesting part of the test is when we run it on ASAN,
which will report that we leaked something without the other changes.

BUG=skia:3257

Review URL: https://codereview.chromium.org/1058873006

6 files changed, 19 insertions, 8 deletions

diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 5604af8d3d..7bc9146041 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -32,8 +32,10 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
     if (!stream) {
         return NULL;
     }
+
+    SkAutoTDelete<SkStream> streamDeleter(stream);
     
-    SkCodec* codec = NULL;
+    SkAutoTDelete<SkCodec> codec(NULL);
     for (uint32_t i = 0; i < SK_ARRAY_COUNT(gDecoderProcs); i++) {
         DecoderProc proc = gDecoderProcs[i];
         const bool correctFormat = proc.IsFormat(stream);
@@ -41,7 +43,7 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
             return NULL;
         }
         if (correctFormat) {
-            codec = proc.NewFromStream(stream);
+            codec.reset(proc.NewFromStream(streamDeleter.detach()));
             break;
         }
     }
@@ -50,12 +52,11 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
     // This is about 4x smaller than a test image that takes a few minutes for
     // dm to decode and draw.
     const int32_t maxSize = 1 << 27;
-    if (codec != NULL &&
-            codec->getInfo().width() * codec->getInfo().height() > maxSize) {
+    if (codec && codec->getInfo().width() * codec->getInfo().height() > maxSize) {
         SkCodecPrintf("Error: Image size too large, cannot decode.\n");
         return NULL;
     } else {
-        return codec;
+        return codec.detach();
     }
 }
 
diff --git a/src/codec/SkCodec_libbmp.cpp b/src/codec/SkCodec_libbmp.cpp
index 75c5715bda..2b20faebad 100644
--- a/src/codec/SkCodec_libbmp.cpp
+++ b/src/codec/SkCodec_libbmp.cpp
@@ -108,6 +108,7 @@ SkCodec* SkBmpCodec::NewFromIco(SkStream* stream) {
  * Read enough of the stream to initialize the SkBmpCodec. Returns a bool
  * representing success or failure. If it returned true, and codecOut was
  * not NULL, it will be set to a new SkBmpCodec.
+ * Does *not* take ownership of the passed in SkStream.
  *
  */
 bool SkBmpCodec::ReadHeader(SkStream* stream, bool isIco, SkCodec** codecOut) {
@@ -496,8 +497,13 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool isIco, SkCodec** codecOut) {
  *
  */
 SkCodec* SkBmpCodec::NewFromStream(SkStream* stream, bool isIco) {
+    SkAutoTDelete<SkStream> streamDeleter(stream);
     SkCodec* codec = NULL;
     if (ReadHeader(stream, isIco, &codec)) {
+        // codec has taken ownership of stream, so we do not need to
+        // delete it.
+        SkASSERT(codec);
+        streamDeleter.detach();
         return codec;
     }
     return NULL;
diff --git a/src/codec/SkCodec_libbmp.h b/src/codec/SkCodec_libbmp.h
index 650259ad16..81b514e8f5 100644
--- a/src/codec/SkCodec_libbmp.h
+++ b/src/codec/SkCodec_libbmp.h
@@ -103,6 +103,7 @@ private:
      * Read enough of the stream to initialize the SkBmpCodec. Returns a bool
      * representing success or failure. If it returned true, and codecOut was
      * not NULL, it will be set to a new SkBmpCodec.
+     * Does *not* take ownership of the passed in SkStream.
      *
      */
     static bool ReadHeader(SkStream*, bool isIco, SkCodec** codecOut);
diff --git a/src/codec/SkCodec_libgif.cpp b/src/codec/SkCodec_libgif.cpp
index 2a1d81fc00..9356a6973b 100644
--- a/src/codec/SkCodec_libgif.cpp
+++ b/src/codec/SkCodec_libgif.cpp
@@ -136,6 +136,7 @@ static uint32_t find_trans_index(const SavedImage& image) {
  * Reads enough of the stream to determine the image format
  */
 SkCodec* SkGifCodec::NewFromStream(SkStream* stream) {
+    SkAutoTDelete<SkStream> streamDeleter(stream);
     // Read gif header, logical screen descriptor, and global color table
     SkAutoTCallIProc<GifFileType, CloseGif> gif(open_gif(stream));
 
@@ -165,7 +166,7 @@ SkCodec* SkGifCodec::NewFromStream(SkStream* stream) {
     // use kPremul directly even when kUnpremul is supported.
     const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
             kIndex_8_SkColorType, kPremul_SkAlphaType);
-    return SkNEW_ARGS(SkGifCodec, (imageInfo, stream, gif.detach()));
+    return SkNEW_ARGS(SkGifCodec, (imageInfo, streamDeleter.detach(), gif.detach()));
 }
 
 SkGifCodec::SkGifCodec(const SkImageInfo& srcInfo, SkStream* stream,
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index 33111cee67..121b74ff0b 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -346,11 +346,12 @@ static bool read_header(SkStream* stream, png_structp* png_ptrp,
 }
 
 SkCodec* SkPngCodec::NewFromStream(SkStream* stream) {
+    SkAutoTDelete<SkStream> streamDeleter(stream);
     png_structp png_ptr;
     png_infop info_ptr;
     SkImageInfo imageInfo;
     if (read_header(stream, &png_ptr, &info_ptr, &imageInfo)) {
-        return SkNEW_ARGS(SkPngCodec, (imageInfo, stream, png_ptr, info_ptr));
+        return SkNEW_ARGS(SkPngCodec, (imageInfo, streamDeleter.detach(), png_ptr, info_ptr));
     }
     return NULL;
 }
diff --git a/src/codec/SkCodec_wbmp.cpp b/src/codec/SkCodec_wbmp.cpp
index 465c76d4dc..073165d2ca 100644
--- a/src/codec/SkCodec_wbmp.cpp
+++ b/src/codec/SkCodec_wbmp.cpp
@@ -154,6 +154,7 @@ bool SkWbmpCodec::IsWbmp(SkStream* stream) {
 }
 
 SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream) {
+    SkAutoTDelete<SkStream> streamDeleter(stream);
     SkISize size;
     if (!read_header(stream, &size)) {
         return NULL;
@@ -161,5 +162,5 @@ SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream) {
     SkImageInfo info =
             SkImageInfo::Make(size.width(), size.height(), kGray_8_SkColorType,
                               kOpaque_SkAlphaType);
-    return SkNEW_ARGS(SkWbmpCodec, (info, stream));
+    return SkNEW_ARGS(SkWbmpCodec, (info, streamDeleter.detach()));
 }