Bug 523528 - Crashes in imgFrame::Draw(). Handle failure to composite frames gracefully, instead of just asserting that it's impossible to get into a situation where we draw paletted frames. r=jrmuizel,vlad,ak
authorJoe Drew <joe@drew.ca>
Thu, 12 Nov 2009 18:18:40 -0500
changeset 34809 fe0a0e18a51f39170c7685b3fcd58ede09219be9
parent 34808 2d854f2c85f6af569b052ff9678ce4138af1f116
child 34810 374169762764044d137b13825ee1f22ac8a965ba
push id10272
push userjdrew@mozilla.com
push dateThu, 12 Nov 2009 23:24:18 +0000
treeherdermozilla-central@fe0a0e18a51f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, vlad, ak
bugs523528
milestone1.9.3a1pre
Bug 523528 - Crashes in imgFrame::Draw(). Handle failure to composite frames gracefully, instead of just asserting that it's impossible to get into a situation where we draw paletted frames. r=jrmuizel,vlad,ak
modules/libpr0n/src/imgContainer.cpp
modules/libpr0n/src/imgContainer.h
modules/libpr0n/src/imgFrame.cpp
modules/libpr0n/src/imgFrame.h
--- a/modules/libpr0n/src/imgContainer.cpp
+++ b/modules/libpr0n/src/imgContainer.cpp
@@ -307,17 +307,17 @@ NS_IMETHODIMP imgContainer::ExtractFrame
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
   // Get the frame. If it's not there, it's probably the caller's fault for
   // not waiting for the data to be loaded from the network or not passing
   // FLAG_SYNC_DECODE
   PRUint32 frameIndex = (aWhichFrame == FRAME_FIRST) ?
                         0 : GetCurrentImgFrameIndex();
-  imgFrame *frame = GetImgFrame(frameIndex);
+  imgFrame *frame = GetDrawableImgFrame(frameIndex);
   if (!frame) {
     *_retval = nsnull;
     return NS_ERROR_FAILURE;
   }
 
   // The frame can be smaller than the image. We want to extract only the part
   // of the frame that actually exists.
   nsIntRect framerect = frame->GetRect();
@@ -373,29 +373,45 @@ imgFrame *imgContainer::GetImgFrame(PRUi
     NS_ASSERTION(framenum == 0, "Don't ask for a frame > 0 if we're not animated!");
     return mFrames.SafeElementAt(0, nsnull);
   }
   if (mAnim->lastCompositedFrameIndex == PRInt32(framenum))
     return mAnim->compositingFrame;
   return mFrames.SafeElementAt(framenum, nsnull);
 }
 
+imgFrame *imgContainer::GetDrawableImgFrame(PRUint32 framenum)
+{
+  imgFrame *frame = GetImgFrame(framenum);
+
+  // We will return a paletted frame if it's not marked as compositing failed
+  // so we can catch crashes for reasons we haven't investigated.
+  if (frame && frame->GetCompositingFailed())
+    return nsnull;
+  return frame;
+}
+
 PRUint32 imgContainer::GetCurrentImgFrameIndex() const
 {
   if (mAnim)
     return mAnim->currentAnimationFrameIndex;
 
   return 0;
 }
 
 imgFrame *imgContainer::GetCurrentImgFrame()
 {
   return GetImgFrame(GetCurrentImgFrameIndex());
 }
 
