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.

--HG--
extra : rebase_source : 629a2ac387a9c8ee1a520c70733adb10cc156aa8

image/BMPHeaders.h

@@ -26,6 +26,8 @@ struct InfoHeaderLength {
     // 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,
   };
 };
 

image/decoders/nsBMPDecoder.cpp

@@ -172,9 +172,12 @@ GetBMPLog()
   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)
@@ -188,6 +191,28 @@ nsBMPDecoder::nsBMPDecoder(RasterImage* aImage)
 {
 }
 
+// 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()
 {
 }
@@ -464,9 +489,6 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, uint32_t aCount)
   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)
 {

image/decoders/nsBMPDecoder.h

@@ -141,7 +141,8 @@ public:
   /// 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
@@ -163,14 +164,6 @@ 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,
@@ -186,6 +179,21 @@ private:
     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);

image/decoders/nsICODecoder.cpp

@@ -21,7 +21,7 @@ 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
@@ -110,42 +110,6 @@ nsICODecoder::GetFinalStateFromContainedDecoder()
   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.
@@ -389,19 +353,6 @@ nsICODecoder::SniffResource(const char* aData)
                                     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)) {
@@ -444,17 +395,30 @@ nsICODecoder::ReadBIH(const char* aData)
   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
+  // bitmap file header. So we create the BMP decoder via the constructor that
-  // generate this header ourselves and feed it to the BMP decoder.
+  // tells it to skip this, and pass in the required data (dataOffset) that
-  int8_t bfhBuffer[BMPFILEHEADERSIZE];
+  // would have been present in the header.
-  if (!FillBitmapFileHeaderBuffer(bfhBuffer)) {
+  uint32_t dataOffset = bmp::FILE_HEADER_LENGTH + BITMAPINFOSIZE;
-    return Transition::Terminate(ICOState::FAILURE);
+  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),
+  // Create a BMP decoder which will do most of the work for us; the exception
-                               sizeof(bfhBuffer))) {
+  // is the AND mask, which isn't present in standalone BMPs.
-    return Transition::Terminate(ICOState::FAILURE);
+  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
@@ -477,8 +441,6 @@ nsICODecoder::ReadBIH(const char* aData)
   // 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.

image/decoders/nsICODecoder.h

@@ -87,8 +87,6 @@ private:
   // 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.
@@ -120,7 +118,7 @@ private:
   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.