Bug 1195878 - If we detect animation during a full decode, drop the results of the full decode on the floor. r=tn, a=lizzard
authorSeth Fowler <mark.seth.fowler@gmail.com>
Wed, 23 Sep 2015 16:53:40 -0700
changeset 297882 1e921bf3f7049279944f832b99a42b4170fce08a
parent 297881 d0a3f5e5adc0dd84e1ddce9451b3ce36f997e897
child 297883 6f724b1e2353d2924b7598a511cea60ebe35e0d6
push id962
push userjlund@mozilla.com
push dateFri, 04 Dec 2015 23:28:54 +0000
treeherdermozilla-release@23a2d286e80f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, lizzard
bugs1195878
milestone43.0a2
Bug 1195878 - If we detect animation during a full decode, drop the results of the full decode on the floor. r=tn, a=lizzard
image/RasterImage.cpp
image/RasterImage.h
image/SurfaceFlags.h
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -937,43 +937,43 @@ RasterImage::OnAddedFrame(uint32_t aNewF
       }
     }
     if (aNewFrameCount > 1) {
       mAnim->UnionFirstFrameRefreshArea(aNewRefreshArea);
     }
   }
 }
 
-void
+bool
 RasterImage::SetMetadata(const ImageMetadata& aMetadata,
                          bool aFromMetadataDecode)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mError) {
-    return;
+    return true;
   }
 
   if (aMetadata.HasSize()) {
     IntSize size = aMetadata.GetSize();
     if (size.width < 0 || size.height < 0) {
       NS_WARNING("Image has negative intrinsic size");
       DoError();
-      return;
+      return true;
     }
 
     MOZ_ASSERT(aMetadata.HasOrientation());
     Orientation orientation = aMetadata.GetOrientation();
 
     // If we already have a size, check the new size against the old one.
     if (mHasSize && (size != mSize || orientation != mOrientation)) {
       NS_WARNING("Image changed size or orientation on redecode! "
                  "This should not happen!");
       DoError();
-      return;
+      return true;
     }
 
     // Set the size and flag that we have it.
     mSize = size;
     mOrientation = orientation;
     mHasSize = true;
   }
 
@@ -985,17 +985,17 @@ RasterImage::SetMetadata(const ImageMeta
     // Lock the image and throw away the key.
     LockImage();
 
     if (!aFromMetadataDecode) {
       // The metadata decode reported that this image isn't animated, but we
       // discovered that it actually was during the full decode. This is a
       // rare failure that only occurs for corrupt images. To recover, we need
       // to discard all existing surfaces and redecode.
-      RecoverFromInvalidFrames(mSize, DECODE_FLAGS_DEFAULT);
+      return false;
     }
   }
 
   if (mAnim) {
     mAnim->SetLoopCount(aMetadata.GetLoopCount());
     mAnim->SetFirstFrameTimeout(aMetadata.GetFirstFrameTimeout());
   }
 
@@ -1007,16 +1007,18 @@ RasterImage::SetMetadata(const ImageMeta
     nsCOMPtr<nsISupportsPRUint32> intwrapy =
       do_CreateInstance(NS_SUPPORTS_PRUINT32_CONTRACTID);
     intwrapx->SetData(hotspot.x);
     intwrapy->SetData(hotspot.y);
 
     Set("hotspotX", intwrapx);
     Set("hotspotY", intwrapy);
   }
+
+  return true;
 }
 
 NS_IMETHODIMP
 RasterImage::SetAnimationMode(uint16_t aAnimationMode)
 {
   if (mAnim) {
     mAnim->SetAnimationMode(aAnimationMode);
   }
@@ -1986,17 +1988,28 @@ RasterImage::FinalizeDecoder(Decoder* aD
   bool done = aDecoder->GetDecodeDone();
 
   // If the decoder detected an error, log it to the error console.
   if (aDecoder->ShouldReportError() && !aDecoder->WasAborted()) {
     ReportDecoderError(aDecoder);
   }
 
   // Record all the metadata the decoder gathered about this image.
-  SetMetadata(aDecoder->GetImageMetadata(), wasMetadata);
+  bool metadataOK = SetMetadata(aDecoder->GetImageMetadata(), wasMetadata);
+  if (!metadataOK) {
+    // This indicates a serious error that requires us to discard all existing
+    // surfaces and redecode to recover. We'll drop the results from this
+    // decoder on the floor, since they aren't valid.
+    aDecoder->TakeProgress();
+    aDecoder->TakeInvalidRect();
+    RecoverFromInvalidFrames(mSize,
+                             FromSurfaceFlags(aDecoder->GetSurfaceFlags()));
+    return;
+  }
+
   MOZ_ASSERT(mError || mHasSize || !aDecoder->HasSize(),
              "SetMetadata should've gotten a size");
 
   if (!wasMetadata && aDecoder->GetDecodeDone() && !aDecoder->WasAborted()) {
     // Flag that we've been decoded before.
     mHasBeenDecoded = true;
     if (mAnim) {
       mAnim->SetDoneDecoding(true);
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -321,18 +321,21 @@ private:
    * information about the image gathered during decoding.
    *
    * This function may be called multiple times, but will throw an error if
    * subsequent calls do not match the first.
    *
    * @param aMetadata The metadata to set on this image.
    * @param aFromMetadataDecode True if this metadata came from a metadata
    *                            decode; false if it came from a full decode.
+   * @return |true| unless a catastrophic failure was discovered. If |false| is
+   * returned, it indicates that the image is corrupt in a way that requires all
+   * surfaces to be discarded to recover.
    */
-  void SetMetadata(const ImageMetadata& aMetadata, bool aFromMetadataDecode);
+  bool SetMetadata(const ImageMetadata& aMetadata, bool aFromMetadataDecode);
 
   /**
    * In catastrophic circumstances like a GPU driver crash, the contents of our
    * frames may become invalid.  If the information we gathered during the
    * metadata decode proves to be wrong due to image corruption, the frames we
    * have may violate this class's invariants. Either way, we need to
    * immediately discard the invalid frames and redecode so that callers don't
    * perceive that we've entered an invalid state. 
--- a/image/SurfaceFlags.h
+++ b/image/SurfaceFlags.h
@@ -45,12 +45,29 @@ ToSurfaceFlags(uint32_t aFlags)
     flags |= SurfaceFlags::NO_PREMULTIPLY_ALPHA;
   }
   if (aFlags & imgIContainer::FLAG_DECODE_NO_COLORSPACE_CONVERSION) {
     flags |= SurfaceFlags::NO_COLORSPACE_CONVERSION;
   }
   return flags;
 }
 
+/**
+ * Given a set of SurfaceFlags, returns a set of imgIContainer FLAG_* flags with
+ * the corresponding flags set.
+ */
+inline uint32_t
+FromSurfaceFlags(SurfaceFlags aFlags)
+{
+  uint32_t flags = imgIContainer::DECODE_FLAGS_DEFAULT;
+  if (aFlags & SurfaceFlags::NO_PREMULTIPLY_ALPHA) {
+    flags |= imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA;
+  }
+  if (aFlags & SurfaceFlags::NO_COLORSPACE_CONVERSION) {
+    flags |= imgIContainer::FLAG_DECODE_NO_COLORSPACE_CONVERSION;
+  }
+  return flags;
+}
+
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_SurfaceFlags_h