Bug 1185799 (Part 2) - Make nsBMPDecoder and nsPNGDecoder no longer friends with nsICODecoder. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Sat, 02 Jul 2016 21:22:06 -0600
changeset 344170 090ab64054fd86f10f7f30c378c84bb57d112f62
parent 344169 07a67db040dca1ae3722eed0cd36880dd66da73b
child 344171 038d7b021492e648483aba1f1291ad7e902d4611
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1185799
milestone50.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1185799 (Part 2) - Make nsBMPDecoder and nsPNGDecoder no longer friends with nsICODecoder. r=edwin
image/decoders/nsBMPDecoder.h
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
--- a/image/decoders/nsBMPDecoder.h
+++ b/image/decoders/nsBMPDecoder.h
@@ -151,38 +151,35 @@ public:
 
   virtual void WriteInternal(const char* aBuffer,
                              uint32_t aCount) override;
   virtual void BeforeFinishInternal() override;
   virtual void FinishInternal() override;
 
 private:
   friend class DecoderFactory;
-  friend class nsICODecoder;
 
   enum class State {
     FILE_HEADER,
     INFO_HEADER_SIZE,
     INFO_HEADER_REST,
     BITFIELDS,
     COLOR_TABLE,
     GAP,
     AFTER_GAP,
     PIXEL_ROW,
     RLE_SEGMENT,
     RLE_DELTA,
     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.
   nsBMPDecoder(RasterImage* aImage, State aState, size_t aLength);
 
   int32_t AbsoluteHeight() const { return abs(mH.mHeight); }
 
   uint32_t* RowBuffer();
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -993,11 +993,43 @@ nsPNGDecoder::warning_callback(png_struc
 }
 
 Telemetry::ID
 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
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -21,22 +21,23 @@ class nsPNGDecoder : public Decoder
 {
 public:
   virtual ~nsPNGDecoder();
 
   virtual void InitInternal() override;
   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,
                        const gfx::IntRect& aFrameRect,
                        bool aIsInterlaced);
   void EndImageFrame();
 
   enum class TransparencyType
@@ -49,47 +50,16 @@ private:
   TransparencyType GetTransparencyType(gfx::SurfaceFormat aFormat,
                                        const gfx::IntRect& aFrameRect);
   void PostHasTransparencyIfNeeded(TransparencyType aTransparencyType);
 
   void PostInvalidationIfNeeded();
 
   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,
     FINISHED_PNG_DATA
   };
 
   LexerTransition<State> ReadPNGData(const char* aData, size_t aLength);
   LexerTransition<State> FinishedPNGData();