Bug 1315554 - Part 9. Get the ICO size from the resource instead of the dir entry if unspecified. r=tnikkel
☠☠ backed out by e2a6c6b64735 ☠ ☠
authorAndrew Osmond <aosmond@mozilla.com>
Sat, 22 Jul 2017 00:14:59 -0400
changeset 419094 18614b05270dc639d6cf9266f4293cf854e3a6c5
parent 419093 d46b7e02802cdb35ad182705bf27dd583f5533e3
child 419095 abc949687bdc774389bdfe12a49f7408373d514f
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)
reviewerstnikkel
bugs1315554
milestone56.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 1315554 - Part 9. Get the ICO size from the resource instead of the dir entry if unspecified. r=tnikkel
image/decoders/nsICODecoder.cpp
image/decoders/nsICODecoder.h
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -50,19 +50,17 @@ nsICODecoder::GetNumColors()
   }
   return numColors;
 }
 
 nsICODecoder::nsICODecoder(RasterImage* aImage)
   : Decoder(aImage)
   , mLexer(Transition::To(ICOState::HEADER, ICOHEADERSIZE),
            Transition::TerminateSuccess())
-  , mBiggestResourceColorDepth(0)
-  , mBestResourceDelta(INT_MIN)
-  , mBestResourceColorDepth(0)
+  , mDirEntry(nullptr)
   , mNumIcons(0)
   , mCurrIcon(0)
   , mBPP(0)
   , mMaskRowSize(0)
   , mCurrMaskLine(0)
   , mIsCursor(false)
   , mHasMaskAlpha(false)
 { }
