Bug 685471 - In PNG decoder, the allocating of mHeaderBuf can be prevented. r=joe@drew.ca
authorAlfred Kayser <alfredkayser@gmail.com>
Sat, 04 May 2013 11:39:47 +0200
changeset 130828 cf288039ea21f79ed1823c0b7090a1f22c5907be
parent 130827 f58eca3886bdc503a3eed66ebd9dee8605b1304a
child 130829 3bff6b2db01c3d7a53374f9f8a575e9c516123c0
push id27555
push useralfredkayser@gmail.com
push dateSat, 04 May 2013 09:40:01 +0000
treeherdermozilla-inbound@cf288039ea21 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe
bugs685471
milestone23.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 685471 - In PNG decoder, the allocating of mHeaderBuf can be prevented. r=joe@drew.ca
image/decoders/nsPNGDecoder.cpp
image/decoders/nsPNGDecoder.h
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -98,27 +98,27 @@ nsPNGDecoder::AnimFrameInfo::AnimFrameIn
     mBlend = RasterImage::kBlendSource;
   } else {
     mBlend = RasterImage::kBlendOver;
   }
 }
 #endif
 
 // First 8 bytes of a PNG file
-const uint8_t 
+const uint8_t
 nsPNGDecoder::pngSignatureBytes[] = { 137, 80, 78, 71, 13, 10, 26, 10 };
 
 nsPNGDecoder::nsPNGDecoder(RasterImage &aImage)
  : Decoder(aImage),
    mPNG(nullptr), mInfo(nullptr),
    mCMSLine(nullptr), interlacebuf(nullptr),
    mInProfile(nullptr), mTransform(nullptr),
-   mHeaderBuf(nullptr), mHeaderBytesRead(0),
+   mHeaderBytesRead(0), mCMSMode(0),
    mChannels(0), mFrameIsHidden(false),
-   mCMSMode(0), mDisablePremultipliedAlpha(false),
+   mDisablePremultipliedAlpha(false),
    mNumFrames(0)
 {
 }
 
 nsPNGDecoder::~nsPNGDecoder()
 {
   if (mPNG)
     png_destroy_read_struct(&mPNG, mInfo ? &mInfo : NULL, NULL);
@@ -128,18 +128,16 @@ nsPNGDecoder::~nsPNGDecoder()
     nsMemory::Free(interlacebuf);
   if (mInProfile) {
     qcms_profile_release(mInProfile);
 
     /* mTransform belongs to us only if mInProfile is non-null */
     if (mTransform)
       qcms_transform_release(mTransform);
   }
-  if (mHeaderBuf)
-    nsMemory::Free(mHeaderBuf);
 }
 
 // CreateFrame() is used for both simple and animated images
 void nsPNGDecoder::CreateFrame(png_uint_32 x_offset, png_uint_32 y_offset,
                                int32_t width, int32_t height,
                                gfxASurface::gfxImageFormat format)
 {
   NeedNewFrame(mNumFrames, x_offset, y_offset, width, height, format);
@@ -188,16 +186,21 @@ void nsPNGDecoder::EndImageFrame()
 #endif
 
   PostFrameStop(alpha, mAnimInfo.mDispose, mAnimInfo.mTimeout, mAnimInfo.mBlend);
 }
 
 void
 nsPNGDecoder::InitInternal()
 {
+  // For size decodes, we don't need to initialize the png decoder
+  if (IsSizeDecode()) {
+    return;
+  }
+
   mCMSMode = gfxPlatform::GetCMSMode();
   if ((mDecodeFlags & DECODER_NO_COLORSPACE_CONVERSION) != 0)
     mCMSMode = eCMSMode_Off;
   mDisablePremultipliedAlpha = (mDecodeFlags & DECODER_NO_PREMULTIPLY_ALPHA) != 0;
 
 #ifdef PNG_HANDLE_AS_UNKNOWN_SUPPORTED
   static png_byte color_chunks[]=
        { 99,  72,  82,  77, '\0',   /* cHRM */
@@ -212,22 +215,16 @@ nsPNGDecoder::InitInternal()
         112,  72,  89, 115, '\0',   /* pHYs */
         115,  66,  73,  84, '\0',   /* sBIT */
         115,  80,  76,  84, '\0',   /* sPLT */
         116,  69,  88, 116, '\0',   /* tEXt */
         116,  73,  77,  69, '\0',   /* tIME */
         122,  84,  88, 116, '\0'};  /* zTXt */
 #endif
 
-  // For size decodes, we only need a small buffer
-  if (IsSizeDecode()) {
-    mHeaderBuf = (uint8_t *)moz_xmalloc(BYTES_NEEDED_FOR_DIMENSIONS);
-    return;
-  }
-
   /* For full decodes, do png init stuff */
 
   /* Initialize the container's source image header. */
   /* Always decode to 24 bit pixdepth */
 
   mPNG = png_create_read_struct(PNG_LIBPNG_VER_STRING,
                                 NULL, nsPNGDecoder::error_callback,
                                 nsPNGDecoder::warning_callback);
@@ -244,17 +241,17 @@ nsPNGDecoder::InitInternal()
   }
 
 #ifdef PNG_HANDLE_AS_UNKNOWN_SUPPORTED
   /* Ignore unused chunks */
   if (mCMSMode == eCMSMode_Off)
     png_set_keep_unknown_chunks(mPNG, 1, color_chunks, 2);
 
   png_set_keep_unknown_chunks(mPNG, 1, unused_chunks,
-                              (int)sizeof(unused_chunks)/5);   
+                              (int)sizeof(unused_chunks)/5);
 #endif
 
 #ifdef PNG_SET_CHUNK_MALLOC_LIMIT_SUPPORTED
   if (mCMSMode != eCMSMode_Off)
     png_set_chunk_malloc_max(mPNG, 4000000L);
 #endif
 
 #ifdef PNG_READ_CHECK_FOR_INVALID_INDEX_SUPPORTED
