Fix bug in ico decodes

src/codec/SkCodec_libico.cpp

Index: src/codec/SkCodec_libico.cpp
diff --git a/src/codec/SkCodec_libico.cpp b/src/codec/SkCodec_libico.cpp
index 6f343369c4cc0cb389d641728e962235ff4538e6..ec92937f5431b3352361ba148da8548307458bf6 100644
--- a/src/codec/SkCodec_libico.cpp
+++ b/src/codec/SkCodec_libico.cpp
@@ -255,10 +255,24 @@ SkCodec::Result SkIcoCodec::onGetPixels(const SkImageInfo& dstInfo,
         if (dstInfo.dimensions() == embeddedCodec->getInfo().dimensions()) {
 
             // Perform the decode
-            // FIXME: (msarett): ICO is considered non-opaque, even if the embedded BMP
+            // FIXME (msarett): ICO is considered non-opaque, even if the embedded BMP
             // incorrectly claims it has no alpha.
-            SkImageInfo info = dstInfo.makeAlphaType(embeddedCodec->getInfo().alphaType());
-            result = embeddedCodec->getPixels(info, dst, dstRowBytes, &opts, ct, ptr);
+            SkAlphaType embeddedAlpha = embeddedCodec->getInfo().alphaType();
+            switch (embeddedAlpha) {
+                case kPremul_SkAlphaType:
+                case kUnpremul_SkAlphaType:
+                    // Use the requested alpha type if the embedded codec supports alpha.
+                    embeddedAlpha = dstInfo.alphaType();

[scroggo, 2015/09/02 21:50:54]

I suppose this is safe because dstInfo.alphaType() was already checked above?

[msarett, 2015/09/02 21:56:36]

Yes - we always guess that ICOs have alpha (and therefore require dstInfo to
have alpha).  This would need to change if we allow icos to sometimes be opaque.

I don't think we can ever allow bmp-in-icos to be opaque given that they have an
alpha mask.  It's possible/allowable for png-in-icos to be opaque.  Though it
seems rare.

I'm guessing that when I address the FIXME, we will allow opaque png-in-icos and
need to make a slight modification here.

+                    break;
+                case kOpaque_SkAlphaType:
+                    // If the embedded codec claims it is opaque, decode as if it is opaque.
+                    break;
+                default:
+                    SkASSERT(false);
+                    break;
+            }
+            SkImageInfo info = dstInfo.makeAlphaType(embeddedAlpha);
+            result = embeddedCodec->getPixels(dstInfo, dst, dstRowBytes, &opts, ct, ptr);
 
             // On a fatal error, keep trying to find an image to decode
             if (kInvalidConversion == result || kInvalidInput == result ||