Bug 1185799 (Part 2) - Make nsBMPDecoder and nsPNGDecoder no longer friends with nsICODecoder. r=edwin

image/decoders/nsBMPDecoder.h

@@ -156,7 +156,6 @@ public:
 
 private:
   friend class DecoderFactory;
-  friend class nsICODecoder;
 
   enum class State {
     FILE_HEADER,
@@ -172,12 +171,10 @@ private:
     RLE_ABSOLUTE
   };
 
-  // This is the constructor used by DecoderFactory.
+  // This is the constructor used for normal BMP images.
   explicit nsBMPDecoder(RasterImage* aImage);
 
-  // This is the constructor used by nsICODecoder.
-  // XXX(seth): nsICODecoder is temporarily an exception to the rule that
-  //            decoders should only be instantiated via DecoderFactory.
+  // This is the constructor used for BMP resources in ICO images.
   nsBMPDecoder(RasterImage* aImage, uint32_t aDataOffset);
 
   // Helper constructor called by the other two.

image/decoders/nsPNGDecoder.cpp

@@ -998,6 +998,38 @@ nsPNGDecoder::SpeedHistogram()
   return Telemetry::IMAGE_DECODE_SPEED_PNG;
 }
 
+bool
+nsPNGDecoder::IsValidICO() const
+{
+  // Only 32-bit RGBA PNGs are valid ICO resources; see here:
+  //   http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
+
+  // If there are errors in the call to png_get_IHDR, the error_callback in
+  // nsPNGDecoder.cpp is called.  In this error callback we do a longjmp, so
+  // we need to save the jump buffer here. Oterwise we'll end up without a
+  // proper callstack.
+  if (setjmp(png_jmpbuf(mPNG))) {
+    // We got here from a longjmp call indirectly from png_get_IHDR
+    return false;
+  }
+
+  png_uint_32
+      png_width,  // Unused
+      png_height; // Unused
+
+  int png_bit_depth,
+      png_color_type;
+
+  if (png_get_IHDR(mPNG, mInfo, &png_width, &png_height, &png_bit_depth,
+                   &png_color_type, nullptr, nullptr, nullptr)) {
+
+    return ((png_color_type == PNG_COLOR_TYPE_RGB_ALPHA ||
+             png_color_type == PNG_COLOR_TYPE_RGB) &&
+            png_bit_depth == 8);
+  } else {
+    return false;
+  }
+}
 
 } // namespace image
 } // namespace mozilla

image/decoders/nsPNGDecoder.h

@@ -26,12 +26,13 @@ public:
   virtual void WriteInternal(const char* aBuffer, uint32_t aCount) override;
   virtual Telemetry::ID SpeedHistogram() override;
 
+  /// @return true if this PNG is a valid ICO resource.
+  bool IsValidICO() const;
+
 private:
   friend class DecoderFactory;
-  friend class nsICODecoder;
 
   // Decoders should only be instantiated via DecoderFactory.
-  // XXX(seth): nsICODecoder is temporarily an exception to this rule.
   explicit nsPNGDecoder(RasterImage* aImage);
 
   nsresult CreateFrame(gfx::SurfaceFormat aFormat,
@@ -54,37 +55,6 @@ private:
 
   void WriteRow(uint8_t* aRow);
 
-  // Check if PNG is valid ICO (32bpp RGBA)
-  // http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
-  bool IsValidICO() const
-  {
-    // If there are errors in the call to png_get_IHDR, the error_callback in
-    // nsPNGDecoder.cpp is called.  In this error callback we do a longjmp, so
-    // we need to save the jump buffer here. Oterwise we'll end up without a
-    // proper callstack.
-    if (setjmp(png_jmpbuf(mPNG))) {
-      // We got here from a longjmp call indirectly from png_get_IHDR
-      return false;
-    }
-
-    png_uint_32
-        png_width,  // Unused
-        png_height; // Unused
-
-    int png_bit_depth,
-        png_color_type;
-
-    if (png_get_IHDR(mPNG, mInfo, &png_width, &png_height, &png_bit_depth,
-                     &png_color_type, nullptr, nullptr, nullptr)) {
-
-      return ((png_color_type == PNG_COLOR_TYPE_RGB_ALPHA ||
-               png_color_type == PNG_COLOR_TYPE_RGB) &&
-              png_bit_depth == 8);
-    } else {
-      return false;
-    }
-  }
-
   enum class State
   {
     PNG_DATA,