Bug 1352282. Always fill in the number of loops when decoding an APNG file. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Wed, 05 Apr 2017 11:28:40 -0500
changeset 399486 5bd86eea082a395f9c45233bac7506108390d267
parent 399485 08e2719fe7be236f1cdff3be4b4b2d9252b90dab
child 399487 cb514a08571583d3602c809ee005414cd02b6fab
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1352282
milestone55.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 1352282. Always fill in the number of loops when decoding an APNG file. r=aosmond If we were doing a first frame only decode we wouldn't fill in this value. The spec says this chunk must come before any image data so it should always be available at the end of any full decode (whether it be truly full or first frame only).
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -938,33 +938,59 @@ nsPNGDecoder::DoYield(png_structp aPNGSt
 
   MOZ_ASSERT(pendingBytes < mLastChunkLength);
   size_t consumedBytes = mLastChunkLength - min(pendingBytes, mLastChunkLength);
 
   mNextTransition =
     Transition::ContinueUnbufferedAfterYield(State::PNG_DATA, consumedBytes);
 }
 
+nsresult
+nsPNGDecoder::FinishInternal()
+{
+  // We shouldn't be called in error cases.
+  MOZ_ASSERT(!HasError(), "Can't call FinishInternal on error!");
+
+  if (IsMetadataDecode()) {
+    return NS_OK;
+  }
+
+  int32_t loop_count = 0;
+#ifdef PNG_APNG_SUPPORTED
+  if (png_get_valid(mPNG, mInfo, PNG_INFO_acTL)) {
+    int32_t num_plays = png_get_num_plays(mPNG, mInfo);
+    loop_count = num_plays - 1;
+  }
+#endif
+
+  if (InFrame()) {
+    EndImageFrame();
+  }
+  PostDecodeDone(loop_count);
+
+  return NS_OK;
+}
+
+
 #ifdef PNG_APNG_SUPPORTED
 // got the header of a new frame that's coming
 void
 nsPNGDecoder::frame_info_callback(png_structp png_ptr, png_uint_32 frame_num)
 {
   nsPNGDecoder* decoder =
                static_cast<nsPNGDecoder*>(png_get_progressive_ptr(png_ptr));
 
   // old frame is done
   decoder->EndImageFrame();
 
   const bool previousFrameWasHidden = decoder->mFrameIsHidden;
 
   if (!previousFrameWasHidden && decoder->IsFirstFrameDecode()) {
     // We're about to get a second non-hidden frame, but we only want the first.
     // Stop decoding now. (And avoid allocating the unnecessary buffers below.)
-    decoder->PostDecodeDone();
     return decoder->DoTerminate(png_ptr, TerminalState::SUCCESS);
   }
 
   // Only the first frame can be hidden, so unhide unconditionally here.
   decoder->mFrameIsHidden = false;
 
   // Save the information necessary to create the frame; we'll actually create
   // it when we return from the yield.
@@ -1020,27 +1046,16 @@ nsPNGDecoder::end_callback(png_structp p
    */
 
   nsPNGDecoder* decoder =
                static_cast<nsPNGDecoder*>(png_get_progressive_ptr(png_ptr));
 
   // We shouldn't get here if we've hit an error
   MOZ_ASSERT(!decoder->HasError(), "Finishing up PNG but hit error!");
 
-  int32_t loop_count = 0;
-#ifdef PNG_APNG_SUPPORTED
-  if (png_get_valid(png_ptr, info_ptr, PNG_INFO_acTL)) {
-    int32_t num_plays = png_get_num_plays(png_ptr, info_ptr);
-    loop_count = num_plays - 1;
-  }
-#endif
-
-  // Send final notifications.
-  decoder->EndImageFrame();
-  decoder->PostDecodeDone(loop_count);
   return decoder->DoTerminate(png_ptr, TerminalState::SUCCESS);
 }
 
 
 void
 nsPNGDecoder::error_callback(png_structp png_ptr, png_const_charp error_msg)
 {
   MOZ_LOG(sPNGLog, LogLevel::Error, ("libpng error: %s\n", error_msg));
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -22,16 +22,17 @@ class nsPNGDecoder : public Decoder
 public:
   virtual ~nsPNGDecoder();
 
   /// @return true if this PNG is a valid ICO resource.
   bool IsValidICO() const;
 
 protected:
   nsresult InitInternal() override;
+  nsresult FinishInternal() override;
   LexerResult DoDecode(SourceBufferIterator& aIterator,
                        IResumable* aOnResume) override;
 
   Maybe<Telemetry::HistogramID> SpeedHistogram() const override;
 
 private:
   friend class DecoderFactory;