Bug 733553 - Allow multipart image streams to cope with stream changes. r=joe
authorAdam Dane [:hobophobe] <unusualtears@gmail.com>
Sat, 19 May 2012 14:32:37 -0500
changeset 94449 ed488f577c84d1213600d2459ad913de0952adb1
parent 94448 32cfab3a6aa5f8d46189e62dc3e8f319fed2669f
child 94450 8f1c93b5b54933a4f88c9e252bc18c3462f36882
push id9619
push userryanvm@gmail.com
push dateSun, 20 May 2012 00:32:50 +0000
treeherdermozilla-inbound@8f1c93b5b549 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjoe
bugs733553
milestone15.0a1
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
Bug 733553 - Allow multipart image streams to cope with stream changes. r=joe
image/decoders/nsBMPDecoder.cpp
image/decoders/nsGIFDecoder2.cpp
image/decoders/nsIconDecoder.cpp
image/decoders/nsJPEGDecoder.cpp
image/decoders/nsPNGDecoder.cpp
image/public/imgIRequest.idl
image/src/RasterImage.cpp
image/src/RasterImage.h
image/src/imgRequest.cpp
image/src/imgRequest.h
image/src/imgRequestProxy.cpp
image/src/imgRequestProxy.h
layout/generic/nsImageFrame.cpp
--- a/image/decoders/nsBMPDecoder.cpp
+++ b/image/decoders/nsBMPDecoder.cpp
@@ -273,18 +273,17 @@ nsBMPDecoder::WriteInternal(const char* 
             return;
         }
 
         PRUint32 real_height = (mBIH.height > 0) ? mBIH.height : -mBIH.height;
 
         // 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;
         }
 
         // We have the size. If we're doing a size decode, we got what
         // we came for.
         if (IsSizeDecode())
             return;
 
--- a/image/decoders/nsGIFDecoder2.cpp
+++ b/image/decoders/nsGIFDecoder2.cpp
@@ -915,18 +915,17 @@ nsGIFDecoder2::WriteInternal(const char 
           mGIFStruct.screen_height = mGIFStruct.height;
           mGIFStruct.screen_width = mGIFStruct.width;
           mGIFStruct.x_offset = 0;
           mGIFStruct.y_offset = 0;
         }    
         // 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;
         }
 
         // If we were doing a size decode, we're done
         if (IsSizeDecode())
           return;
       }
--- a/image/decoders/nsIconDecoder.cpp
+++ b/image/decoders/nsIconDecoder.cpp
@@ -96,18 +96,17 @@ nsIconDecoder::WriteInternal(const char 
       case iconStateHaveHeight:
 
         // Grab the Height
         mHeight = (PRUint8)*aBuffer;
 
         // 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;
         }
 
         // If We're doing a size decode, we're done
         if (IsSizeDecode()) {
           mState = iconStateFinished;
           break;
--- a/image/decoders/nsJPEGDecoder.cpp
+++ b/image/decoders/nsJPEGDecoder.cpp
@@ -258,18 +258,17 @@ nsJPEGDecoder::WriteInternal(const char 
       PR_LOG(gJPEGDecoderAccountingLog, PR_LOG_DEBUG,
              ("} (JPEG_SUSPENDED)"));
       return; /* I/O suspension */
     }
 
     // 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;
     }
 
     /* If we're doing a size decode, we're done. */
     if (IsSizeDecode())
       return;
 
--- a/image/decoders/nsPNGDecoder.cpp
+++ b/image/decoders/nsPNGDecoder.cpp
@@ -514,18 +514,17 @@ nsPNGDecoder::info_callback(png_structp 
 
   /* Are we too big? */
   if (width > MOZ_PNG_MAX_DIMENSION || height > MOZ_PNG_MAX_DIMENSION)
     longjmp(png_jmpbuf(decoder->mPNG), 1);
 
   // 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);
   }
 
   if (color_type == PNG_COLOR_TYPE_PALETTE)
     png_set_expand(png_ptr);
 
   if (color_type == PNG_COLOR_TYPE_GRAY && bit_depth < 8)
     png_set_expand(png_ptr);
--- a/image/public/imgIRequest.idl
+++ b/image/public/imgIRequest.idl
@@ -47,17 +47,17 @@ interface nsIPrincipal;
 
 /**
  * imgIRequest interface
  *
  * @author Stuart Parmenter <stuart@mozilla.com>
  * @version 0.1
  * @see imagelib2
  */
