Backed out changeset fd310390a64a (bug 1315554) for failing GTest's ImageDecoders.LargeICOWithPNGSingleChunk on OS X opt. r=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Sat, 22 Jul 2017 11:05:35 +0200
changeset 419105 8b52b9a7c5f80abc74ab0716b51b9471d28ad6a2
parent 419104 d0bb537fcefe580aaebb059f6879e7bd8b2bfe0e
child 419106 bf96c2e5614817945b2e5d8ff19c4c85a75c2c11
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)
reviewersbackout
bugs1315554
milestone56.0a1
backs outfd310390a64affc366cc85759a7624a5e3f9f239
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 fd310390a64a (bug 1315554) for failing GTest's ImageDecoders.LargeICOWithPNGSingleChunk on OS X opt. r=backout
image/Decoder.cpp
image/Decoder.h
image/DecoderFactory.cpp
image/DecoderFactory.h
image/decoders/nsBMPDecoder.cpp
image/decoders/nsICODecoder.cpp
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -400,24 +400,16 @@ Decoder::PostSize(int32_t aWidth,
 {
   // Validate.
   MOZ_ASSERT(aWidth >= 0, "Width can't be negative!");
   MOZ_ASSERT(aHeight >= 0, "Height can't be negative!");
 
   // Set our intrinsic size.
   mImageMetadata.SetSize(aWidth, aHeight, aOrientation);
 
-  // Verify it is the expected size, if given. Note that this is only used by
-  // the ICO decoder for embedded image types, so only its subdecoders are
-  // required to handle failures in PostSize.
-  if (!IsExpectedSize()) {
-    PostError();
-    return;
-  }
-
   // Set our output size if it's not already set.
   if (!mOutputSize) {
     mOutputSize = Some(IntSize(aWidth, aHeight));
   }
 
   MOZ_ASSERT(mOutputSize->width <= aWidth && mOutputSize->height <= aHeight,
              "Output size will result in upscaling");
 
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -211,33 +211,16 @@ public:
 
   /**
    * @return either the size passed to SetOutputSize() or Nothing(), indicating
    * that SetOutputSize() was not explicitly called.
    */
   Maybe<gfx::IntSize> ExplicitOutputSize() const;
 
   /**
-   * Sets the expected image size of this decoder. Decoding will fail if this
-   * does not match.
-   */
-  void SetExpectedSize(const gfx::IntSize& aSize)
-  {
-    mExpectedSize.emplace(aSize);
-  }
-
-  /**
-   * Is the image size what was expected, if specified?
-   */
-  bool IsExpectedSize() const
-  {
-    return mExpectedSize.isNothing() || *mExpectedSize == Size();
-  }
-
-  /**
    * Set an iterator to the SourceBuffer which will feed data to this decoder.
    * This must always be called before calling Init(). (And only before Init().)
    *
    * XXX(seth): We should eliminate this method and pass a SourceBufferIterator
    * to the various decoder constructors instead.
    */
   void SetIterator(SourceBufferIterator&& aIterator)
   {
@@ -539,17 +522,16 @@ protected:
 
 private:
   RefPtr<RasterImage> mImage;
   Maybe<SourceBufferIterator> mIterator;
   RawAccessFrameRef mCurrentFrame;
   ImageMetadata mImageMetadata;
   gfx::IntRect mInvalidRect; // Tracks an invalidation region in the current frame.
   Maybe<gfx::IntSize> mOutputSize;  // The size of our output surface.
-  Maybe<gfx::IntSize> mExpectedSize; // The expected size of the image.
   Progress mProgress;
 
   uint32_t mFrameCount; // Number of frames, including anything in-progress
   FrameTimeout mLoopLength;  // Length of a single loop of this image.
   gfx::IntRect mFirstFrameRefreshArea;  // The area of the image that needs to
                                         // be invalidated when the animation loops.
 
   // Telemetry data for this decoder.
--- a/image/DecoderFactory.cpp
+++ b/image/DecoderFactory.cpp
@@ -231,17 +231,16 @@ DecoderFactory::CreateMetadataDecoder(De
   RefPtr<IDecodingTask> task = new MetadataDecodingTask(WrapNotNull(decoder));
   return task.forget();
 }
 
 /* static */ already_AddRefed<Decoder>
 DecoderFactory::CreateDecoderForICOResource(DecoderType aType,
                                             NotNull<SourceBuffer*> aSourceBuffer,
                                             NotNull<nsICODecoder*> aICODecoder,
-                                            const Maybe<IntSize>& aExpectedSize,
                                             const Maybe<uint32_t>& aDataOffset
                                               /* = Nothing() */)
 {
   // Create the decoder.
   RefPtr<Decoder> decoder;
   switch (aType) {
     case DecoderType::BMP:
       MOZ_ASSERT(aDataOffset);
@@ -260,19 +259,16 @@ DecoderFactory::CreateDecoderForICOResou
 
   MOZ_ASSERT(decoder);
 
   // Initialize the decoder, copying settings from @aICODecoder.
   MOZ_ASSERT(!aICODecoder->IsMetadataDecode());
   decoder->SetMetadataDecode(aICODecoder->IsMetadataDecode());
   decoder->SetIterator(aSourceBuffer->Iterator());
   decoder->SetOutputSize(aICODecoder->OutputSize());
-  if (aExpectedSize) {
-    decoder->SetExpectedSize(*aExpectedSize);
-  }
   decoder->SetDecoderFlags(aICODecoder->GetDecoderFlags());
   decoder->SetSurfaceFlags(aICODecoder->GetSurfaceFlags());
   decoder->SetFinalizeFrames(false);
 
   if (NS_FAILED(decoder->Init())) {
     return nullptr;
   }
 
--- a/image/DecoderFactory.h
+++ b/image/DecoderFactory.h
@@ -121,26 +121,24 @@ public:
    *              PNG.
    * @param aSourceBuffer The SourceBuffer which the decoder will read its data
    *                      from.
    * @param aICODecoder The ICO decoder which is controlling this resource
    *                    decoder. @aICODecoder's settings will be copied to the
    *                    resource decoder, so the two decoders will have the
    *                    same decoder flags, surface flags, target size, and
    *                    other parameters.
-   * @param aExpectedSize The expected size of the resource from the ICO header.
    * @param aDataOffset If @aType is BMP, specifies the offset at which data
    *                    begins in the BMP resource. Must be Some() if and only
    *                    if @aType is BMP.
    */
   static already_AddRefed<Decoder>
   CreateDecoderForICOResource(DecoderType aType,
                               NotNull<SourceBuffer*> aSourceBuffer,
                               NotNull<nsICODecoder*> aICODecoder,
-                              const Maybe<gfx::IntSize>& aExpectedSize,
                               const Maybe<uint32_t>& aDataOffset = Nothing());
 
   /**
    * Creates and initializes an anonymous decoder (one which isn't associated
    * with an Image object). Only the first frame of the image will be decoded.
    *
    * @param aType Which type of decoder to create - JPEG, PNG, etc.
    * @param aSourceBuffer The SourceBuffer which the decoder will read its data
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -645,19 +645,16 @@ nsBMPDecoder::ReadBitfields(const char* 
     (mH.mCompression == Compression::BITFIELDS &&
      mBitFields.mAlpha.IsPresent());
   if (mMayHaveTransparency) {
     PostHasTransparency();
   }
 
   // Post our size to the superclass.
   PostSize(mH.mWidth, AbsoluteHeight());
-  if (HasError()) {
-    return Transition::TerminateFailure();
-  }
 
   // We've now read all the headers. If we're doing a metadata decode, we're
   // done.
   if (IsMetadataDecode()) {
     return Transition::TerminateSuccess();
   }
 
   // Set up the color table, if present; it'll be filled in by ReadColorTable().
--- a/image/decoders/nsICODecoder.cpp
+++ b/image/decoders/nsICODecoder.cpp
@@ -276,20 +276,16 @@ nsICODecoder::ReadDirEntry(const char* a
       mImageMetadata.SetHotspot(mBiggestResourceHotSpot.width,
                                 mBiggestResourceHotSpot.height);
     }
 
     // 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);
@@ -315,18 +311,17 @@ nsICODecoder::SniffResource(const char* 
                        PNGSIGNATURESIZE);
   if (isPNG) {
     // Create a PNG decoder which will do the rest of the work for us.
     mContainedSourceBuffer = new SourceBuffer();
     mContainedSourceBuffer->ExpectLength(mDirEntry.mBytesInRes);
     mContainedDecoder =
       DecoderFactory::CreateDecoderForICOResource(DecoderType::PNG,
                                                   WrapNotNull(mContainedSourceBuffer),
-                                                  WrapNotNull(this),
-                                                  Some(GetRealSize()));
+                                                  WrapNotNull(this));
 
     if (!WriteToContainedDecoder(aData, PNGSIGNATURESIZE)) {
       return Transition::TerminateFailure();
     }
 
     if (mDirEntry.mBytesInRes <= PNGSIGNATURESIZE) {
       return Transition::TerminateFailure();
     }
@@ -395,17 +390,16 @@ nsICODecoder::ReadBIH(const char* aData)
   // 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.
   mContainedSourceBuffer = new SourceBuffer();
   mContainedSourceBuffer->ExpectLength(mDirEntry.mBytesInRes);
   mContainedDecoder =
     DecoderFactory::CreateDecoderForICOResource(DecoderType::BMP,
                                                 WrapNotNull(mContainedSourceBuffer),
                                                 WrapNotNull(this),
-                                                Some(GetRealSize()),
                                                 Some(dataOffset));
   RefPtr<nsBMPDecoder> bmpDecoder =
     static_cast<nsBMPDecoder*>(mContainedDecoder.get());
 
   // Verify that the BIH width and height values match the ICO directory entry,
   // and fix the BIH height value to compensate for the fact that the underlying
   // BMP decoder doesn't know about AND masks.
   if (!CheckAndFixBitmapSize(reinterpret_cast<int8_t*>(mBIHraw))) {