+imgFrame *imgContainer::GetCurrentDrawableImgFrame()
+{
+  return GetDrawableImgFrame(GetCurrentImgFrameIndex());
+}
+
 //******************************************************************************
 /* readonly attribute boolean currentFrameIsOpaque; */
 NS_IMETHODIMP imgContainer::GetCurrentFrameIsOpaque(PRBool *aIsOpaque)
 {
   NS_ENSURE_ARG_POINTER(aIsOpaque);
 
   if (mError)
     return NS_ERROR_FAILURE;
@@ -525,17 +541,17 @@ NS_IMETHODIMP imgContainer::CopyFrame(PR
 
   NS_ENSURE_ARG_POINTER(_retval);
 
   // Get the frame. If it's not there, it's probably the caller's fault for
   // not waiting for the data to be loaded from the network or not passing
   // FLAG_SYNC_DECODE
   PRUint32 frameIndex = (aWhichFrame == FRAME_FIRST) ?
                         0 : GetCurrentImgFrameIndex();
-  imgFrame *frame = GetImgFrame(frameIndex);
+  imgFrame *frame = GetDrawableImgFrame(frameIndex);
   if (!frame) {
     *_retval = nsnull;
     return NS_ERROR_FAILURE;
   }
 
   nsRefPtr<gfxPattern> pattern;
   frame->GetPattern(getter_AddRefs(pattern));
   nsIntRect intframerect = frame->GetRect();
@@ -576,17 +592,17 @@ NS_IMETHODIMP imgContainer::GetFrame(PRU
     CONTAINER_ENSURE_SUCCESS(rv);
   }
 
   // Get the frame. If it's not there, it's probably the caller's fault for
   // not waiting for the data to be loaded from the network or not passing
   // FLAG_SYNC_DECODE
   PRUint32 frameIndex = (aWhichFrame == FRAME_FIRST) ?
                           0 : GetCurrentImgFrameIndex();
-  imgFrame *frame = GetImgFrame(frameIndex);
+  imgFrame *frame = GetDrawableImgFrame(frameIndex);
   if (!frame) {
     *_retval = nsnull;
     return NS_ERROR_FAILURE;
   }
 
   nsRefPtr<gfxASurface> framesurf;
 
   // If this frame covers the entire image, we can just reuse its existing
@@ -1440,18 +1456,21 @@ NS_IMETHODIMP imgContainer::Notify(nsITi
     if (!prevFrame)
       return NS_OK;
 
     // Change frame and announce it
     if (NS_FAILED(DoComposite(&frameToUse, &dirtyRect, prevFrame,
                               nextFrame, nextFrameIndex))) {
       // something went wrong, move on to next
       NS_WARNING("imgContainer::Notify(): Composing Frame Failed\n");
+      nextFrame->SetCompositingFailed(PR_TRUE);
       mAnim->currentAnimationFrameIndex = nextFrameIndex;
       return NS_OK;
+    } else {
+      nextFrame->SetCompositingFailed(PR_FALSE);
     }
   }
   // Set currentAnimationFrameIndex at the last possible moment
   mAnim->currentAnimationFrameIndex = nextFrameIndex;
   // Refreshes the screen
   observer->FrameChanged(this, &dirtyRect);
   
   return NS_OK;
@@ -2323,17 +2342,17 @@ NS_IMETHODIMP imgContainer::Draw(gfxCont
   NS_ENSURE_ARG_POINTER(aContext);
 
   // If a synchronous draw is requested, flush anything that might be sitting around
   if (aFlags & FLAG_SYNC_DECODE) {
     nsresult rv = SyncDecode();
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
-  imgFrame *frame = GetCurrentImgFrame();
+  imgFrame *frame = GetCurrentDrawableImgFrame();
   if (!frame) {
     NS_ABORT_IF_FALSE(!mDecoded, "Decoded but frame not available?");
     return NS_OK; // Getting the frame (above) touches the image and kicks off decoding
   }
 
   nsIntRect framerect = frame->GetRect();
   nsIntMargin padding(framerect.x, framerect.y, 
                       mSize.width - framerect.XMost(),
--- a/modules/libpr0n/src/imgContainer.h
+++ b/modules/libpr0n/src/imgContainer.h
@@ -201,17 +201,19 @@ private:
     ~Anim()
     {
       if (timer)
         timer->Cancel();
     }
   };
 
   imgFrame* GetImgFrame(PRUint32 framenum);
+  imgFrame* GetDrawableImgFrame(PRUint32 framenum);
   imgFrame* GetCurrentImgFrame();
+  imgFrame* GetCurrentDrawableImgFrame();
   PRUint32 GetCurrentImgFrameIndex() const;
   
   inline Anim* ensureAnimExists()
   {
     if (!mAnim) {
 
       // Create the animation context
       mAnim = new Anim();
--- a/modules/libpr0n/src/imgFrame.cpp
+++ b/modules/libpr0n/src/imgFrame.cpp
@@ -147,17 +147,18 @@ imgFrame::imgFrame() :
   mDecoded(0, 0, 0, 0),
   mPalettedImageData(nsnull),
   mSinglePixelColor(0),
   mTimeout(100),
   mDisposalMethod(0), /* imgIContainer::kDisposeNotSpecified */
   mBlendMethod(1), /* imgIContainer::kBlendOver */
   mSinglePixel(PR_FALSE),
   mNeverUseDeviceSurface(PR_FALSE),
-  mFormatChanged(PR_FALSE)
+  mFormatChanged(PR_FALSE),
+  mCompositingFailed(PR_FALSE)
 #ifdef USE_WIN_SURFACE
   , mIsDDBSurface(PR_FALSE)
 #endif
 {
   static PRBool hasCheckedOptimize = PR_FALSE;
   if (!hasCheckedOptimize) {
     if (PR_GetEnv("MOZ_DISABLE_IMAGE_OPTIMIZE")) {
       gDisableOptimize = PR_TRUE;
@@ -910,8 +911,18 @@ PRBool imgFrame::ImageComplete() const
 // set to 0xff.
 void imgFrame::SetHasNoAlpha()
 {
   if (mFormat == gfxASurface::ImageFormatARGB32) {
       mFormat = gfxASurface::ImageFormatRGB24;
       mFormatChanged = PR_TRUE;
   }
 }
+
+PRBool imgFrame::GetCompositingFailed() const
+{
+  return mCompositingFailed;
+}
+
+void imgFrame::SetCompositingFailed(PRBool val)
+{
+  mCompositingFailed = val;
+}
--- a/modules/libpr0n/src/imgFrame.h
+++ b/modules/libpr0n/src/imgFrame.h
@@ -92,16 +92,19 @@ public:
   PRInt32 GetFrameDisposalMethod() const;
   void SetFrameDisposalMethod(PRInt32 aFrameDisposalMethod);
   PRInt32 GetBlendMethod() const;
   void SetBlendMethod(PRInt32 aBlendMethod);
   PRBool ImageComplete() const;
 
   void SetHasNoAlpha();
 
+  PRBool GetCompositingFailed() const;
+  void SetCompositingFailed(PRBool val);
+
   nsresult LockImageData();
   nsresult UnlockImageData();
 
   nsresult GetSurface(gfxASurface **aSurface) const
   {
     *aSurface = ThebesSurface();
     NS_IF_ADDREF(*aSurface);
     return NS_OK;
@@ -163,16 +166,17 @@ private: // data
   PRInt32      mDisposalMethod;
 
   gfxASurface::gfxImageFormat mFormat;
   PRInt8       mPaletteDepth;
   PRInt8       mBlendMethod;
   PRPackedBool mSinglePixel;
   PRPackedBool mNeverUseDeviceSurface;
   PRPackedBool mFormatChanged;
+  PRPackedBool mCompositingFailed;
 
 #ifdef XP_WIN
   PRPackedBool mIsDDBSurface;
 #endif
 
 };
 
 #endif /* imgFrame_h */