@@ -284,35 +281,44 @@ nsPNGDecoder::WriteInternal(const char *
 
   // If we only want width/height, we don't need to go through libpng
   if (IsSizeDecode()) {
 
     // Are we done?
     if (mHeaderBytesRead == BYTES_NEEDED_FOR_DIMENSIONS)
       return;
 
-    // Read data into our header buffer
-    uint32_t bytesToRead = std::min(aCount, BYTES_NEEDED_FOR_DIMENSIONS -
-                                  mHeaderBytesRead);
-    memcpy(mHeaderBuf + mHeaderBytesRead, aBuffer, bytesToRead);
-    mHeaderBytesRead += bytesToRead;
+    // Scan the header for the width and height bytes
+    PRUint32 pos = 0;
+    const uint8_t *bptr = (uint8_t *)aBuffer;
+
+    while (pos < aCount && mHeaderBytesRead < BYTES_NEEDED_FOR_DIMENSIONS) {
+      // Verify the signature bytes
+      if (mHeaderBytesRead < sizeof(pngSignatureBytes)) {
+        if (bptr[pos] != nsPNGDecoder::pngSignatureBytes[mHeaderBytesRead]) {
+          PostDataError();
+          return;
+        }
+      }
+
+      // Get width and height bytes into the buffer
+      if ((mHeaderBytesRead >= WIDTH_OFFSET) &&
+          (mHeaderBytesRead < BYTES_NEEDED_FOR_DIMENSIONS)) {
+        mSizeBytes[mHeaderBytesRead - WIDTH_OFFSET] = bptr[pos];
+      }
+      pos ++;
+      mHeaderBytesRead ++;
+    }
 
     // If we're done now, verify the data and set up the container
     if (mHeaderBytesRead == BYTES_NEEDED_FOR_DIMENSIONS) {
 
-      // Check that the signature bytes are right
-      if (memcmp(mHeaderBuf, nsPNGDecoder::pngSignatureBytes, 
-                 sizeof(pngSignatureBytes))) {
-        PostDataError();
-        return;
-      }
-
       // Grab the width and height, accounting for endianness (thanks libpng!)
-      uint32_t width = png_get_uint_32(mHeaderBuf + WIDTH_OFFSET);
-      uint32_t height = png_get_uint_32(mHeaderBuf + HEIGHT_OFFSET);
+      PRUint32 width = png_get_uint_32(mSizeBytes);
+      PRUint32 height = png_get_uint_32(mSizeBytes + 4);
 
       // Too big?
       if ((width > MOZ_PNG_MAX_DIMENSION) || (height > MOZ_PNG_MAX_DIMENSION)) {
         PostDataError();
         return;
       }
 
       // Post our size to the superclass
--- a/image/decoders/nsPNGDecoder.h
+++ b/image/decoders/nsPNGDecoder.h
@@ -74,25 +74,25 @@ public:
   uint8_t *mCMSLine;
   uint8_t *interlacebuf;
   qcms_profile *mInProfile;
   qcms_transform *mTransform;
 
   gfxASurface::gfxImageFormat format;
 
   // For size decodes
-  uint8_t *mHeaderBuf;
-  uint32_t mHeaderBytesRead;
+  PRUint8  mSizeBytes[8]; // Space for width and height, both 4 bytes
+  PRUint32 mHeaderBytesRead;
+
+  // whether CMS or premultiplied alpha are forced off
+  uint32_t mCMSMode;
 
   uint8_t mChannels;
   bool mFrameHasNoAlpha;
   bool mFrameIsHidden;
-
-  // whether CMS or premultiplied alpha are forced off
-  uint32_t mCMSMode;
   bool mDisablePremultipliedAlpha;
 
   struct AnimFrameInfo
   {
     AnimFrameInfo();
 #ifdef PNG_APNG_SUPPORTED
     AnimFrameInfo(png_structp aPNG, png_infop aInfo);
 #endif