Creating a new wrapper for gif decoder

src/codec/SkCodec.cpp

Index: src/codec/SkCodec.cpp
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 33cc9e1c00b7d0e314cb96dbc57a87569b3bbc72..4aa422c184e5b2b5e9c322787e80bb410b2c152d 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -8,9 +8,11 @@
 #include "SkCodec.h"
 #include "SkData.h"
 #include "SkCodec_libbmp.h"
+#include "SkCodec_libgif.h"
 #include "SkCodec_libico.h"
 #include "SkCodec_libpng.h"
 #include "SkCodec_wbmp.h"
+#include "SkCodecPriv.h"
 #include "SkStream.h"
 
 struct DecoderProc {
@@ -20,6 +22,7 @@ struct DecoderProc {
 
 static const DecoderProc gDecoderProcs[] = {
     { SkPngCodec::IsPng, SkPngCodec::NewFromStream },
+    { SkGifCodec::IsGif, SkGifCodec::NewFromStream },
     { SkIcoCodec::IsIco, SkIcoCodec::NewFromStream },
     { SkBmpCodec::IsBmp, SkBmpCodec::NewFromStream },
     { SkWbmpCodec::IsWbmp, SkWbmpCodec::NewFromStream }
@@ -29,6 +32,8 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
     if (!stream) {
         return NULL;
     }
+    
+    SkCodec* codec = NULL;
     for (uint32_t i = 0; i < SK_ARRAY_COUNT(gDecoderProcs); i++) {
         DecoderProc proc = gDecoderProcs[i];
         const bool correctFormat = proc.IsFormat(stream);
@@ -36,10 +41,22 @@ SkCodec* SkCodec::NewFromStream(SkStream* stream) {
             return NULL;
         }
         if (correctFormat) {
-            return proc.NewFromStream(stream);
+            codec = proc.NewFromStream(stream);
+            break;
         }
     }
-    return NULL;
+
+    // Set the max size at 128 megapixels (512 MB for kN32).
+    // 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) {

[scroggo, 2015/03/31 18:39:56]

This may never/rarely come up in practice, but I'm wondering whether this is the
right approach. It seems like it's possible that the image's default info is too
large, but the client uses a smaller info which is not too big, and that is
supported by the codec.

[msarett, 2015/03/31 20:11:40]

Yeah that is certainly possible.  Maybe this isn't the right approach.  I'm not
exactly sure what the best approach is.

Another option is to make the check in onGetPixels based on the requestedInfo? 
Maybe we only fail if the requested dimensions and the original dimensions are
too large?  Currently we fail in most decoders when the requested dimensions
don't match the original dimensions, but I'm guessing this may not be the
permanent strategy.

Also, we could simply not check size at all?  Excessive app memory use and long
wait times could simply be our deterrent to users decoding large images.

[scroggo, 2015/03/31 20:39:11]

You are correct. JPEG supports downscaling by powers of two. And Webp supports
arbitrary
scales (see https://codereview.chromium.org/1044433002/).
That's certainly an option. The nasty part about *not* doing something special
is that a client may write the code but not be in control of the images they end
up decoding. On the other hand, different clients may make different decisions
about how big is too big, and it would be a shame if we set the limit smaller
than they need. That said, some of our SkImageDecoders do their own size checks,
which I have included in their corresponding SkCodecs (those may be or may have
been real limits for us, rather than "this is too slow", though).

I forgot, what does Chromium do?

+        SkCodecPrintf("Error: Image size too large, cannot decode.\n");
+        return NULL;
+    } else {
+        return codec;
+    }
 }
 
 SkCodec* SkCodec::NewFromData(SkData* data) {