Bug 1215334 (part 2) - Avoid creating a fake header for BMP files in ICO files. r=seth.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 15 Oct 2015 15:43:31 -0700
changeset 305040 255b8f808c0bc4bab320ed281e89b62f8053ef27
parent 305039 09dc3489f429418d610e7e6b48ce914e0e44200d
child 305041 5fea785542c270c133ed51c6d84f111194c0727f
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersseth
bugs1215334
milestone44.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 1215334 (part 2) - Avoid creating a fake header for BMP files in ICO files. r=seth. This requires delaying the creation of the BMP decoder used by the ICO decoder.
image/BMPHeaders.h
image/decoders/nsBMPDecoder.cpp
image/decoders/nsBMPDecoder.h
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
--- a/image/BMPHeaders.h
+++ b/image/BMPHeaders.h
@@ -21,16 +21,18 @@ struct InfoHeaderLength {
     WIN_V2 = 12,
     WIN_V3 = 40,
     WIN_V4 = 108,
     WIN_V5 = 124,
 
     // OS2_V1 is omitted; it's the same as WIN_V2.
     OS2_V2_MIN = 16,    // Minimum allowed value for OS2v2.
     OS2_V2_MAX = 64,    // Maximum allowed value for OS2v2.
+
+    WIN_ICO = WIN_V3,
   };
 };
 
 } // namespace bmp
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_BMPHeaders_h
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -167,32 +167,57 @@ GetBMPLog()
 {
   static PRLogModuleInfo* sBMPLog;
   if (!sBMPLog) {
     sBMPLog = PR_NewLogModule("BMPDecoder");
   }
   return sBMPLog;
 }
 
-nsBMPDecoder::nsBMPDecoder(RasterImage* aImage)
+// The length of the mBIHSize field in the info header.
+static const uint32_t BIHSIZE_FIELD_LENGTH = 4;
+
+nsBMPDecoder::nsBMPDecoder(RasterImage* aImage, State aState, size_t aLength)
   : Decoder(aImage)
-  , mLexer(Transition::To(State::FILE_HEADER, FILE_HEADER_LENGTH))
+  , mLexer(Transition::To(aState, aLength))
   , mIsWithinICO(false)
   , mMayHaveTransparency(false)
   , mDoesHaveTransparency(false)
   , mNumColors(0)
   , mColors(nullptr)
   , mBytesPerColor(0)
   , mPreGapLength(0)
   , mCurrentRow(0)
   , mCurrentPos(0)
   , mAbsoluteModeNumPixels(0)
 {
 }
 
+// Constructor for normal BMP files.
+nsBMPDecoder::nsBMPDecoder(RasterImage* aImage)
+  : nsBMPDecoder(aImage, State::FILE_HEADER, FILE_HEADER_LENGTH)
+{
+}
+
+// Constructor used for WinBMPv3-ICO files, which lack a file header.
+nsBMPDecoder::nsBMPDecoder(RasterImage* aImage, uint32_t aDataOffset)
+  : nsBMPDecoder(aImage, State::INFO_HEADER_SIZE, BIHSIZE_FIELD_LENGTH)
+{
+  SetIsWithinICO();
+
+  // Even though the file header isn't present in this case, the dataOffset
+  // field is set as if it is, and so we must increment mPreGapLength
+  // accordingly.
+  mPreGapLength += FILE_HEADER_LENGTH;
+
+  // This is the one piece of data we normally get from a BMP file header, so
+  // it must be provided via an argument.
+  mH.mDataOffset = aDataOffset;
+}
+
 nsBMPDecoder::~nsBMPDecoder()
 {
 }
 
 // Obtains the bits per pixel from the internal BIH header.
 int32_t
 nsBMPDecoder::GetBitsPerPixel() const
 {
@@ -459,19 +484,16 @@ nsBMPDecoder::WriteInternal(const char* 
     return;
   }
 
   MOZ_ASSERT(*terminalState == State::SUCCESS);
 
   return;
 }
 