-[scriptable, uuid(d35a9adb-8328-4b64-b06f-72a165acd080)]
+[scriptable, uuid(a5a785a8-9881-11e1-aaff-001fbc092072)]
 interface imgIRequest : nsIRequest
 {
   /**
    * the image container...
    * @return the image object associated with the request.
    * @attention NEED DOCS
    */
   readonly attribute imgIContainer image;
@@ -128,16 +128,21 @@ interface imgIRequest : nsIRequest
   imgIRequest clone(in imgIDecoderObserver aObserver);
 
   /**
    * The principal gotten from the channel the image was loaded from.
    */
   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.
    *
    * By default, all images are loaded with CORS_NONE and cannot be used
    * cross-origin in context like WebGL.
    *
    * If an HTML img element has the crossorigin attribute set, the imgIRequest
    * will be validated for cross-origin usage with CORS, and, if successful,
    * will have its CORS mode set to the relevant type.
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -338,17 +338,17 @@ RasterImage::AdvanceFrame(TimeStamp aTim
   // just checking for mFrames.Length() because decoders append their frames
   // before they're filled in.
   NS_ABORT_IF_FALSE(mDecoder || nextFrameIndex <= mFrames.Length(),
                     "How did we get 2 indices too far by incrementing?");
 
   // 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
   // reinit with the next frame's delay time.
   if (haveFullNextFrame) {
     if (mFrames.Length() == nextFrameIndex) {
       // End of Animation, unless we are looping forever
 
@@ -1131,24 +1131,19 @@ RasterImage::SetSize(PRInt32 aWidth, PRI
     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 (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.
     if (mDecoder)
       mDecoder->PostResizeError();
 
     DoError();
     return NS_ERROR_UNEXPECTED;
@@ -1210,20 +1205,24 @@ RasterImage::EnsureFrame(PRUint32 aFrame
     if (*imageData && paletteData && *paletteData) {
       return NS_OK;
     }
     if (*imageData && !paletteData) {
       return NS_OK;
     }
   }
 
+  // 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<imgFrame> 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
 RasterImage::EnsureFrame(PRUint32 aFramenum, PRInt32 aX, PRInt32 aY,
                          PRInt32 aWidth, PRInt32 aHeight,
                          gfxASurface::gfxImageFormat aFormat,
                          PRUint8** imageData, PRUint32* imageLength)
 {
@@ -1486,16 +1485,41 @@ RasterImage::AddSourceData(const char *a
 
   // We should not call this if we're already finished adding source data
   NS_ABORT_IF_FALSE(!mHasSourceData, "Calling AddSourceData() after calling "
                                      "sourceDataComplete()!");
 
   // 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);
     CONTAINER_ENSURE_SUCCESS(rv);
 
     // We're not storing source data, so this data is probably coming straight
     // from the network. In this case, we want to display data as soon as we
     // get it, so we want to flush invalidations after every write.
@@ -1615,17 +1639,17 @@ RasterImage::SourceDataComplete()
   if (CanDiscard()) {
     nsresult rv = DiscardTracker::Reset(&mDiscardTrackerNode);
     CONTAINER_ENSURE_SUCCESS(rv);
   }
   return NS_OK;
 }
 
 nsresult
-RasterImage::NewSourceData()
+RasterImage::NewSourceData(const char* aMimeType)
 {
   nsresult rv;
 
   if (mError)
     return NS_ERROR_FAILURE;
 
   // The source data should be complete before calling this
   NS_ABORT_IF_FALSE(mHasSourceData,
@@ -1650,16 +1674,18 @@ RasterImage::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;
 
+  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);
   CONTAINER_ENSURE_SUCCESS(rv);
 
   return NS_OK;
 }
 
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -299,17 +299,17 @@ public:
    * value. Should this just return void?
    */
   nsresult AddSourceData(const char *aBuffer, PRUint32 aCount);
 
   /* Called after the all the source data has been added with addSourceData. */
   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
    * called early on, this can help reduce copying and reallocations by
    * appropriately preallocating the source data buffer.
    *
    * We take this approach rather than having the source data management code do
    * something more complicated (like chunklisting) because HTTP is by far the
--- a/image/src/imgRequest.cpp
+++ b/image/src/imgRequest.cpp
@@ -177,17 +177,17 @@ nsresult imgRequest::Init(nsIURI *aURI,
   SetLoadId(aLoadId);
 
   return NS_OK;
 }
 
 imgStatusTracker&
 imgRequest::GetStatusTracker()
 {
-  if (mImage) {
+  if (mImage && mGotData) {
     NS_ABORT_IF_FALSE(!mStatusTracker,
                       "Should have given mStatusTracker to mImage");
     return mImage->GetStatusTracker();
   } else {
     NS_ABORT_IF_FALSE(mStatusTracker,
                       "Should have mStatusTracker until we create mImage");
     return *mStatusTracker;
   }
@@ -763,24 +763,29 @@ NS_IMETHODIMP imgRequest::OnStartRequest
       mIsMultiPartChannel = true;
 
   // If we're not multipart, we shouldn't have an image yet
   NS_ABORT_IF_FALSE(mIsMultiPartChannel || !mImage,
                     "Already have an image for non-multipart request");
 
   // 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<nsIChannel> 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<RasterImage*>(mImage.get())->NewSourceData();
-    } else {  // imageType == imgIContainer::TYPE_VECTOR
-      nsCOMPtr<nsIStreamListener> imageAsStream = do_QueryInterface(mImage);
-      NS_ABORT_IF_FALSE(imageAsStream,
-                        "SVG-typed Image failed QI to nsIStreamListener");
-      imageAsStream->OnStartRequest(aRequest, ctxt);
+      static_cast<RasterImage*>(mImage.get())->NewSourceData(mContentType.get());
     }
   }
 
   /*
    * If mRequest is null here, then we need to set it so that we'll be able to
    * cancel it if our Cancel() method is called.  Note that this can only
    * happen for multipart channels.  We could simply not null out mRequest for
    * non-last parts, if GetIsLastPart() were reliable, but it's not.  See
--- a/image/src/imgRequest.h
+++ b/image/src/imgRequest.h
@@ -126,16 +126,18 @@ public:
   }
 
   // Set the cache validation information (expiry time, whether we must
   // validate, etc) on the cache entry based on the request information.
   // If this function is called multiple times, the information set earliest
   // 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; }
 
   // The principal for the document that loaded this image. Used when trying to
   // validate a CORS image load.
   already_AddRefed<nsIPrincipal> GetLoadingPrincipal() const
   {
     nsCOMPtr<nsIPrincipal> principal = mLoadingPrincipal;
--- a/image/src/imgRequestProxy.cpp
+++ b/image/src/imgRequestProxy.cpp
@@ -553,16 +553,27 @@ NS_IMETHODIMP imgRequestProxy::GetImageP
   if (!mPrincipal)
     return NS_ERROR_FAILURE;
 
   NS_ADDREF(*aPrincipal = mPrincipal);
 
   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)
 {
   if (!mOwner)
     return NS_ERROR_FAILURE;
 
   *aCorsMode = mOwner->GetCORSMode();
 
@@ -690,16 +701,20 @@ void imgRequestProxy::OnStopContainer(im
 {
   LOG_FUNC(gImgLog, "imgRequestProxy::OnStopContainer");
 
   if (mListener && !mCanceled) {
     // Hold a ref to the listener while we call it, just in case.
     nsCOMPtr<imgIDecoderObserver> 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)
 {
   LOG_FUNC(gImgLog, "imgRequestProxy::OnStopDecode");
 
   if (mListener && !mCanceled) {
     // Hold a ref to the listener while we call it, just in case.
@@ -875,17 +890,18 @@ void imgRequestProxy::SyncNotifyListener
 
   GetStatusTracker().SyncNotify(this);
 }
 
 void
 imgRequestProxy::SetImage(Image* aImage)
 {
   NS_ABORT_IF_FALSE(aImage,  "Setting null image");
-  NS_ABORT_IF_FALSE(!mImage, "Setting image when we already have one");
+  NS_ABORT_IF_FALSE(!mImage || mOwner->GetMultipart(),
+                    "Setting image when we already have one");
 
   mImage = aImage;
 
   // Apply any locks we have
   for (PRUint32 i = 0; i < mLockCount; ++i)
     mImage->LockImage();
 
   // Apply any animation consumers we have
--- a/image/src/imgRequestProxy.h
+++ b/image/src/imgRequestProxy.h
@@ -248,13 +248,13 @@ private:
   bool mListenerIsStrongRef;
   bool mDecodeRequested;
 
   // Whether we want to defer our notifications by the non-virtual Observer
   // interfaces as image loads proceed.
   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;
 };
 
 #endif // imgRequestProxy_h__
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -647,17 +647,20 @@ nsImageFrame::OnStopDecode(imgIRequest *
   NS_ASSERTION(imageLoader, "Who's notifying us??");
   PRInt32 loadType = nsIImageLoadingContent::UNKNOWN_REQUEST;
   imageLoader->GetRequestType(aRequest, &loadType);
   if (loadType != nsIImageLoadingContent::CURRENT_REQUEST &&
       loadType != nsIImageLoadingContent::PENDING_REQUEST) {
     return NS_ERROR_FAILURE;
   }
 
-  if (loadType == nsIImageLoadingContent::PENDING_REQUEST) {
+  bool multipart = false;
+  aRequest->GetMultipart(&multipart);
+
+  if (loadType == nsIImageLoadingContent::PENDING_REQUEST || multipart) {
     NotifyNewCurrentRequest(aRequest, aStatus);
   }
 
   return NS_OK;
 }
 
 void
 nsImageFrame::NotifyNewCurrentRequest(imgIRequest *aRequest,