Backed out changeset dbae61d1cbee (bug 1315554)
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 22 Jul 2017 11:03:57 +0200
changeset 419099 e2e062406f6320a608921b0eaa0ffc95dbd9b108
parent 419098 b6cbfeed5f34e75410645c92c6ee2633736d6d68
child 419100 aee78154a945c91d826a856d7ad1dfd3553d9eeb
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)
bugs1315554
milestone56.0a1
backs outdbae61d1cbee8540e6e7dd7f27d7fd9fc3d31f70
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
Backed out changeset dbae61d1cbee (bug 1315554)
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -244,117 +244,114 @@ nsICODecoder::ReadDirEntry(const char* a
   }
 
   return Transition::To(ICOState::DIR_ENTRY, ICODIRENTRYSIZE);
 }
 
 LexerTransition<ICOState>
 nsICODecoder::SniffResource(const char* aData)
 {
-  // We have BITMAPINFOSIZE bytes buffered at this point. We know an embedded
-  // BMP will have at least that many bytes by definition. We can also infer
-  // that any valid embedded PNG will contain that many bytes as well because:
-  //    BITMAPINFOSIZE
-  //      <
-  //    signature (8 bytes) +
-  //    IHDR (12 bytes header + 13 bytes data)
-  //    IDAT (12 bytes header)
+  // Prepare a new iterator for the contained decoder to advance as it wills.
+  // Cloning at the point ensures it will begin at the resource offset.
+  mContainedIterator.emplace(mLexer.Clone(*mIterator, mDirEntry.mBytesInRes));
 
   // We use the first PNGSIGNATURESIZE bytes to determine whether this resource
   // is a PNG or a BMP.
   bool isPNG = !memcmp(aData, nsPNGDecoder::pngSignatureBytes,
                        PNGSIGNATURESIZE);
   if (isPNG) {
-    if (mDirEntry.mBytesInRes <= BITMAPINFOSIZE) {
+    if (mDirEntry.mBytesInRes <= PNGSIGNATURESIZE) {
       return Transition::TerminateFailure();
     }
 
-    // Prepare a new iterator for the contained decoder to advance as it wills.
-    // Cloning at the point ensures it will begin at the resource offset.
-    SourceBufferIterator containedIterator
-      = mLexer.Clone(*mIterator, mDirEntry.mBytesInRes);
-
     // Create a PNG decoder which will do the rest of the work for us.
     mContainedDecoder =
       DecoderFactory::CreateDecoderForICOResource(DecoderType::PNG,
-                                                  Move(containedIterator),
+                                                  Move(*mContainedIterator),
                                                   WrapNotNull(this),
                                                   Some(GetRealSize()));
 
     // Read in the rest of the PNG unbuffered.
-    size_t toRead = mDirEntry.mBytesInRes - BITMAPINFOSIZE;
+    size_t toRead = mDirEntry.mBytesInRes - PNGSIGNATURESIZE;
     return Transition::ToUnbuffered(ICOState::FINISHED_RESOURCE,
                                     ICOState::READ_RESOURCE,
                                     toRead);
   } else {
     // Make sure we have a sane size for the bitmap information header.
     int32_t bihSize = LittleEndian::readUint32(aData);
     if (bihSize != static_cast<int32_t>(BITMAPINFOSIZE)) {
       return Transition::TerminateFailure();
     }
 
+    // Buffer the first part of the bitmap information header.
+    memcpy(mBIHraw, aData, PNGSIGNATURESIZE);
+
     // Read in the rest of the bitmap information header.
-    return ReadBIH(aData);
+    return Transition::To(ICOState::READ_BIH,
+                          BITMAPINFOSIZE - PNGSIGNATURESIZE);
   }
 }
 
 LexerTransition<ICOState>
 nsICODecoder::ReadResource()
 {
   if (!FlushContainedDecoder()) {
     return Transition::TerminateFailure();
   }
 
   return Transition::ContinueUnbuffered(ICOState::READ_RESOURCE);
 }
 
 LexerTransition<ICOState>
 nsICODecoder::ReadBIH(const char* aData)
 {
+  // Buffer the rest of the bitmap information header.
+  memcpy(mBIHraw + PNGSIGNATURESIZE, aData, BITMAPINFOSIZE - PNGSIGNATURESIZE);
+
   // Extract the BPP from the BIH header; it should be trusted over the one
   // we have from the ICO header which is usually set to 0.
-  mBPP = LittleEndian::readUint16(aData + 14);
-
-  // Check to make sure we have valid color settings.
-  uint16_t numColors = GetNumColors();
-  if (numColors == uint16_t(-1)) {
-    return Transition::TerminateFailure();
-  }
-
-  // The color table is present only if BPP is <= 8.
-  MOZ_ASSERT_IF(mBPP > 8, numColors == 0);
+  mBPP = LittleEndian::readUint16(mBIHraw + 14);
 
   // The ICO format when containing a BMP does not include the 14 byte
   // bitmap file header. So we create the BMP decoder via the constructor that
   // tells it to skip this, and pass in the required data (dataOffset) that
   // would have been present in the header.
-  uint32_t dataOffset = bmp::FILE_HEADER_LENGTH + BITMAPINFOSIZE + 4 * numColors;
-
-  // Prepare a new iterator for the contained decoder to advance as it wills.
-  // Cloning at the point ensures it will begin at the resource offset.
-  SourceBufferIterator containedIterator
-    = mLexer.Clone(*mIterator, mDirEntry.mBytesInRes);
+  uint32_t dataOffset = bmp::FILE_HEADER_LENGTH + BITMAPINFOSIZE;
+  if (mBPP <= 8) {
+    // The color table is present only if BPP is <= 8.
+    uint16_t numColors = GetNumColors();
+    if (numColors == (uint16_t)-1) {
+      return Transition::TerminateFailure();
+    }
+    dataOffset += 4 * numColors;
+  }
 
   // Create a BMP decoder which will do most of the work for us; the exception
   // is the AND mask, which isn't present in standalone BMPs.
   mContainedDecoder =
     DecoderFactory::CreateDecoderForICOResource(DecoderType::BMP,
-                                                Move(containedIterator),
+                                                Move(*mContainedIterator),
                                                 WrapNotNull(this),
                                                 Some(GetRealSize()),
                                                 Some(dataOffset));
 
   RefPtr<nsBMPDecoder> bmpDecoder =
     static_cast<nsBMPDecoder*>(mContainedDecoder.get());
 
   // Ensure the decoder has parsed at least the BMP's bitmap info header.
   if (!FlushContainedDecoder()) {
     return Transition::TerminateFailure();
   }
 
+  // Check to make sure we have valid color settings.
+  uint16_t numColors = GetNumColors();
+  if (numColors == uint16_t(-1)) {
+    return Transition::TerminateFailure();
+  }
+
   // Do we have an AND mask on this BMP? If so, we need to read it after we read
   // the BMP data itself.
   uint32_t bmpDataLength = bmpDecoder->GetCompressedImageSize() + 4 * numColors;
   bool hasANDMask = (BITMAPINFOSIZE + bmpDataLength) < mDirEntry.mBytesInRes;
   ICOState afterBMPState = hasANDMask ? ICOState::PREPARE_FOR_MASK
                                       : ICOState::FINISHED_RESOURCE;
 
   // Read in the rest of the BMP unbuffered.
@@ -560,21 +557,23 @@ nsICODecoder::DoDecode(SourceBufferItera
     switch (aState) {
       case ICOState::HEADER:
         return ReadHeader(aData);
       case ICOState::DIR_ENTRY:
         return ReadDirEntry(aData);
       case ICOState::SKIP_TO_RESOURCE:
         return Transition::ContinueUnbuffered(ICOState::SKIP_TO_RESOURCE);
       case ICOState::FOUND_RESOURCE:
-        return Transition::To(ICOState::SNIFF_RESOURCE, BITMAPINFOSIZE);
+        return Transition::To(ICOState::SNIFF_RESOURCE, PNGSIGNATURESIZE);
       case ICOState::SNIFF_RESOURCE:
         return SniffResource(aData);
       case ICOState::READ_RESOURCE:
         return ReadResource();
+      case ICOState::READ_BIH:
+        return ReadBIH(aData);
       case ICOState::PREPARE_FOR_MASK:
         return PrepareForMask();
       case ICOState::READ_MASK_ROW:
         return ReadMaskRow(aData);
       case ICOState::FINISH_MASK:
         return FinishMask();
       case ICOState::SKIP_MASK:
         return Transition::ContinueUnbuffered(ICOState::SKIP_MASK);
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -23,16 +23,17 @@ class RasterImage;
 enum class ICOState
 {
   HEADER,
   DIR_ENTRY,
   SKIP_TO_RESOURCE,
   FOUND_RESOURCE,
   SNIFF_RESOURCE,
   READ_RESOURCE,
+  READ_BIH,
   PREPARE_FOR_MASK,
   READ_MASK_ROW,
   FINISH_MASK,
   SKIP_MASK,
   FINISHED_RESOURCE
 };
 
 class nsICODecoder : public Decoder
@@ -95,17 +96,19 @@ private:
   LexerTransition<ICOState> ReadBIH(const char* aData);
   LexerTransition<ICOState> PrepareForMask();
   LexerTransition<ICOState> ReadMaskRow(const char* aData);
   LexerTransition<ICOState> FinishMask();
   LexerTransition<ICOState> FinishResource();
 
   StreamingLexer<ICOState, 32> mLexer; // The lexer.
   RefPtr<Decoder> mContainedDecoder; // Either a BMP or PNG decoder.
+  Maybe<SourceBufferIterator> mContainedIterator; // Iterator for the subdecoder.
   UniquePtr<uint8_t[]> mMaskBuffer;    // A temporary buffer for the alpha mask.
+  char mBIHraw[bmp::InfoHeaderLength::WIN_ICO]; // The bitmap information header.
   IconDirEntry mDirEntry;              // The dir entry for the selected resource.
   gfx::IntSize mBiggestResourceSize;   // Used to select the intrinsic size.
   gfx::IntSize mBiggestResourceHotSpot; // Used to select the intrinsic size.
   uint16_t mBiggestResourceColorDepth; // Used to select the intrinsic size.
   int32_t mBestResourceDelta;          // Used to select the best resource.
   uint16_t mBestResourceColorDepth;    // Used to select the best resource.
   uint16_t mNumIcons; // Stores the number of icons in the ICO file.
   uint16_t mCurrIcon; // Stores the current dir entry index we are processing.