Bug 1282566 (Part 1) - Use png_process_data_pause for early exits in nsPNGDecoder. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 27 Jun 2016 12:00:16 -0700
changeset 343211 748f5424739f63fd4596f7e5adb876b28a4cfc79
parent 343210 49b3e41bb16b2f464f38a049943379ce6c612eb8
child 343212 442c01c74c5f715e2f6263257ae994a07fa1956d
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
bugs1282566
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 1282566 (Part 1) - Use png_process_data_pause for early exits in nsPNGDecoder. r=edwin
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -102,17 +102,16 @@ nsPNGDecoder::nsPNGDecoder(RasterImage* 
  , mTransform(nullptr)
  , format(gfx::SurfaceFormat::UNKNOWN)
  , mHeaderBytesRead(0)
  , mCMSMode(0)
  , mChannels(0)
  , mPass(0)
  , mFrameIsHidden(false)
  , mDisablePremultipliedAlpha(false)
- , mSuccessfulEarlyFinish(false)
  , mNumFrames(0)
 { }
 
 nsPNGDecoder::~nsPNGDecoder()
 {
   if (mPNG) {
     png_destroy_read_struct(&mPNG, mInfo ? &mInfo : nullptr, nullptr);
   }
@@ -348,24 +347,22 @@ nsPNGDecoder::InitInternal()
 void
 nsPNGDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
 {
   MOZ_ASSERT(!HasError(), "Shouldn't call WriteInternal after error!");
 
   // libpng uses setjmp/longjmp for error handling. Set it up.
   if (setjmp(png_jmpbuf(mPNG))) {
 
-    // We exited early. If mSuccessfulEarlyFinish isn't true, then we
-    // encountered an error. We might not really know what caused it, but it
-    // makes more sense to blame the data.
-    if (!mSuccessfulEarlyFinish && !HasError()) {
+    // We exited early due to an error. We might not really know what caused
+    // it, but it makes more sense to blame the data.
+    if (!HasError()) {
       PostDataError();
     }
 
-    png_destroy_read_struct(&mPNG, &mInfo, nullptr);
     return;
   }
 
   // Pass the data off to libpng.
   png_process_data(mPNG, mInfo,
                    reinterpret_cast<unsigned char*>(const_cast<char*>((aBuffer))),
                    aCount);
 }
@@ -647,20 +644,20 @@ nsPNGDecoder::info_callback(png_structp 
     // if the IDAT chunk is part of the animation 2) the frame rect of the first
     // fDAT chunk otherwise. If we are not animated then we want to make sure to
     // call PostHasTransparency in the metadata decode if we need to. So it's okay
     // to pass IntRect(0, 0, width, height) here for animated images; they will
     // call with the proper first frame rect in the full decode.
     auto transparency = decoder->GetTransparencyType(decoder->format, frameRect);
     decoder->PostHasTransparencyIfNeeded(transparency);
 
-    // We have the metadata we're looking for, so we don't need to decode any
-    // further.
-    decoder->mSuccessfulEarlyFinish = true;
-    png_longjmp(decoder->mPNG, 1);
+    // We have the metadata we're looking for, so stop here, before we allocate
+    // buffers below.
+    png_process_data_pause(png_ptr, /* save = */ false);
+    return;
   }
 
 #ifdef PNG_APNG_SUPPORTED
   if (isAnimated) {
     png_set_progressive_frame_fn(png_ptr, nsPNGDecoder::frame_info_callback,
                                  nullptr);
   }
 
@@ -771,16 +768,18 @@ nsPNGDecoder::row_callback(png_structp p
    */
   nsPNGDecoder* decoder =
     static_cast<nsPNGDecoder*>(png_get_progressive_ptr(png_ptr));
 
   if (decoder->mFrameIsHidden) {
     return;  // Skip this frame.
   }
 
+  MOZ_ASSERT_IF(decoder->IsFirstFrameDecode(), decoder->mNumFrames == 0);
+
   while (pass > decoder->mPass) {
     // Advance to the next pass. We may have to do this multiple times because
     // libpng will skip passes if the image is so small that no pixels have
     // changed on a given pass, but ADAM7InterpolatingFilter needs to be reset
     // once for every pass to perform interpolation properly.
     decoder->mPipe.ResetToFirstRow();
     decoder->mPass++;
   }
@@ -876,20 +875,20 @@ nsPNGDecoder::frame_info_callback(png_st
   nsPNGDecoder* decoder =
                static_cast<nsPNGDecoder*>(png_get_progressive_ptr(png_ptr));
 
   // old frame is done
   decoder->EndImageFrame();
 
   if (!decoder->mFrameIsHidden && decoder->IsFirstFrameDecode()) {
     // We're about to get a second non-hidden frame, but we only want the first.
-    // Stop decoding now.
+    // Stop decoding now. (And avoid allocating the unnecessary buffers below.)
     decoder->PostDecodeDone();
-    decoder->mSuccessfulEarlyFinish = true;
-    png_longjmp(decoder->mPNG, 1);
+    png_process_data_pause(png_ptr, /* save = */ false);
+    return;
   }
 
   // Only the first frame can be hidden, so unhide unconditionally here.
   decoder->mFrameIsHidden = false;
 
   const IntRect frameRect(png_get_next_frame_x_offset(png_ptr, decoder->mInfo),
                           png_get_next_frame_y_offset(png_ptr, decoder->mInfo),
                           png_get_next_frame_width(png_ptr, decoder->mInfo),
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -101,17 +101,16 @@ public:
 
   // whether CMS or premultiplied alpha are forced off
   uint32_t mCMSMode;
 
   uint8_t mChannels;
   uint8_t mPass;
   bool mFrameIsHidden;
   bool mDisablePremultipliedAlpha;
-  bool mSuccessfulEarlyFinish;
 
   struct AnimFrameInfo
   {
     AnimFrameInfo();
 #ifdef PNG_APNG_SUPPORTED
     AnimFrameInfo(png_structp aPNG, png_infop aInfo);
 #endif