Bug 1112972 - Part 3: Remove almost all special handling of multipart images in RasterImage. r=tn, a=sledru
authorSeth Fowler <seth@mozilla.com>
Wed, 07 Jan 2015 01:37:20 -0800
changeset 243673 c858f34b8153
parent 243672 438fed84c7c3
child 243674 c4689eff54db
push id4432
push userryanvm@gmail.com
push date2015-02-04 15:12 +0000
treeherdermozilla-beta@7422906b1a32 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, sledru
bugs1112972
milestone36.0
Bug 1112972 - Part 3: Remove almost all special handling of multipart images in RasterImage. r=tn, a=sledru
image/src/Image.h
image/src/ImageFactory.cpp
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/Image.h
+++ b/image/src/Image.h
@@ -42,24 +42,26 @@ public:
    *
    * INIT_FLAG_NONE: Lack of flags
    *
    * INIT_FLAG_DISCARDABLE: The container should be discardable
    *
    * INIT_FLAG_DECODE_ON_DRAW: The container should decode on draw rather than
    * decoding on load.
    *
-   * INIT_FLAG_MULTIPART: The container will be used to display a stream of
-   * images in a multipart channel. If this flag is set, INIT_FLAG_DISCARDABLE
-   * and INIT_FLAG_DECODE_ON_DRAW must not be set.
+   * INIT_FLAG_TRANSIENT: The container is likely to exist for only a short time
+   * before being destroyed. (For example, containers for
+   * multipart/x-mixed-replace image parts fall into this category.) If this
+   * flag is set, INIT_FLAG_DISCARDABLE and INIT_FLAG_DECODE_ON_DRAW must not be
+   * set.
    */
   static const uint32_t INIT_FLAG_NONE           = 0x0;
   static const uint32_t INIT_FLAG_DISCARDABLE    = 0x1;
   static const uint32_t INIT_FLAG_DECODE_ON_DRAW = 0x2;
