Bug 1315554 - Part 3. Expose Decoder::IsValidICOResource for all decoders. r=tnikkel
☠☠ backed out by 082df1a7a641 ☠ ☠
authorAndrew Osmond <aosmond@mozilla.com>
Sat, 22 Jul 2017 00:14:59 -0400
changeset 419088 9780a01b3e949a5bf0f6a95fe0e1ed25d5cebe29
parent 419087 e1eec63b920fe003676d38aca1303dc158646ebb
child 419089 e67f6df41836b70373f7337ae392533ac9c8c7bf
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1315554
milestone56.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 1315554 - Part 3. Expose Decoder::IsValidICOResource for all decoders. r=tnikkel
image/Decoder.h
image/decoders/nsBMPDecoder.h
image/decoders/nsICODecoder.cpp
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -288,16 +288,22 @@ public:
   {
     return mReachedTerminalState || mDecodeDone ||
            (mMetadataDecode && HasSize()) || HasError();
   }
 
   /// Are we in the middle of a frame right now? Used for assertions only.
   bool InFrame() const { return mInFrame; }
 
+  /// Is the image valid if embedded inside an ICO.
+  virtual bool IsValidICOResource() const
+  {
+    return false;
+  }
+
   enum DecodeStyle {
       PROGRESSIVE, // produce intermediate frames representing the partial
                    // state of the image
       SEQUENTIAL   // decode to final image immediately
   };
 
   /**
    * Get or set the DecoderFlags that influence the behavior of this decoder.
--- a/image/decoders/nsBMPDecoder.h
+++ b/image/decoders/nsBMPDecoder.h
@@ -120,16 +120,19 @@ class RasterImage;
 
 /// Decoder for BMP-Files, as used by Windows and OS/2.
 
 class nsBMPDecoder : public Decoder
 {
 public:
   ~nsBMPDecoder();
 
+  /// @return true if this BMP is a valid ICO resource.
+  bool IsValidICOResource() const override { return true; }
+
   /// Obtains the internal output image buffer.
   uint32_t* GetImageData() { return reinterpret_cast<uint32_t*>(mImageData); }
 
   /// Obtains the length of the internal output image buffer.
   size_t GetImageDataLength() const { return mImageDataLength; }
 
   /// Obtains the size of the compressed image resource.
   int32_t GetCompressedImageSize() const;
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -300,22 +300,16 @@ nsICODecoder::SniffResource(const char* 
 
 LexerTransition<ICOState>
 nsICODecoder::ReadPNG(const char* aData, uint32_t aLen)
 {
   if (!WriteToContainedDecoder(aData, aLen)) {
     return Transition::TerminateFailure();
   }
 
-  // Raymond Chen says that 32bpp only are valid PNG ICOs
-  // http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
-  if (!static_cast<nsPNGDecoder*>(mContainedDecoder.get())->IsValidICO()) {
-    return Transition::TerminateFailure();
-  }
-
   return Transition::ContinueUnbuffered(ICOState::READ_PNG);
 }
 
 LexerTransition<ICOState>
 nsICODecoder::ReadBIH(const char* aData)
 {
   // Buffer the rest of the bitmap information header.
   memcpy(mBIHraw + PNGSIGNATURESIZE, aData, BITMAPINFOSIZE - PNGSIGNATURESIZE);
@@ -535,16 +529,22 @@ nsICODecoder::FinishResource()
 {
   // Make sure the actual size of the resource matches the size in the directory
   // entry. If not, we consider the image corrupt.
   if (mContainedDecoder->HasSize() &&
       mContainedDecoder->Size() != GetRealSize()) {
     return Transition::TerminateFailure();
   }
 
+  // Raymond Chen says that 32bpp only are valid PNG ICOs
+  // http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
+  if (!mContainedDecoder->IsValidICOResource()) {
+    return Transition::TerminateFailure();
+  }
+
   // Finalize the frame which we deferred to ensure we could modify the final
   // result (e.g. to apply the BMP mask).
   MOZ_ASSERT(!mContainedDecoder->GetFinalizeFrames());
   if (mCurrentFrame) {
     mCurrentFrame->FinalizeSurface();
   }
 
   return Transition::TerminateSuccess();
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -1085,17 +1085,17 @@ nsPNGDecoder::warning_callback(png_struc
 
 Maybe<Telemetry::HistogramID>
 nsPNGDecoder::SpeedHistogram() const
 {
   return Some(Telemetry::IMAGE_DECODE_SPEED_PNG);
 }
 
 bool
-nsPNGDecoder::IsValidICO() const
+nsPNGDecoder::IsValidICOResource() 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. Otherwise we'll end up without a
   // proper callstack.
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -18,17 +18,17 @@ namespace image {
 class RasterImage;
 
 class nsPNGDecoder : public Decoder
 {
 public:
   virtual ~nsPNGDecoder();
 
   /// @return true if this PNG is a valid ICO resource.
-  bool IsValidICO() const;
+  bool IsValidICOResource() const override;
 
 protected:
   nsresult InitInternal() override;
   nsresult FinishInternal() override;
   LexerResult DoDecode(SourceBufferIterator& aIterator,
                        IResumable* aOnResume) override;
 
   Maybe<Telemetry::HistogramID> SpeedHistogram() const override;