Fix ICO bug exposed by the fuzzer

src/codec/SkBmpStandardCodec.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkBmpStandardCodec.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
@@ skipping 132 matching lines @@
             SkCodecPrintf("Error: unable to skip to image data.\n");
             return false;
         }
     }
 
     // Return true on success
     return true;
 }
 
 void SkBmpStandardCodec::initializeSwizzler(const SkImageInfo& dstInfo, const Options& opts) {
-    // In the case of paletted ico-in-bmps, we will report BGRA to the client,
-    // since we may be required to apply an alpha mask after the decode.  But
-    // the swizzler needs to know the actual format of the bmp.
+    // In the case of ico-in-bmps, we will report BGRA to the client,

[scroggo, 2016/05/04 13:02:19]

nit: Shouldn't this be bmp-in-icos?

[msarett, 2016/05/04 19:27:49]

Done.

+    // since we may be required to apply an alpha mask after the decode.
+    // However, the swizzler needs to know the actual format of the bmp.
     SkEncodedInfo swizzlerInfo = this->getEncodedInfo();
-    if (fInIco && this->bitsPerPixel() <= 8) {
-        swizzlerInfo = SkEncodedInfo::Make(SkEncodedInfo::kPalette_Color, swizzlerInfo.alpha(),
-                this->bitsPerPixel());
+    if (fInIco) {
+        switch (this->bitsPerPixel()) {
+            case 8:

[scroggo, 2016/05/04 13:02:19]

We used to handle <= 8. I take it < was unnecessary?

[msarett, 2016/05/04 19:27:49]

Nice catch!  This is me trying to introduce a new bug.  <= 8 is correct, I'll
fix this.

+                swizzlerInfo = SkEncodedInfo::Make(SkEncodedInfo::kPalette_Color,
+                        swizzlerInfo.alpha(), this->bitsPerPixel());

[scroggo, 2016/05/04 13:02:19]

8?

[msarett, 2016/05/04 19:27:49]

It could be 1, 2, 4, or 8.

+                break;
+            case 24:
+                swizzlerInfo = SkEncodedInfo::Make(SkEncodedInfo::kBGR_Color,
+                        SkEncodedInfo::kOpaque_Alpha, swizzlerInfo.bitsPerComponent());

[scroggo, 2016/05/04 13:02:19]

Why do you need to check bitsPerComponent here? Will it ever be anything other
than 8? If so, it seems like something doesn't match up.

[msarett, 2016/05/04 19:27:49]

It will always be 8.  I'll put 8.

+                break;
+            default:
+                break;
+        }
     }
 
     // Get a pointer to the color table if it exists
     const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
 
     // Create swizzler
     fSwizzler.reset(SkSwizzler::CreateSwizzler(swizzlerInfo, colorPtr, dstInfo, opts));
     SkASSERT(fSwizzler);
 }
 
@@ skipping 127 matching lines @@
     }
 }
 
 uint32_t SkBmpStandardCodec::onGetFillValue(SkColorType colorType) const {
     const SkPMColor* colorPtr = get_color_ptr(fColorTable.get());
     if (colorPtr) {
         return get_color_table_fill_value(colorType, colorPtr, 0);
     }
     return INHERITED::onGetFillValue(colorType);
 }