Fix colorType/alphaType checks in SkCodec

src/codec/SkWbmpCodec.cpp

Index: src/codec/SkWbmpCodec.cpp
diff --git a/src/codec/SkWbmpCodec.cpp b/src/codec/SkWbmpCodec.cpp
index 4dd0f2544d181f848dde3b0cf015967e69f12518..dd6699473040bed3fd79001321a8d28299dc48ff 100644
--- a/src/codec/SkWbmpCodec.cpp
+++ b/src/codec/SkWbmpCodec.cpp
@@ -30,6 +30,19 @@ static inline void setup_color_table(SkColorType colorType,
     }
 }
 
+static inline bool valid_color_type(SkColorType colorType, SkAlphaType alphaType) {
+    switch (colorType) {
+        case kN32_SkColorType:
+        case kIndex_8_SkColorType:
+            return true;
+        case kGray_8_SkColorType:
+        case kRGB_565_SkColorType:
+            return kOpaque_SkAlphaType == alphaType;

[scroggo, 2016/02/12 17:13:55]

Here you're checking the dst instead of the src, unlike in SkCodecPriv.

It's also weird that someone would request you to decode to 565 premul, but
maybe it's fine?

Similar for Gray...?

[msarett, 2016/02/12 17:41:31]

I think it makes sense to disable it.  Let me know if you feel otherwise.  Right
now we disable it for getPixels(), but Gray/Premul can sneak through on scanline
decodes...

[scroggo, 2016/02/12 17:59:00]

Ah, I see now. I would lean towards calling the validate method for scanline
decodes. (Should we have tests for this?)

+        default:
+            return false;
+    }
+}
+
 static bool read_byte(SkStream* stream, uint8_t* data)
 {
     return stream->read(data, 1) == 1;
@@ -84,16 +97,6 @@ bool SkWbmpCodec::onRewind() {
 
 SkSwizzler* SkWbmpCodec::initializeSwizzler(const SkImageInfo& info, const SkPMColor* ctable,
         const Options& opts) {
-    // Create the swizzler based on the desired color type
-    switch (info.colorType()) {
-        case kIndex_8_SkColorType:
-        case kN32_SkColorType:
-        case kRGB_565_SkColorType:
-        case kGray_8_SkColorType:
-            break;
-        default:
-            return nullptr;
-    }
     return SkSwizzler::CreateSwizzler(SkSwizzler::kBit, ctable, info, opts);
 }
 
@@ -124,7 +127,8 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
         return kUnimplemented;
     }
 
-    if (!valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
+    if (!valid_color_type(info.colorType(), info.alphaType()) ||
+            !valid_alpha(info.alphaType(), this->getInfo().alphaType())) {
         return kInvalidConversion;
     }
 
@@ -133,9 +137,7 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info,
 
     // Initialize the swizzler
     SkAutoTDelete<SkSwizzler> swizzler(this->initializeSwizzler(info, ctable, options));
-    if (nullptr == swizzler.get()) {
-        return kInvalidConversion;
-    }
+    SkASSERT(swizzler);
 
     // Perform the decode
     SkISize size = info.dimensions();
@@ -188,7 +190,8 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
         return kUnimplemented;
     }
 
-    if (!valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
+    if (!valid_color_type(dstInfo.colorType(), dstInfo.alphaType()) ||
+            !valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) {
         return kInvalidConversion;
     }
 
@@ -202,9 +205,7 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo,
 
     // Initialize the swizzler
     fSwizzler.reset(this->initializeSwizzler(dstInfo, get_color_ptr(fColorTable.get()), options));
-    if (nullptr == fSwizzler.get()) {
-        return kInvalidConversion;
-    }
+    SkASSERT(fSwizzler);
 
     fSrcBuffer.reset(fSrcRowBytes);