From 30d7a6f1435c6c21ca638322aba39939584fb546 Mon Sep 17 00:00:00 2001 From: "Adam Dane [:hobophobe]" Date: Tue, 8 May 2012 21:35:32 -0500 Subject: [PATCH] Bug 733553 - Allow multipart image streams to cope with stream changes. r=joe --- image/decoders/nsBMPDecoder.cpp | 3 +- image/decoders/nsGIFDecoder2.cpp | 3 +- image/decoders/nsIconDecoder.cpp | 3 +- image/decoders/nsJPEGDecoder.cpp | 3 +- image/decoders/nsPNGDecoder.cpp | 3 +- image/public/imgIRequest.idl | 7 ++++- image/src/RasterImage.cpp | 50 ++++++++++++++++++++++++-------- image/src/RasterImage.h | 2 +- image/src/imgRequest.cpp | 19 +++++++----- image/src/imgRequest.h | 2 ++ image/src/imgRequestProxy.cpp | 15 ++++++++++ image/src/imgRequestProxy.h | 2 +- layout/generic/nsImageFrame.cpp | 5 +++- 13 files changed, 84 insertions(+), 33 deletions(-) diff --git a/image/decoders/nsBMPDecoder.cpp b/image/decoders/nsBMPDecoder.cpp index 1e369d7e9da6..2b3b84fbeb38 100644 --- a/image/decoders/nsBMPDecoder.cpp +++ b/image/decoders/nsBMPDecoder.cpp @@ -278,8 +278,7 @@ nsBMPDecoder::WriteInternal(const char* aBuffer, PRUint32 aCount) // Post our size to the superclass PostSize(mBIH.width, real_height); if (HasError()) { - // Setting the size lead to an error; this can happen when for example - // a multipart channel sends an image of a different size. + // Setting the size led to an error. return; } diff --git a/image/decoders/nsGIFDecoder2.cpp b/image/decoders/nsGIFDecoder2.cpp index c321aa70dc9d..54dcb05a3a75 100644 --- a/image/decoders/nsGIFDecoder2.cpp +++ b/image/decoders/nsGIFDecoder2.cpp @@ -920,8 +920,7 @@ nsGIFDecoder2::WriteInternal(const char *aBuffer, PRUint32 aCount) // Create the image container with the right size. BeginGIF(); if (HasError()) { - // Setting the size lead to an error; this can happen when for example - // a multipart channel sends an image of a different size. + // Setting the size led to an error. mGIFStruct.state = gif_error; return; } diff --git a/image/decoders/nsIconDecoder.cpp b/image/decoders/nsIconDecoder.cpp index 8a13ef839253..851a17500628 100644 --- a/image/decoders/nsIconDecoder.cpp +++ b/image/decoders/nsIconDecoder.cpp @@ -101,8 +101,7 @@ nsIconDecoder::WriteInternal(const char *aBuffer, PRUint32 aCount) // Post our size to the superclass PostSize(mWidth, mHeight); if (HasError()) { - // Setting the size lead to an error; this can happen when for example - // a multipart channel sends an image of a different size. + // Setting the size led to an error. mState = iconStateFinished; return; } diff --git a/image/decoders/nsJPEGDecoder.cpp b/image/decoders/nsJPEGDecoder.cpp index 30590f0eecf0..95464fd0eb8d 100644 --- a/image/decoders/nsJPEGDecoder.cpp +++ b/image/decoders/nsJPEGDecoder.cpp @@ -263,8 +263,7 @@ nsJPEGDecoder::WriteInternal(const char *aBuffer, PRUint32 aCount) // Post our size to the superclass PostSize(mInfo.image_width, mInfo.image_height); if (HasError()) { - // Setting the size lead to an error; this can happen when for example - // a multipart channel sends an image of a different size. + // Setting the size led to an error. mState = JPEG_ERROR; return; } diff --git a/image/decoders/nsPNGDecoder.cpp b/image/decoders/nsPNGDecoder.cpp index ccfe038dbcbc..c60eb8119c06 100644 --- a/image/decoders/nsPNGDecoder.cpp +++ b/image/decoders/nsPNGDecoder.cpp @@ -519,8 +519,7 @@ nsPNGDecoder::info_callback(png_structp png_ptr, png_infop info_ptr) // Post our size to the superclass decoder->PostSize(width, height); if (decoder->HasError()) { - // Setting the size lead to an error; this can happen when for example - // a multipart channel sends an image of a different size. + // Setting the size led to an error. longjmp(png_jmpbuf(decoder->mPNG), 1); } diff --git a/image/public/imgIRequest.idl b/image/public/imgIRequest.idl index 080e2f9661ba..45098c86ba12 100644 --- a/image/public/imgIRequest.idl +++ b/image/public/imgIRequest.idl @@ -52,7 +52,7 @@ interface nsIPrincipal; * @version 0.1 * @see imagelib2 */ -[scriptable, uuid(d35a9adb-8328-4b64-b06f-72a165acd080)] +[scriptable, uuid(a5a785a8-9881-11e1-aaff-001fbc092072)] interface imgIRequest : nsIRequest { /** @@ -132,6 +132,11 @@ interface imgIRequest : nsIRequest */ readonly attribute nsIPrincipal imagePrincipal; + /** + * Whether the request is multipart (ie, multipart/x-mixed-replace) + */ + readonly attribute bool multipart; + /** * CORS modes images can be loaded with. * diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index 14647fa89bb2..194493bcc6df 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -343,7 +343,7 @@ RasterImage::AdvanceFrame(TimeStamp aTime, nsIntRect* aDirtyRect) // If we don't have a decoder, we know we've got everything we're going to // get. If we do, we only display fully-downloaded frames; everything else // gets delayed. - bool haveFullNextFrame = !mDecoder || + bool haveFullNextFrame = (mMultipart && mBytesDecoded == 0) || !mDecoder || nextFrameIndex < mDecoder->GetCompleteFrameCount(); // If we're done decoding the next frame, go ahead and display it now and @@ -1136,14 +1136,9 @@ RasterImage::SetSize(PRInt32 aWidth, PRInt32 aHeight) return NS_ERROR_INVALID_ARG; // if we already have a size, check the new size against the old one - if (mHasSize && + if (!mMultipart && mHasSize && ((aWidth != mSize.width) || (aHeight != mSize.height))) { - - // Alter the warning depending on whether the channel is multipart - if (!mMultipart) - NS_WARNING("Image changed size on redecode! This should not happen!"); - else - NS_WARNING("Multipart channel sent an image of a different size"); + 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. @@ -1215,10 +1210,14 @@ RasterImage::EnsureFrame(PRUint32 aFrameNum, PRInt32 aX, PRInt32 aY, } } + // Not reusable, so replace the frame directly. DeleteImgFrame(aFrameNum); - return InternalAddFrame(aFrameNum, aX, aY, aWidth, aHeight, aFormat, - aPaletteDepth, imageData, imageLength, - paletteData, paletteLength); + mFrames.RemoveElementAt(aFrameNum); + nsAutoPtr newFrame(new imgFrame()); + nsresult rv = newFrame->Init(aX, aY, aWidth, aHeight, aFormat, aPaletteDepth); + NS_ENSURE_SUCCESS(rv, rv); + return InternalAddFrameHelper(aFrameNum, newFrame.forget(), imageData, + imageLength, paletteData, paletteLength); } nsresult @@ -1491,6 +1490,31 @@ RasterImage::AddSourceData(const char *aBuffer, PRUint32 aCount) // This call should come straight from necko - no reentrancy allowed NS_ABORT_IF_FALSE(!mInDecoder, "Re-entrant call to AddSourceData!"); + // 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 (mBytesDecoded == 0) { + // Our previous state may have been animated, so let's clean up + bool wasAnimating = mAnimating; + if (mAnimating) { + StopAnimation(); + mAnimating = false; + } + mAnimationFinished = false; + if (mAnim) { + delete mAnim; + mAnim = nsnull; + } + // If there's only one frame, this could cause flickering + int old_frame_count = mFrames.Length(); + if (old_frame_count > 1) { + for (int i = 0; i < old_frame_count; ++i) { + DeleteImgFrame(i); + } + mFrames.Clear(); + } + } + // If we're not storing source data, write it directly to the decoder if (!StoringSourceData()) { rv = WriteToDecoder(aBuffer, aCount); @@ -1620,7 +1644,7 @@ RasterImage::SourceDataComplete() } nsresult -RasterImage::NewSourceData() +RasterImage::NewSourceData(const char* aMimeType) { nsresult rv; @@ -1655,6 +1679,8 @@ RasterImage::NewSourceData() mDecoded = false; mHasSourceData = false; + mSourceDataMimeType.Assign(aMimeType); + // We're decode-on-load here. Open up a new decoder just like what happens when // we call Init() for decode-on-load images. rv = InitDecoder(/* aDoSizeDecode = */ false); diff --git a/image/src/RasterImage.h b/image/src/RasterImage.h index 953412a2da03..21ef9ee0f73d 100644 --- a/image/src/RasterImage.h +++ b/image/src/RasterImage.h @@ -304,7 +304,7 @@ class RasterImage : public Image nsresult SourceDataComplete(); /* Called for multipart images when there's a new source image to add. */ - nsresult NewSourceData(); + nsresult NewSourceData(const char *aMimeType); /** * A hint of the number of bytes of source data that the image contains. If diff --git a/image/src/imgRequest.cpp b/image/src/imgRequest.cpp index 85ec2fa0d919..a241504516b3 100644 --- a/image/src/imgRequest.cpp +++ b/image/src/imgRequest.cpp @@ -768,14 +768,19 @@ NS_IMETHODIMP imgRequest::OnStartRequest(nsIRequest *aRequest, nsISupports *ctxt // If we're multipart, and our image is initialized, fix things up for another round if (mIsMultiPartChannel && mImage) { - if (mImage->GetType() == imgIContainer::TYPE_RASTER) { + // Update the content type for this new part + nsCOMPtr partChan(do_QueryInterface(aRequest)); + partChan->GetContentType(mContentType); + if (mContentType.EqualsLiteral(SVG_MIMETYPE) || + mImage->GetType() == imgIContainer::TYPE_VECTOR) { + // mImage won't be reusable due to format change or a new SVG part + // Reset the tracker and forget that we have data for OnDataAvailable to + // treat its next call as a fresh image. + mStatusTracker = new imgStatusTracker(nsnull); + mGotData = false; + } else if (mImage->GetType() == imgIContainer::TYPE_RASTER) { // Inform the RasterImage that we have new source data - static_cast(mImage.get())->NewSourceData(); - } else { // imageType == imgIContainer::TYPE_VECTOR - nsCOMPtr imageAsStream = do_QueryInterface(mImage); - NS_ABORT_IF_FALSE(imageAsStream, - "SVG-typed Image failed QI to nsIStreamListener"); - imageAsStream->OnStartRequest(aRequest, ctxt); + static_cast(mImage.get())->NewSourceData(mContentType.get()); } } diff --git a/image/src/imgRequest.h b/image/src/imgRequest.h index 7b200dff1b9c..503fe5b52e8d 100644 --- a/image/src/imgRequest.h +++ b/image/src/imgRequest.h @@ -131,6 +131,8 @@ class imgRequest : public imgIDecoderObserver, // wins. static void SetCacheValidation(imgCacheEntry* aEntry, nsIRequest* aRequest); + bool GetMultipart() const { return mIsMultiPartChannel; } + // The CORS mode for which we loaded this image. PRInt32 GetCORSMode() const { return mCORSMode; } diff --git a/image/src/imgRequestProxy.cpp b/image/src/imgRequestProxy.cpp index 32edcc9360a4..29744e3a6b47 100644 --- a/image/src/imgRequestProxy.cpp +++ b/image/src/imgRequestProxy.cpp @@ -558,6 +558,17 @@ NS_IMETHODIMP imgRequestProxy::GetImagePrincipal(nsIPrincipal **aPrincipal) return NS_OK; } +/* readonly attribute bool multipart; */ +NS_IMETHODIMP imgRequestProxy::GetMultipart(bool *aMultipart) +{ + if (!mOwner) + return NS_ERROR_FAILURE; + + *aMultipart = mOwner->GetMultipart(); + + return NS_OK; +} + /* readonly attribute PRInt32 CORSMode; */ NS_IMETHODIMP imgRequestProxy::GetCORSMode(PRInt32* aCorsMode) { @@ -695,6 +706,10 @@ void imgRequestProxy::OnStopContainer(imgIContainer *image) nsCOMPtr kungFuDeathGrip(mListener); mListener->OnStopContainer(this, image); } + + // Multipart needs reset for next OnStartContainer + if (mOwner && mOwner->GetMultipart()) + mSentStartContainer = false; } void imgRequestProxy::OnStopDecode(nsresult status, const PRUnichar *statusArg) diff --git a/image/src/imgRequestProxy.h b/image/src/imgRequestProxy.h index ef42a0631d64..f7ec5283d420 100644 --- a/image/src/imgRequestProxy.h +++ b/image/src/imgRequestProxy.h @@ -253,7 +253,7 @@ class imgRequestProxy : public imgIRequest, bool mDeferNotifications; // We only want to send OnStartContainer once for each proxy, but we might - // get multiple OnStartContainer calls (e.g. from multipart/x-mixed-replace). + // get multiple OnStartContainer calls. bool mSentStartContainer; }; diff --git a/layout/generic/nsImageFrame.cpp b/layout/generic/nsImageFrame.cpp index 81e20adb2bf7..fa262cd0db2e 100644 --- a/layout/generic/nsImageFrame.cpp +++ b/layout/generic/nsImageFrame.cpp @@ -652,7 +652,10 @@ nsImageFrame::OnStopDecode(imgIRequest *aRequest, return NS_ERROR_FAILURE; } - if (loadType == nsIImageLoadingContent::PENDING_REQUEST) { + bool multipart = false; + aRequest->GetMultipart(&multipart); + + if (loadType == nsIImageLoadingContent::PENDING_REQUEST || multipart) { NotifyNewCurrentRequest(aRequest, aStatus); }