-  static const uint32_t INIT_FLAG_MULTIPART      = 0x4;
+  static const uint32_t INIT_FLAG_TRANSIENT      = 0x4;
 
   /**
    * Creates a new image container.
    *
    * @param aMimeType The mimetype of the image.
    * @param aFlags Initialization flags of the INIT_FLAG_* variety.
    */
   virtual nsresult Init(const char* aMimeType,
--- a/image/src/ImageFactory.cpp
+++ b/image/src/ImageFactory.cpp
@@ -62,17 +62,17 @@ ComputeImageFlags(ImageURL* uri, bool is
 
   // We have all the information we need.
   uint32_t imageFlags = Image::INIT_FLAG_NONE;
   if (isDiscardable)
     imageFlags |= Image::INIT_FLAG_DISCARDABLE;
   if (doDecodeOnDraw)
     imageFlags |= Image::INIT_FLAG_DECODE_ON_DRAW;
   if (isMultiPart)
-    imageFlags |= Image::INIT_FLAG_MULTIPART;
+    imageFlags |= Image::INIT_FLAG_TRANSIENT;
 
   return imageFlags;
 }
 
 /* static */ bool
 ImageFactory::CanRetargetOnDataAvailable(ImageURL* aURI, bool aIsMultiPart)
 {
   // We can't retarget OnDataAvailable safely in cases where we aren't storing
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -294,17 +294,17 @@ RasterImage::RasterImage(ProgressTracker
 #endif
   mDecodingMonitor("RasterImage Decoding Monitor"),
   mDecoder(nullptr),
   mDecodeStatus(DecodeStatus::INACTIVE),
   mNotifyProgress(NoProgress),
   mNotifying(false),
   mHasSize(false),
   mDecodeOnDraw(false),
-  mMultipart(false),
+  mTransient(false),
   mDiscardable(false),
   mHasSourceData(false),
   mDecoded(false),
   mHasFirstFrame(false),
   mHasBeenDecoded(false),
   mPendingAnimation(false),
   mAnimationFinished(false),
   mWantFullDecode(false),
@@ -373,27 +373,27 @@ RasterImage::Init(const char* aMimeType,
 
   // Not sure an error can happen before init, but be safe
   if (mError)
     return NS_ERROR_FAILURE;
 
   NS_ENSURE_ARG_POINTER(aMimeType);
 
   // We must be non-discardable and non-decode-on-draw for
-  // multipart channels
-  NS_ABORT_IF_FALSE(!(aFlags & INIT_FLAG_MULTIPART) ||
-                    (!(aFlags & INIT_FLAG_DISCARDABLE) &&
-                     !(aFlags & INIT_FLAG_DECODE_ON_DRAW)),
-                    "Can't be discardable or decode-on-draw for multipart");
+  // transient images.
+  MOZ_ASSERT(!(aFlags & INIT_FLAG_TRANSIENT) ||
+               (!(aFlags & INIT_FLAG_DISCARDABLE) &&
+                !(aFlags & INIT_FLAG_DECODE_ON_DRAW)),
+             "Transient images can't be discardable or decode-on-draw");
 
   // Store initialization data
   mSourceDataMimeType.Assign(aMimeType);
   mDiscardable = !!(aFlags & INIT_FLAG_DISCARDABLE);
   mDecodeOnDraw = !!(aFlags & INIT_FLAG_DECODE_ON_DRAW);
-  mMultipart = !!(aFlags & INIT_FLAG_MULTIPART);
+  mTransient = !!(aFlags & INIT_FLAG_TRANSIENT);
 
   // Statistics
   if (mDiscardable) {
     num_discardable_containers++;
     discardable_source_bytes += mSourceData.Length();
   }
 
   // Lock this image's surfaces in the SurfaceCache if we're not discardable.
@@ -562,25 +562,16 @@ RasterImage::LookupFrameInternal(uint32_
 DrawableFrameRef
 RasterImage::LookupFrame(uint32_t aFrameNum,
                          const nsIntSize& aSize,
                          uint32_t aFlags,
                          bool aShouldSyncNotify /* = true */)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (mMultipart &&
-      aFrameNum == GetCurrentFrameIndex() &&
-      mMultipartDecodedFrame) {
-    // In the multipart case we prefer to use mMultipartDecodedFrame, which is
-    // the most recent one we completely decoded, rather than display the real
-    // current frame and risk severe tearing.
-    return mMultipartDecodedFrame->DrawableRef();
-  }
-
   DrawableFrameRef ref = LookupFrameInternal(aFrameNum, aSize, aFlags);
 
   if (!ref && IsOpaque()) {
     // We can use non-premultiplied alpha frames when premultipled alpha is
     // requested, or vice versa, if this image is opaque. Try again with the bit
     // toggled.
     ref = LookupFrameInternal(aFrameNum, aSize,
                               aFlags ^ FLAG_DECODE_NO_PREMULTIPLY_ALPHA);
@@ -1098,17 +1089,17 @@ RasterImage::SetSize(int32_t aWidth, int
     return NS_ERROR_FAILURE;
 
   // Ensure that we have positive values
   // XXX - Why isn't the size unsigned? Should this be changed?
   if ((aWidth < 0) || (aHeight < 0))
     return NS_ERROR_INVALID_ARG;
 
   // if we already have a size, check the new size against the old one
-  if (!mMultipart && mHasSize &&
+  if (mHasSize &&
       ((aWidth != mSize.width) ||
        (aHeight != mSize.height) ||
        (aOrientation != mOrientation))) {
     NS_WARNING("Image changed size on redecode! This should not happen!");
 
     // Make the decoder aware of the error so that it doesn't try to call
     // FinishInternal during ShutdownDecoder.
     if (mDecoder)
@@ -1189,42 +1180,22 @@ RasterImage::DecodingComplete(imgFrame* 
   }
 
   // Flag that we're done decoding.
   // XXX - these should probably be combined when we fix animated image
   // discarding with bug 500402.
   mDecoded = true;
   mHasBeenDecoded = true;
 
-  bool singleFrame = GetNumFrames() == 1;
-
   // If there's only 1 frame, mark it as optimizable. Optimizing animated images
-  // is not supported.
-  //
-  // We don't optimize the frame for multipart images because we reuse
-  // the frame.
-  if (singleFrame && !mMultipart && aFinalFrame) {
+  // is not supported. Optimizing transient images isn't worth it.
+  if (GetNumFrames() == 1 && !mTransient && aFinalFrame) {
     aFinalFrame->SetOptimizable();
   }
 
-  // Double-buffer our frame in the multipart case, since we'll start decoding
-  // into the first frame again immediately and this produces severe tearing.
-  if (mMultipart) {
-    if (singleFrame && aFinalFrame) {
-      // aFinalFrame must be the first frame since we only have one.
-      mMultipartDecodedFrame = aFinalFrame->DrawableRef();
-    } else {
-      // Don't double buffer for animated multipart images. It entails more
-      // complexity and it's not really needed since we already are smart about
-      // not displaying the still-decoding frame of an animated image. We may
-      // have already stored an extra frame, though, so we'll release it here.
-      mMultipartDecodedFrame.reset();
-    }
-  }
-
   if (mAnim) {
     mAnim->SetDoneDecoding(true);
   }
 }
 
 NS_IMETHODIMP
 RasterImage::SetAnimationMode(uint16_t aAnimationMode)
 {
@@ -1387,42 +1358,16 @@ RasterImage::AddSourceData(const char *a
                                      "sourceDataComplete()!");
 
   // Image is already decoded, we shouldn't be getting data, but it could
   // be extra garbage data at the end of a file.
   if (mDecoded) {
     return NS_OK;
   }
 
-  // Starting a new part's frames, let's clean up before we add any
-  // This needs to happen just before we start getting EnsureFrame() call(s),
-  // so that there's no gap for anything to miss us.
-  if (mMultipart && (!mDecoder || mDecoder->BytesDecoded() == 0)) {
-    // Our previous state may have been animated, so let's clean up.
-    if (mAnimating) {
-      StopAnimation();
-    }
-    mAnimationFinished = false;
-    mPendingAnimation = false;
-    if (mAnim) {
-      mAnim = nullptr;
-    }
-
-    // If we had a FrameBlender, clean it up. We'll hold on to the first frame
-    // so we have something to draw until the next frame is decoded.
-    if (mFrameBlender) {
-      nsRefPtr<imgFrame> firstFrame = mFrameBlender->RawGetFrame(0);
-      mMultipartDecodedFrame = firstFrame->DrawableRef();
-      mFrameBlender.reset();
-    }
-
-    // Remove everything stored in the surface cache for this image.
-    SurfaceCache::RemoveImage(ImageKey(this));
-  }
-
   // If we're not storing source data and we've previously gotten the size,
   // write the data directly to the decoder. (If we haven't gotten the size,
   // we'll queue up the data and write it out when we do.)
   if (!StoringSourceData() && mHasSize) {
     rv = WriteToDecoder(aBuffer, aCount, DecodeStrategy::SYNC);
     CONTAINER_ENSURE_SUCCESS(rv);
 
     rv = FinishedSomeDecoding();
@@ -1563,64 +1508,19 @@ RasterImage::OnImageDataAvailable(nsIReq
     "the image must be in error!");
 
   return rv;
 }
 
 nsresult
 RasterImage::OnNewSourceData()
 {
-  MOZ_ASSERT(NS_IsMainThread());
-
-  nsresult rv;
-
-  if (mError)
-    return NS_ERROR_FAILURE;
-
-  // The source data should be complete before calling this
-  NS_ABORT_IF_FALSE(mHasSourceData,
-                    "Calling NewSourceData before SourceDataComplete!");
-  if (!mHasSourceData)
-    return NS_ERROR_ILLEGAL_VALUE;
-
-  // Only supported for multipart channels. It wouldn't be too hard to change this,
-  // but it would involve making sure that things worked for decode-on-draw and
-  // discarding. Presently there's no need for this, so we don't.
-  NS_ABORT_IF_FALSE(mMultipart, "NewSourceData only supported for multipart");
-  if (!mMultipart)
-    return NS_ERROR_ILLEGAL_VALUE;
-
-  // We're multipart, so we shouldn't be storing source data
-  NS_ABORT_IF_FALSE(!StoringSourceData(),
-                    "Shouldn't be storing source data for multipart");
-
-  // We're not storing the source data and we got SourceDataComplete. We should
-  // have shut down the previous decoder
-  NS_ABORT_IF_FALSE(!mDecoder, "Shouldn't have a decoder in NewSourceData");
-
-  // The decoder was shut down and we didn't flag an error, so we should be decoded
-  NS_ABORT_IF_FALSE(mDecoded, "Should be decoded in NewSourceData");
-
-  // Reset some flags
-  mDecoded = false;
-  mHasSourceData = false;
-  mHasSize = false;
-  mHasFirstFrame = false;
-  mWantFullDecode = true;
-  mDecodeStatus = DecodeStatus::INACTIVE;
-
-  if (mAnim) {
-    mAnim->SetDoneDecoding(false);
-  }
-
-  // We always need the size first.
-  rv = InitDecoder(/* aDoSizeDecode = */ true);
-  CONTAINER_ENSURE_SUCCESS(rv);
-
-  return NS_OK;
+  // XXX(seth): This will be removed in a subsequent patch.
+  MOZ_ASSERT_UNREACHABLE();
+  return NS_ERROR_ILLEGAL_VALUE;
 }
 
 /* static */ already_AddRefed<nsIEventTarget>
 RasterImage::GetEventTarget()
 {
   return DecodePool::Singleton()->GetEventTarget();
 }
 
@@ -1697,19 +1597,16 @@ RasterImage::Discard()
 
   // For post-operation logging
   int old_frame_count = GetNumFrames();
 
   // Delete all the decoded frames
   mFrameBlender.reset();
   SurfaceCache::RemoveImage(ImageKey(this));
 
-  // Clear the last decoded multipart frame.
-  mMultipartDecodedFrame.reset();
-
   // Flag that we no longer have decoded frames for this image
   mDecoded = false;
   mHasFirstFrame = false;
 
   // Notify that we discarded
   if (mProgressTracker) {
     mProgressTracker->OnDiscard();
   }
@@ -1738,17 +1635,17 @@ RasterImage::CanDiscard() {
          !mDecoder &&            // Can't discard with an open decoder
          !mAnim;                 // Can never discard animated images
 }
 
 // Helper method to determine if we're storing the source data in a buffer
 // or just writing it directly to the decoder
 bool
 RasterImage::StoringSourceData() const {
-  return !mMultipart;
+  return !mTransient;
 }
 
 
 // Sets up a decoder for this image. It is an error to call this function
 // when decoding is already in process (ie - when mDecoder is non-null).
 nsresult
 RasterImage::InitDecoder(bool aDoSizeDecode)
 {
@@ -2182,19 +2079,19 @@ RasterImage::CanScale(GraphicsFilter aFi
   // we're drawing to something else (e.g. a canvas) we usually have no way of
   // updating what we've drawn, so HQ scaling is useless.
   if (!gfxPrefs::ImageHQDownscalingEnabled() || !mDecoded ||
       !(aFlags & imgIContainer::FLAG_HIGH_QUALITY_SCALING) ||
       aFilter != GraphicsFilter::FILTER_GOOD) {
     return false;
   }
 
-  // We don't use the scaler for animated or multipart images to avoid doing a
+  // We don't use the scaler for animated or transient images to avoid doing a
   // bunch of work on an image that just gets thrown away.
-  if (mAnim || mMultipart) {
+  if (mAnim || mTransient) {
     return false;
   }
 
   // If target size is 1:1 with original, don't scale.
   if (aSize == mSize) {
     return false;
   }
 
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -346,19 +346,16 @@ private: // data
   //
   // Valid flag bits are imgIContainer::FLAG_DECODE_NO_PREMULTIPLY_ALPHA
   // and imgIContainer::FLAG_DECODE_NO_COLORSPACE_CONVERSION.
   uint32_t                   mFrameDecodeFlags;
 
   //! All the frames of the image.
   Maybe<FrameBlender>       mFrameBlender;
 
-  //! The last frame we decoded for multipart images.
-  DrawableFrameRef          mMultipartDecodedFrame;
-
   nsCOMPtr<nsIProperties>   mProperties;
 
   // IMPORTANT: if you use mAnim in a method, call EnsureImageIsDecoded() first to ensure
   // that the frames actually exist (they may have been discarded to save memory, or
   // we maybe decoding on draw).
   UniquePtr<FrameAnimator> mAnim;
 
   // Image locking.
@@ -403,17 +400,17 @@ private: // data
   // Notification state. Used to avoid recursive notifications.
   Progress                   mNotifyProgress;
   nsIntRect                  mNotifyInvalidRect;
   bool                       mNotifying:1;
 
   // Boolean flags (clustered together to conserve space):
   bool                       mHasSize:1;       // Has SetSize() been called?
   bool                       mDecodeOnDraw:1;  // Decoding on draw?
-  bool                       mMultipart:1;     // Multipart?
+  bool                       mTransient:1;     // Is the image short-lived?
   bool                       mDiscardable:1;   // Is container discardable?
   bool                       mHasSourceData:1; // Do we have source data?
 
   // Do we have the frames in decoded form?
   bool                       mDecoded:1;
   bool                       mHasFirstFrame:1;
   bool                       mHasBeenDecoded:1;