From 0e83d78d8f8279d2c43bbad40d078b7c25d2a023 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Wed, 7 Jan 2015 01:37:20 -0800 Subject: [PATCH] Bug 1112972 (Part 3) - Remove almost all special handling of multipart images in RasterImage. r=tn --- image/src/Image.h | 10 +-- image/src/ImageFactory.cpp | 2 +- image/src/RasterImage.cpp | 135 +++++-------------------------------- image/src/RasterImage.h | 5 +- 4 files changed, 24 insertions(+), 128 deletions(-) diff --git a/image/src/Image.h b/image/src/Image.h index 78485c1972a..f38b6c8fdcd 100644 --- a/image/src/Image.h +++ b/image/src/Image.h @@ -47,14 +47,16 @@ public: * 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. diff --git a/image/src/ImageFactory.cpp b/image/src/ImageFactory.cpp index 9d524cf070d..30db2c0b37a 100644 --- a/image/src/ImageFactory.cpp +++ b/image/src/ImageFactory.cpp @@ -71,7 +71,7 @@ ComputeImageFlags(ImageURL* uri, bool isMultiPart) imageFlags |= Image::INIT_FLAG_DECODE_ON_DRAW; } if (isMultiPart) { - imageFlags |= Image::INIT_FLAG_MULTIPART; + imageFlags |= Image::INIT_FLAG_TRANSIENT; } return imageFlags; diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index 75ac949d4bf..355e31204dc 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -299,7 +299,7 @@ RasterImage::RasterImage(ProgressTracker* aProgressTracker, mNotifying(false), mHasSize(false), mDecodeOnDraw(false), - mMultipart(false), + mTransient(false), mDiscardable(false), mHasSourceData(false), mDecoded(false), @@ -378,17 +378,17 @@ RasterImage::Init(const char* aMimeType, 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) { @@ -567,15 +567,6 @@ RasterImage::LookupFrame(uint32_t aFrameNum, { 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()) { @@ -1103,7 +1094,7 @@ RasterImage::SetSize(int32_t aWidth, int32_t aHeight, Orientation aOrientation) 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))) { @@ -1194,32 +1185,12 @@ RasterImage::DecodingComplete(imgFrame* aFinalFrame) 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); } @@ -1392,32 +1363,6 @@ RasterImage::AddSourceData(const char *aBuffer, uint32_t aCount) 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 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.) @@ -1568,54 +1513,9 @@ RasterImage::OnImageDataAvailable(nsIRequest*, 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 @@ -1702,9 +1602,6 @@ RasterImage::Discard() 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; @@ -1743,7 +1640,7 @@ RasterImage::CanDiscard() { // or just writing it directly to the decoder bool RasterImage::StoringSourceData() const { - return !mMultipart; + return !mTransient; } @@ -2187,9 +2084,9 @@ RasterImage::CanScale(GraphicsFilter aFilter, 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; } diff --git a/image/src/RasterImage.h b/image/src/RasterImage.h index a68a5d12fac..7a190971172 100644 --- a/image/src/RasterImage.h +++ b/image/src/RasterImage.h @@ -351,9 +351,6 @@ private: // data //! All the frames of the image. Maybe mFrameBlender; - //! The last frame we decoded for multipart images. - DrawableFrameRef mMultipartDecodedFrame; - nsCOMPtr mProperties; // IMPORTANT: if you use mAnim in a method, call EnsureImageIsDecoded() first to ensure @@ -408,7 +405,7 @@ private: // data // 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?