Bug 1315554 - Part 3. Expose Decoder::IsValidICOResource for all decoders. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Sat, 22 Jul 2017 07:50:31 -0400
changeset 419113 92d1377f6bb80267ab189483e1859c3b807e9c46
parent 419112 6fef952ac780d1b42ae97e21e9d6204a6bb49e78
child 419114 94eda3aa3808c7d9c26847716850f3001033d35e
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;