-// The length of the mBIHSize field in the info header.
-static const uint32_t BIHSIZE_FIELD_LENGTH = 4;
-
 LexerTransition<nsBMPDecoder::State>
 nsBMPDecoder::ReadFileHeader(const char* aData, size_t aLength)
 {
   mPreGapLength += aLength;
 
   bool signatureOk = aData[0] == 'B' && aData[1] == 'M';
   if (!signatureOk) {
     PostDataError();
--- a/image/decoders/nsBMPDecoder.h
+++ b/image/decoders/nsBMPDecoder.h
@@ -136,17 +136,18 @@ public:
 
   /// Obtains the internal output image buffer.
   uint32_t* GetImageData();
   size_t GetImageDataLength() const { return mImageDataLength; }
 
   /// Obtains the size of the compressed image resource.
   int32_t GetCompressedImageSize() const;
 
-  /// Mark this BMP as being within an ICO file.
+  /// Mark this BMP as being within an ICO file. Only used for testing purposes
+  /// because the ICO-specific constructor does this marking automatically.
   void SetIsWithinICO() { mIsWithinICO = true; }
 
   /// Did the BMP file have alpha data of any kind? (Only use this after the
   /// bitmap has been fully decoded.)
   bool HasTransparency() const { return mDoesHaveTransparency; }
 
   /// Force transparency from outside. (Used by the ICO decoder.)
   void SetHasTransparency()
@@ -158,39 +159,46 @@ public:
   virtual void WriteInternal(const char* aBuffer,
                              uint32_t aCount) override;
   virtual void FinishInternal() override;
 
 private:
   friend class DecoderFactory;
   friend class nsICODecoder;
 
-  // Decoders should only be instantiated via DecoderFactory.
-  // XXX(seth): nsICODecoder is temporarily an exception to this rule.
-  explicit nsBMPDecoder(RasterImage* aImage);
-
-  uint32_t* RowBuffer();
-
-  void FinishRow();
-
   enum class State {
     FILE_HEADER,
     INFO_HEADER_SIZE,
     INFO_HEADER_REST,
     BITFIELDS,
     COLOR_TABLE,
     GAP,
     PIXEL_ROW,
     RLE_SEGMENT,
     RLE_DELTA,
     RLE_ABSOLUTE,
     SUCCESS,
     FAILURE
   };
 
+  // This is the constructor used by DecoderFactory.
+  explicit nsBMPDecoder(RasterImage* aImage);
+
+  // This is the constructor used by nsICODecoder.
+  // XXX(seth): nsICODecoder is temporarily an exception to the rule that
+  //            decoders should only be instantiated via DecoderFactory.
+  nsBMPDecoder(RasterImage* aImage, uint32_t aDataOffset);
+
+  // Helper constructor called by the other two.
+  nsBMPDecoder(RasterImage* aImage, State aState, size_t aLength);
+
+  uint32_t* RowBuffer();
+
+  void FinishRow();
+
   LexerTransition<State> ReadFileHeader(const char* aData, size_t aLength);
   LexerTransition<State> ReadInfoHeaderSize(const char* aData, size_t aLength);
   LexerTransition<State> ReadInfoHeaderRest(const char* aData, size_t aLength);
   LexerTransition<State> ReadBitfields(const char* aData, size_t aLength);
   LexerTransition<State> ReadColorTable(const char* aData, size_t aLength);
   LexerTransition<State> SkipGap();
   LexerTransition<State> ReadPixelRow(const char* aData);
   LexerTransition<State> ReadRLESegment(const char* aData);
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -16,17 +16,17 @@
 
 using namespace mozilla::gfx;
 
 namespace mozilla {
 namespace image {
 
 // Constants.
 static const uint32_t ICOHEADERSIZE = 6;
-static const uint32_t BITMAPINFOSIZE = 40;
+static const uint32_t BITMAPINFOSIZE = bmp::InfoHeaderLength::WIN_ICO;
 
 // ----------------------------------------
 // Actual Data Processing
 // ----------------------------------------
 
 uint32_t
 nsICODecoder::CalcAlphaRowSize()
 {
@@ -105,52 +105,16 @@ nsICODecoder::GetFinalStateFromContained
   mDecodeAborted = mContainedDecoder->WasAborted();
   mProgress |= mContainedDecoder->TakeProgress();
   mInvalidRect.UnionRect(mInvalidRect, mContainedDecoder->TakeInvalidRect());
   mCurrentFrame = mContainedDecoder->GetCurrentFrameRef();
 
   MOZ_ASSERT(HasError() || !mCurrentFrame || mCurrentFrame->IsImageComplete());
 }
 
-// Returns a buffer filled with the bitmap file header in little endian:
-// Signature 2 bytes 'BM'
-// FileSize      4 bytes File size in bytes
-// reserved      4 bytes unused (=0)
-// DataOffset    4 bytes File offset to Raster Data
-// Returns true if successful
-bool
-nsICODecoder::FillBitmapFileHeaderBuffer(int8_t* bfh)
-{
-  memset(bfh, 0, 14);
-  bfh[0] = 'B';
-  bfh[1] = 'M';
-  int32_t dataOffset = 0;
-  int32_t fileSize = 0;
-  dataOffset = bmp::FILE_HEADER_LENGTH + BITMAPINFOSIZE;
-
-  // The color table is present only if BPP is <= 8
-  if (mDirEntry.mBitCount <= 8) {
-    uint16_t numColors = GetNumColors();
-    if (numColors == (uint16_t)-1) {
-      return false;
-    }
-    dataOffset += 4 * numColors;
-    fileSize = dataOffset + GetRealWidth() * GetRealHeight();
-  } else {
-    fileSize = dataOffset + (mDirEntry.mBitCount * GetRealWidth() *
-                             GetRealHeight()) / 8;
-  }
-
-  NativeEndian::swapToLittleEndianInPlace(&fileSize, 1);
-  memcpy(bfh + 2, &fileSize, sizeof(fileSize));
-  NativeEndian::swapToLittleEndianInPlace(&dataOffset, 1);
-  memcpy(bfh + 10, &dataOffset, sizeof(dataOffset));
-  return true;
-}
-
 // A BMP inside of an ICO has *2 height because of the AND mask
 // that follows the actual bitmap.  The BMP shouldn't know about
 // this difference though.
 bool
 nsICODecoder::FixBitmapHeight(int8_t* bih)
 {
   // Get the height from the BMP file information header
   int32_t height;
@@ -384,29 +348,16 @@ nsICODecoder::SniffResource(const char* 
     }
 
     // Read in the rest of the PNG unbuffered.
     size_t toRead = mDirEntry.mBytesInRes - PNGSIGNATURESIZE;
     return Transition::ToUnbuffered(ICOState::FINISHED_RESOURCE,
                                     ICOState::READ_PNG,
                                     toRead);
   } else {
-    // 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.
-    nsBMPDecoder* bmpDecoder = new nsBMPDecoder(mImage);
-    mContainedDecoder = bmpDecoder;
-    bmpDecoder->SetIsWithinICO();
-    mContainedDecoder->SetMetadataDecode(IsMetadataDecode());
-    mContainedDecoder->SetDecoderFlags(GetDecoderFlags());
-    mContainedDecoder->SetSurfaceFlags(GetSurfaceFlags());
-    if (mDownscaler) {
-      mContainedDecoder->SetTargetSize(mDownscaler->TargetSize());
-    }
-    mContainedDecoder->Init();
-
     // Make sure we have a sane size for the bitmap information header.
     int32_t bihSize = ReadBIHSize(aData);
     if (bihSize != static_cast<int32_t>(BITMAPINFOSIZE)) {
       return Transition::Terminate(ICOState::FAILURE);
     }
 
     // Buffer the first part of the bitmap information header.
     memcpy(mBIHraw, aData, PNGSIGNATURESIZE);
@@ -439,27 +390,40 @@ nsICODecoder::ReadBIH(const char* aData)
   // Buffer the rest of the bitmap information header.
   memcpy(mBIHraw + PNGSIGNATURESIZE, aData, BITMAPINFOSIZE - PNGSIGNATURESIZE);
 
   // Extracting the BPP from the BIH header; it should be trusted over the one
   // we have from the ICO header.
   mBPP = ReadBPP(mBIHraw);
 
   // The ICO format when containing a BMP does not include the 14 byte
-  // bitmap file header. To use the code of the BMP decoder we need to
-  // generate this header ourselves and feed it to the BMP decoder.
-  int8_t bfhBuffer[BMPFILEHEADERSIZE];
-  if (!FillBitmapFileHeaderBuffer(bfhBuffer)) {
-    return Transition::Terminate(ICOState::FAILURE);
+  // 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;
+  if (mDirEntry.mBitCount <= 8) {
+    // The color table is present only if BPP is <= 8.
+    uint16_t numColors = GetNumColors();
+    if (numColors == (uint16_t)-1) {
+      return Transition::Terminate(ICOState::FAILURE);
+    }
+    dataOffset += 4 * numColors;
   }
 
-  if (!WriteToContainedDecoder(reinterpret_cast<const char*>(bfhBuffer),
-                               sizeof(bfhBuffer))) {
-    return Transition::Terminate(ICOState::FAILURE);
+  // 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.
+  RefPtr<nsBMPDecoder> bmpDecoder = new nsBMPDecoder(mImage, dataOffset);
+  mContainedDecoder = bmpDecoder;
+  mContainedDecoder->SetMetadataDecode(IsMetadataDecode());
+  mContainedDecoder->SetDecoderFlags(GetDecoderFlags());
+  mContainedDecoder->SetSurfaceFlags(GetSurfaceFlags());
+  if (mDownscaler) {
+    mContainedDecoder->SetTargetSize(mDownscaler->TargetSize());
   }
+  mContainedDecoder->Init();
 
   // Fix the ICO height from the BIH. It needs to be halved so our BMP decoder
   // will understand, because the BMP decoder doesn't expect the alpha mask that
   // follows the BMP data in an ICO.
   if (!FixBitmapHeight(reinterpret_cast<int8_t*>(mBIHraw))) {
     return Transition::Terminate(ICOState::FAILURE);
   }
 
@@ -472,18 +436,16 @@ nsICODecoder::ReadBIH(const char* aData)
   if (!WriteToContainedDecoder(mBIHraw, sizeof(mBIHraw))) {
     return Transition::Terminate(ICOState::FAILURE);
   }
 
   // Sometimes the ICO BPP header field is not filled out so we should trust the
   // contained resource over our own information.
   // XXX(seth): Is this ever different than the value we obtained from
   // ReadBPP() above?
-  RefPtr<nsBMPDecoder> bmpDecoder =
-    static_cast<nsBMPDecoder*>(mContainedDecoder.get());
   mBPP = bmpDecoder->GetBitsPerPixel();
 
   // Check to make sure we have valid color settings.
   uint16_t numColors = GetNumColors();
   if (numColors == uint16_t(-1)) {
     return Transition::Terminate(ICOState::FAILURE);
   }
 
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -82,18 +82,16 @@ private:
 
   // Writes to the contained decoder and sets the appropriate errors
   // Returns true if there are no errors.
   bool WriteToContainedDecoder(const char* aBuffer, uint32_t aCount);
 
   // Gets decoder state from the contained decoder so it's visible externally.
   void GetFinalStateFromContainedDecoder();
 
-  // Creates a bitmap file header buffer, returns true if successful
-  bool FillBitmapFileHeaderBuffer(int8_t* bfh);
   // Fixes the ICO height to match that of the BIH.
   // and also fixes the BIH height to be /2 of what it was.
   // See definition for explanation.
   // Returns false if invalid information is contained within.
   bool FixBitmapHeight(int8_t* bih);
   // Fixes the ICO width to match that of the BIH.
   // Returns false if invalid information is contained within.
   bool FixBitmapWidth(int8_t* bih);
@@ -115,17 +113,17 @@ private:
   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.
   UniquePtr<uint8_t[]> mMaskBuffer;    // A temporary buffer for the alpha mask.
-  char mBIHraw[40];                    // The bitmap information header.
+  char mBIHraw[bmp::InfoHeaderLength::WIN_ICO]; // The bitmap information header.
   IconDirEntry mDirEntry;              // The dir entry for the selected resource.
   IntSize mBiggestResourceSize;        // Used to select the intrinsic size.
   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.