@@ -146,147 +144,222 @@ nsICODecoder::FirstResourceOffset() cons
   return ICOHEADERSIZE + mNumIcons * ICODIRENTRYSIZE;
 }
 
 LexerTransition<ICOState>
 nsICODecoder::ReadDirEntry(const char* aData)
 {
   mCurrIcon++;
 
-  // Read the directory entry.
-  IconDirEntry e;
-  e.mWidth       = aData[0];
-  e.mHeight      = aData[1];
-  e.mColorCount  = aData[2];
-  e.mReserved    = aData[3];
-  e.mPlanes      = LittleEndian::readUint16(aData + 4);
-  e.mBitCount    = LittleEndian::readUint16(aData + 6);
-  e.mBytesInRes  = LittleEndian::readUint32(aData + 8);
-  e.mImageOffset = LittleEndian::readUint32(aData + 12);
-
-  // If an explicit output size was specified, we'll try to select the resource
-  // that matches it best below.
-  const Maybe<IntSize> desiredSize = ExplicitOutputSize();
-
-  // Determine if this is the biggest resource we've seen so far. We always use
-  // the biggest resource for the intrinsic size, and if we don't have a
-  // specific desired size, we select it as the best resource as well.
-  IntSize entrySize(GetRealWidth(e), GetRealHeight(e));
-  if (e.mBitCount >= mBiggestResourceColorDepth &&
-      entrySize.width * entrySize.height >=
-        mBiggestResourceSize.width * mBiggestResourceSize.height) {
-    mBiggestResourceSize = entrySize;
-    mBiggestResourceColorDepth = e.mBitCount;
-    mBiggestResourceHotSpot = IntSize(e.mXHotspot, e.mYHotspot);
-
-    if (!desiredSize) {
-      mDirEntry = e;
-    }
-  }
-
-  mImageMetadata.AddNativeSize(entrySize);
-
-  if (desiredSize) {
-    // Calculate the delta between this resource's size and the desired size, so
-    // we can see if it is better than our current-best option.  In the case of
-    // several equally-good resources, we use the last one. "Better" in this
-    // case is determined by |delta|, a measure of the difference in size
-    // between the entry we've found and the desired size. We will choose the
-    // smallest resource that is greater than or equal to the desired size (i.e.
-    // we assume it's better to downscale a larger icon than to upscale a
-    // smaller one).
-    int32_t delta = std::min(entrySize.width - desiredSize->width,
-                             entrySize.height - desiredSize->height);
-    if (e.mBitCount >= mBestResourceColorDepth &&
-        ((mBestResourceDelta < 0 && delta >= mBestResourceDelta) ||
-         (delta >= 0 && delta <= mBestResourceDelta))) {
-      mBestResourceDelta = delta;
-      mBestResourceColorDepth = e.mBitCount;
-      mDirEntry = e;
+  // Ensure the resource has an offset past the ICO headers.
+  uint32_t offset = LittleEndian::readUint32(aData + 12);
+  if (offset >= FirstResourceOffset()) {
+    // Read the directory entry.
+    IconDirEntryEx e;
+    e.mWidth       = aData[0];
+    e.mHeight      = aData[1];
+    e.mColorCount  = aData[2];
+    e.mReserved    = aData[3];
+    e.mPlanes      = LittleEndian::readUint16(aData + 4);
+    e.mBitCount    = LittleEndian::readUint16(aData + 6);
+    e.mBytesInRes  = LittleEndian::readUint32(aData + 8);
+    e.mImageOffset = offset;
+    e.mSize        = IntSize(e.mWidth, e.mHeight);
+    if (e.mWidth == 0 || e.mHeight == 0) {
+      mUnsizedDirEntries.AppendElement(e);
+    } else {
+      mDirEntries.AppendElement(e);
     }
   }
 
   if (mCurrIcon == mNumIcons) {
-    // Ensure the resource we selected has an offset past the ICO headers.
-    if (mDirEntry.mImageOffset < FirstResourceOffset()) {
-      return Transition::TerminateFailure();
-    }
-
-    // If this is a cursor, set the hotspot. We use the hotspot from the biggest
-    // resource since we also use that resource for the intrinsic size.
-    if (mIsCursor) {
-      mImageMetadata.SetHotspot(mBiggestResourceHotSpot.width,
-                                mBiggestResourceHotSpot.height);
+    if (mUnsizedDirEntries.IsEmpty()) {
+      return Transition::To(ICOState::FINISHED_DIR_ENTRY, 0);
     }
-
-    // We always report the biggest resource's size as the intrinsic size; this
-    // is necessary for downscale-during-decode to work since we won't even
-    // attempt to *upscale* while decoding.
-    PostSize(mBiggestResourceSize.width, mBiggestResourceSize.height);
-    if (HasError()) {
-      return Transition::TerminateFailure();
-    }
-
-    if (IsMetadataDecode()) {
-      return Transition::TerminateSuccess();
-    }
-
-    // If the resource we selected matches the output size perfectly, we don't
-    // need to do any downscaling.
-    if (GetRealSize() == OutputSize()) {
-      MOZ_ASSERT_IF(desiredSize, GetRealSize() == *desiredSize);
-      MOZ_ASSERT_IF(!desiredSize, GetRealSize() == Size());
-      mDownscaler.reset();
-    }
-
-    size_t offsetToResource = mDirEntry.mImageOffset - FirstResourceOffset();
-    return Transition::ToUnbuffered(ICOState::FOUND_RESOURCE,
-                                    ICOState::SKIP_TO_RESOURCE,
-                                    offsetToResource);
+    return Transition::To(ICOState::ITERATE_UNSIZED_DIR_ENTRY, 0);
   }
 
   return Transition::To(ICOState::DIR_ENTRY, ICODIRENTRYSIZE);
 }
 
 LexerTransition<ICOState>
+nsICODecoder::IterateUnsizedDirEntry()
+{
+  MOZ_ASSERT(!mUnsizedDirEntries.IsEmpty());
+
+  if (!mDirEntry) {
+    // The first time we are here, there is no entry selected. We must prepare a
+    // new iterator for the contained decoder to advance as it wills. Cloning at
+    // this point ensures it will begin at the end of the dir entries.
+    mReturnIterator.emplace(mLexer.Clone(*mIterator, SIZE_MAX));
+  } else {
+    // We have already selected an entry which means a metadata decoder has
+    // finished. Verify the size is valid and if so, add to the discovered
+    // resources.
+    if (mDirEntry->mSize.width > 0 && mDirEntry->mSize.height > 0) {
+      mDirEntries.AppendElement(*mDirEntry);
+    }
+
+    // Remove the entry from the unsized list either way.
+    mDirEntry = nullptr;
+    mUnsizedDirEntries.RemoveElementAt(0);
+
+    // Our iterator is at an unknown point, so reset it to the point that we
+    // saved.
+    mIterator.reset();
+    mIterator.emplace(mLexer.Clone(*mReturnIterator, SIZE_MAX));
+  }
+
+  // There are no more unsized entries, so we can finally decide which entry to
+  // select for decoding.
+  if (mUnsizedDirEntries.IsEmpty()) {
+    mReturnIterator.reset();
+    return Transition::To(ICOState::FINISHED_DIR_ENTRY, 0);
+  }
+
+  // Move to the resource data to start metadata decoding.
+  mDirEntry = &mUnsizedDirEntries[0];
+  size_t offsetToResource = mDirEntry->mImageOffset - FirstResourceOffset();
+  return Transition::ToUnbuffered(ICOState::FOUND_RESOURCE,
+                                  ICOState::SKIP_TO_RESOURCE,
+                                  offsetToResource);
+}
+
+LexerTransition<ICOState>
+nsICODecoder::FinishDirEntry()
+{
+  MOZ_ASSERT(!mDirEntry);
+
+  if (mDirEntries.IsEmpty()) {
+    return Transition::TerminateFailure();
+  }
+
+  // If an explicit output size was specified, we'll try to select the resource
+  // that matches it best below.
+  const Maybe<IntSize> desiredSize = ExplicitOutputSize();
+
+  // Determine the biggest resource. We always use the biggest resource for the
+  // intrinsic size, and if we don't have a specific desired size, we select it
+  // as the best resource as well.
+  int32_t bestDelta = INT32_MIN;
+  IconDirEntryEx* biggestEntry = nullptr;
+
+  for (size_t i = 0; i < mDirEntries.Length(); ++i) {
+    IconDirEntryEx& e = mDirEntries[i];
+    mImageMetadata.AddNativeSize(e.mSize);
+
+    if (!biggestEntry ||
+        (e.mBitCount >= biggestEntry->mBitCount &&
+         e.mSize.width * e.mSize.height >=
+           biggestEntry->mSize.width * biggestEntry->mSize.height)) {
+      biggestEntry = &e;
+
+      if (!desiredSize) {
+        mDirEntry = &e;
+      }
+    }
+
+    if (desiredSize) {
+      // Calculate the delta between this resource's size and the desired size, so
+      // we can see if it is better than our current-best option.  In the case of
+      // several equally-good resources, we use the last one. "Better" in this
+      // case is determined by |delta|, a measure of the difference in size
+      // between the entry we've found and the desired size. We will choose the
+      // smallest resource that is greater than or equal to the desired size (i.e.
+      // we assume it's better to downscale a larger icon than to upscale a
+      // smaller one).
+      int32_t delta = std::min(e.mSize.width - desiredSize->width,
+                               e.mSize.height - desiredSize->height);
+      if (!mDirEntry ||
+          (e.mBitCount >= mDirEntry->mBitCount &&
+           ((bestDelta < 0 && delta >= bestDelta) ||
+            (delta >= 0 && delta <= bestDelta)))) {
+        mDirEntry = &e;
+        bestDelta = delta;
+      }
+    }
+  }
+
+  MOZ_ASSERT(mDirEntry);
+  MOZ_ASSERT(biggestEntry);
+
+  // If this is a cursor, set the hotspot. We use the hotspot from the biggest
+  // resource since we also use that resource for the intrinsic size.
+  if (mIsCursor) {
+    mImageMetadata.SetHotspot(biggestEntry->mXHotspot,
+                              biggestEntry->mYHotspot);
+  }
+
+  // We always report the biggest resource's size as the intrinsic size; this
+  // is necessary for downscale-during-decode to work since we won't even
+  // attempt to *upscale* while decoding.
+  PostSize(biggestEntry->mSize.width, biggestEntry->mSize.height);
+  if (HasError()) {
+    return Transition::TerminateFailure();
+  }
+
+  if (IsMetadataDecode()) {
+    return Transition::TerminateSuccess();
+  }
+
+  // If the resource we selected matches the output size perfectly, we don't
+  // need to do any downscaling.
+  if (mDirEntry->mSize == OutputSize()) {
+    MOZ_ASSERT_IF(desiredSize, mDirEntry->mSize == *desiredSize);
+    MOZ_ASSERT_IF(!desiredSize, mDirEntry->mSize == Size());
+    mDownscaler.reset();
+  }
+
+  size_t offsetToResource = mDirEntry->mImageOffset - FirstResourceOffset();
+  return Transition::ToUnbuffered(ICOState::FOUND_RESOURCE,
+                                  ICOState::SKIP_TO_RESOURCE,
+                                  offsetToResource);
+}
+
+LexerTransition<ICOState>
 nsICODecoder::SniffResource(const char* aData)
 {
+  MOZ_ASSERT(mDirEntry);
+
   // 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)
 
   // 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 <= BITMAPINFOSIZE) {
       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);
+      = mLexer.Clone(*mIterator, mDirEntry->mBytesInRes);
 
     // Create a PNG decoder which will do the rest of the work for us.
+    bool metadataDecode = mReturnIterator.isSome();
+    Maybe<IntSize> expectedSize = metadataDecode ? Nothing()
+                                                 : Some(mDirEntry->mSize);
     mContainedDecoder =
       DecoderFactory::CreateDecoderForICOResource(DecoderType::PNG,
                                                   Move(containedIterator),
                                                   WrapNotNull(this),
-                                                  false,
-                                                  Some(GetRealSize()));
+                                                  metadataDecode,
+                                                  expectedSize);
 
     // Read in the rest of the PNG unbuffered.
-    size_t toRead = mDirEntry.mBytesInRes - BITMAPINFOSIZE;
+    size_t toRead = mDirEntry->mBytesInRes - BITMAPINFOSIZE;
     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();
@@ -305,16 +378,18 @@ nsICODecoder::ReadResource()
   }
 
   return Transition::ContinueUnbuffered(ICOState::READ_RESOURCE);
 }
 
 LexerTransition<ICOState>
 nsICODecoder::ReadBIH(const char* aData)
 {
+  MOZ_ASSERT(mDirEntry);
+
   // 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();
@@ -327,52 +402,63 @@ nsICODecoder::ReadBIH(const char* aData)
   // 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);
+    = mLexer.Clone(*mIterator, mDirEntry->mBytesInRes);
 
   // 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.
+  bool metadataDecode = mReturnIterator.isSome();
+  Maybe<IntSize> expectedSize = metadataDecode ? Nothing()
+                                               : Some(mDirEntry->mSize);
   mContainedDecoder =
     DecoderFactory::CreateDecoderForICOResource(DecoderType::BMP,
                                                 Move(containedIterator),
                                                 WrapNotNull(this),
-                                                false,
-                                                Some(GetRealSize()),
+                                                metadataDecode,
+                                                expectedSize,
                                                 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();
   }
 
+  // If this is a metadata decode, FinishResource will any necessary checks.
+  if (mContainedDecoder->IsMetadataDecode()) {
+    return Transition::To(ICOState::FINISHED_RESOURCE, 0);
+  }
+
   // 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;
+  bool hasANDMask = (BITMAPINFOSIZE + bmpDataLength) < mDirEntry->mBytesInRes;
   ICOState afterBMPState = hasANDMask ? ICOState::PREPARE_FOR_MASK
                                       : ICOState::FINISHED_RESOURCE;
 
   // Read in the rest of the BMP unbuffered.
   return Transition::ToUnbuffered(afterBMPState,
                                   ICOState::READ_RESOURCE,
                                   bmpDataLength);
 }
 
 LexerTransition<ICOState>
 nsICODecoder::PrepareForMask()
 {
+  MOZ_ASSERT(mDirEntry);
+  MOZ_ASSERT(mContainedDecoder->GetDecodeDone());
+
   // We have received all of the data required by the BMP decoder so flushing
   // here guarantees the decode has finished.
   if (!FlushContainedDecoder()) {
     return Transition::TerminateFailure();
   }
 
   MOZ_ASSERT(mContainedDecoder->GetDecodeDone());
 
@@ -380,93 +466,93 @@ nsICODecoder::PrepareForMask()
     static_cast<nsBMPDecoder*>(mContainedDecoder.get());
 
   uint16_t numColors = GetNumColors();
   MOZ_ASSERT(numColors != uint16_t(-1));
 
   // Determine the length of the AND mask.
   uint32_t bmpLengthWithHeader =
     BITMAPINFOSIZE + bmpDecoder->GetCompressedImageSize() + 4 * numColors;
-  MOZ_ASSERT(bmpLengthWithHeader < mDirEntry.mBytesInRes);
-  uint32_t maskLength = mDirEntry.mBytesInRes - bmpLengthWithHeader;
+  MOZ_ASSERT(bmpLengthWithHeader < mDirEntry->mBytesInRes);
+  uint32_t maskLength = mDirEntry->mBytesInRes - bmpLengthWithHeader;
 
-  // If the BMP provides its own transparency, we ignore the AND mask. We can
-  // also obviously ignore it if the image has zero width or zero height.
-  if (bmpDecoder->HasTransparency() ||
-      GetRealWidth() == 0 || GetRealHeight() == 0) {
+  // If the BMP provides its own transparency, we ignore the AND mask.
+  if (bmpDecoder->HasTransparency()) {
     return Transition::ToUnbuffered(ICOState::FINISHED_RESOURCE,
                                     ICOState::SKIP_MASK,
                                     maskLength);
   }
 
   // Compute the row size for the mask.
-  mMaskRowSize = ((GetRealWidth() + 31) / 32) * 4; // + 31 to round up
+  mMaskRowSize = ((mDirEntry->mSize.width + 31) / 32) * 4; // + 31 to round up
 
   // If the expected size of the AND mask is larger than its actual size, then
   // we must have a truncated (and therefore corrupt) AND mask.
-  uint32_t expectedLength = mMaskRowSize * GetRealHeight();
+  uint32_t expectedLength = mMaskRowSize * mDirEntry->mSize.height;
   if (maskLength < expectedLength) {
     return Transition::TerminateFailure();
   }
 
   // If we're downscaling, the mask is the wrong size for the surface we've
   // produced, so we need to downscale the mask into a temporary buffer and then
   // combine the mask's alpha values with the color values from the image.
   if (mDownscaler) {
     MOZ_ASSERT(bmpDecoder->GetImageDataLength() ==
                  mDownscaler->TargetSize().width *
                  mDownscaler->TargetSize().height *
                  sizeof(uint32_t));
     mMaskBuffer = MakeUnique<uint8_t[]>(bmpDecoder->GetImageDataLength());
-    nsresult rv = mDownscaler->BeginFrame(GetRealSize(), Nothing(),
+    nsresult rv = mDownscaler->BeginFrame(mDirEntry->mSize, Nothing(),
                                           mMaskBuffer.get(),
                                           /* aHasAlpha = */ true,
                                           /* aFlipVertically = */ true);
     if (NS_FAILED(rv)) {
       return Transition::TerminateFailure();
     }
   }
 
-  mCurrMaskLine = GetRealHeight();
+  mCurrMaskLine = mDirEntry->mSize.height;
   return Transition::To(ICOState::READ_MASK_ROW, mMaskRowSize);
 }
 
 
 LexerTransition<ICOState>
 nsICODecoder::ReadMaskRow(const char* aData)
 {
+  MOZ_ASSERT(mDirEntry);
+
   mCurrMaskLine--;
 
   uint8_t sawTransparency = 0;
 
   // Get the mask row we're reading.
   const uint8_t* mask = reinterpret_cast<const uint8_t*>(aData);
   const uint8_t* maskRowEnd = mask + mMaskRowSize;
 
   // Get the corresponding row of the mask buffer (if we're downscaling) or the
   // decoded image data (if we're not).
   uint32_t* decoded = nullptr;
   if (mDownscaler) {
     // Initialize the row to all white and fully opaque.
-    memset(mDownscaler->RowBuffer(), 0xFF, GetRealWidth() * sizeof(uint32_t));
+    memset(mDownscaler->RowBuffer(), 0xFF, mDirEntry->mSize.width * sizeof(uint32_t));
 
     decoded = reinterpret_cast<uint32_t*>(mDownscaler->RowBuffer());
   } else {
     RefPtr<nsBMPDecoder> bmpDecoder =
       static_cast<nsBMPDecoder*>(mContainedDecoder.get());
     uint32_t* imageData = bmpDecoder->GetImageData();
     if (!imageData) {
       return Transition::TerminateFailure();
     }
 
-    decoded = imageData + mCurrMaskLine * GetRealWidth();
+    decoded = imageData + mCurrMaskLine * mDirEntry->mSize.width;
   }
 
   MOZ_ASSERT(decoded);
-  uint32_t* decodedRowEnd = decoded + GetRealWidth();
+  uint32_t* decodedRowEnd = decoded + mDirEntry->mSize.width;
 
   // Iterate simultaneously through the AND mask and the image data.
   while (mask < maskRowEnd) {
     uint8_t idx = *mask++;
     sawTransparency |= idx;
     for (uint8_t bit = 0x80; bit && decoded < decodedRowEnd; bit >>= 1) {
       // Clear pixel completely for transparency.
       if (idx & bit) {
@@ -516,37 +602,45 @@ nsICODecoder::FinishMask()
   }
 
   return Transition::To(ICOState::FINISHED_RESOURCE, 0);
 }
 
 LexerTransition<ICOState>
 nsICODecoder::FinishResource()
 {
+  MOZ_ASSERT(mDirEntry);
+
   // We have received all of the data required by the PNG/BMP decoder so
   // flushing here guarantees the decode has finished.
   if (!FlushContainedDecoder()) {
     return Transition::TerminateFailure();
   }
 
   MOZ_ASSERT(mContainedDecoder->GetDecodeDone());
 
-  // Make sure the actual size of the resource matches the size in the directory
-  // entry. If not, we consider the image corrupt.
-  if (mContainedDecoder->HasSize() &&
-      mContainedDecoder->Size() != GetRealSize()) {
-    return Transition::TerminateFailure();
+  // If it is a metadata decode, all we were trying to get was the size
+  // information missing from the dir entry.
+  if (mContainedDecoder->IsMetadataDecode()) {
+    if (mContainedDecoder->HasSize()) {
+      mDirEntry->mSize = mContainedDecoder->Size();
+    }
+    return Transition::To(ICOState::ITERATE_UNSIZED_DIR_ENTRY, 0);
   }
 
   // Raymond Chen says that 32bpp only are valid PNG ICOs
   // http://blogs.msdn.com/b/oldnewthing/archive/2010/10/22/10079192.aspx
   if (!mContainedDecoder->IsValidICOResource()) {
     return Transition::TerminateFailure();
   }
 
+  // This size from the resource should match that from the dir entry.
+  MOZ_ASSERT_IF(mContainedDecoder->HasSize(),
+                mContainedDecoder->Size() == mDirEntry->mSize);
+
   // Finalize the frame which we deferred to ensure we could modify the final
   // result (e.g. to apply the BMP mask).
   MOZ_ASSERT(!mContainedDecoder->GetFinalizeFrames());
   if (mCurrentFrame) {
     mCurrentFrame->FinalizeSurface();
   }
 
   return Transition::TerminateSuccess();
@@ -559,16 +653,20 @@ nsICODecoder::DoDecode(SourceBufferItera
 
   return mLexer.Lex(aIterator, aOnResume,
                     [=](ICOState aState, const char* aData, size_t aLength) {
     switch (aState) {
       case ICOState::HEADER:
         return ReadHeader(aData);
       case ICOState::DIR_ENTRY:
         return ReadDirEntry(aData);
+      case ICOState::FINISHED_DIR_ENTRY:
+        return FinishDirEntry();
+      case ICOState::ITERATE_UNSIZED_DIR_ENTRY:
+        return IterateUnsizedDirEntry();
       case ICOState::SKIP_TO_RESOURCE:
         return Transition::ContinueUnbuffered(ICOState::SKIP_TO_RESOURCE);
       case ICOState::FOUND_RESOURCE:
         return Transition::To(ICOState::SNIFF_RESOURCE, BITMAPINFOSIZE);
       case ICOState::SNIFF_RESOURCE:
         return SniffResource(aData);
       case ICOState::READ_RESOURCE:
         return ReadResource();
--- a/image/decoders/nsICODecoder.h
+++ b/image/decoders/nsICODecoder.h
@@ -19,56 +19,34 @@ namespace mozilla {
 namespace image {
 
 class RasterImage;
 
 enum class ICOState
 {
   HEADER,
   DIR_ENTRY,
+  FINISHED_DIR_ENTRY,
+  ITERATE_UNSIZED_DIR_ENTRY,
   SKIP_TO_RESOURCE,
   FOUND_RESOURCE,
   SNIFF_RESOURCE,
   READ_RESOURCE,
   PREPARE_FOR_MASK,
   READ_MASK_ROW,
   FINISH_MASK,
   SKIP_MASK,
   FINISHED_RESOURCE
 };
 
 class nsICODecoder : public Decoder
 {
 public:
   virtual ~nsICODecoder() { }
 
-  /// @return the width of the icon directory entry @aEntry.
-  static uint32_t GetRealWidth(const IconDirEntry& aEntry)
-  {
-    return aEntry.mWidth == 0 ? 256 : aEntry.mWidth;
-  }
-
-  /// @return the width of the selected directory entry (mDirEntry).
-  uint32_t GetRealWidth() const { return GetRealWidth(mDirEntry); }
-
-  /// @return the height of the icon directory entry @aEntry.
-  static uint32_t GetRealHeight(const IconDirEntry& aEntry)
-  {
-    return aEntry.mHeight == 0 ? 256 : aEntry.mHeight;
-  }
-
-  /// @return the height of the selected directory entry (mDirEntry).
-  uint32_t GetRealHeight() const { return GetRealHeight(mDirEntry); }
-
-  /// @return the size of the selected directory entry (mDirEntry).
-  gfx::IntSize GetRealSize() const
-  {
-    return gfx::IntSize(GetRealWidth(), GetRealHeight());
-  }
-
   /// @return The offset from the beginning of the ICO to the first resource.
   size_t FirstResourceOffset() const;
 
   LexerResult DoDecode(SourceBufferIterator& aIterator,
                        IResumable* aOnResume) override;
   nsresult FinishInternal() override;
   nsresult FinishWithErrorInternal() override;
 
@@ -85,36 +63,40 @@ private:
   // Gets decoder state from the contained decoder so it's visible externally.
   nsresult GetFinalStateFromContainedDecoder();
 
   // Obtains the number of colors from the BPP, mBPP must be filled in
   uint16_t GetNumColors();
 
   LexerTransition<ICOState> ReadHeader(const char* aData);
   LexerTransition<ICOState> ReadDirEntry(const char* aData);
+  LexerTransition<ICOState> IterateUnsizedDirEntry();
+  LexerTransition<ICOState> FinishDirEntry();
   LexerTransition<ICOState> SniffResource(const char* aData);
   LexerTransition<ICOState> ReadResource();
   LexerTransition<ICOState> ReadBIH(const char* aData);
   LexerTransition<ICOState> PrepareForMask();
   LexerTransition<ICOState> ReadMaskRow(const char* aData);
   LexerTransition<ICOState> FinishMask();
   LexerTransition<ICOState> FinishResource();
 
+  struct IconDirEntryEx : public IconDirEntry {
+    gfx::IntSize mSize;
+  };
+
   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.
-  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.
-  uint16_t mBPP;      // The BPP of the resource we're decoding.
+  Maybe<SourceBufferIterator> mReturnIterator; // Iterator to save return point.
+  UniquePtr<uint8_t[]> mMaskBuffer; // A temporary buffer for the alpha mask.
+  nsTArray<IconDirEntryEx> mDirEntries; // Valid dir entries with a size.
+  nsTArray<IconDirEntryEx> mUnsizedDirEntries; // Dir entries without a size.
+  IconDirEntryEx* mDirEntry; // The dir entry for the selected 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.
+  uint16_t mBPP;          // The BPP of the resource we're decoding.
   uint32_t mMaskRowSize;  // The size in bytes of each row in the BMP alpha mask.
   uint32_t mCurrMaskLine; // The line of the BMP alpha mask we're processing.
   bool mIsCursor;         // Is this ICO a cursor?
   bool mHasMaskAlpha;     // Did the BMP alpha mask have any transparency?
 };
 
 } // namespace image
 } // namespace mozilla