diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index d71b8ab6d4d..f86b07009a7 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -999,15 +999,15 @@ RasterImage::InternalAddFrameHelper(uint32_t framenum, imgFrame *aFrame, nsAutoPtr frame(aFrame); + // We are in the middle of decoding. This will be unlocked when we finish the + // decoder->Write() call. + frame->LockImageData(); + if (paletteData && paletteLength) frame->GetPaletteData(paletteData, paletteLength); frame->GetImageData(imageData, imageLength); - // We are in the middle of decoding. This will be unlocked when we finish the - // decoder->Write() call. - frame->LockImageData(); - mFrames.InsertElementAt(framenum, frame.forget()); return NS_OK; @@ -1187,6 +1187,11 @@ RasterImage::EnsureFrame(uint32_t aFrameNum, int32_t aX, int32_t aY, } // Not reusable, so replace the frame directly. + + // We know this frame is already locked, because it's the one we're currently + // writing to. + frame->UnlockImageData(); + DeleteImgFrame(aFrameNum); mFrames.RemoveElementAt(aFrameNum); nsAutoPtr newFrame(new imgFrame()); @@ -1999,18 +2004,21 @@ RasterImage::CopyFrameImage(imgFrame *aSrcFrame, if (!aSrcFrame || !aDstFrame) return false; - if (NS_FAILED(aDstFrame->LockImageData())) + AutoFrameLocker dstLock(aDstFrame); + AutoFrameLocker srcLock(aSrcFrame); + + if (!srcLock.Succeeded() || !dstLock.Succeeded()) { return false; + } // Copy Image Over aSrcFrame->GetImageData(&aDataSrc, &aDataLengthSrc); aDstFrame->GetImageData(&aDataDest, &aDataLengthDest); if (!aDataDest || !aDataSrc || aDataLengthDest != aDataLengthSrc) { - aDstFrame->UnlockImageData(); return false; } + memcpy(aDataDest, aDataSrc, aDataLengthSrc); - aDstFrame->UnlockImageData(); return true; } @@ -2030,6 +2038,9 @@ RasterImage::DrawFrameTo(imgFrame *aSrc, NS_ENSURE_ARG_POINTER(aSrc); NS_ENSURE_ARG_POINTER(aDst); + AutoFrameLocker srcLock(aSrc); + AutoFrameLocker dstLock(aDst); + nsIntRect dstRect = aDst->GetRect(); // According to both AGIF and APNG specs, offsets are unsigned @@ -2057,7 +2068,7 @@ RasterImage::DrawFrameTo(imgFrame *aSrc, NS_ASSERTION((width <= aSrcRect.width) && (height <= aSrcRect.height), "RasterImage::DrawFrameTo: source must be smaller than dest"); - if (NS_FAILED(aDst->LockImageData())) + if (!srcLock.Succeeded() || !dstLock.Succeeded()) return NS_ERROR_FAILURE; // Get pointers to image data @@ -2070,7 +2081,6 @@ RasterImage::DrawFrameTo(imgFrame *aSrc, aSrc->GetPaletteData(&colormap, &size); aDst->GetImageData((uint8_t **)&dstPixels, &size); if (!srcPixels || !dstPixels || !colormap) { - aDst->UnlockImageData(); return NS_ERROR_FAILURE; } @@ -2098,14 +2108,12 @@ RasterImage::DrawFrameTo(imgFrame *aSrc, } } - aDst->UnlockImageData(); return NS_OK; } nsRefPtr srcPatt; aSrc->GetPattern(getter_AddRefs(srcPatt)); - aDst->LockImageData(); nsRefPtr dstSurf; aDst->GetSurface(getter_AddRefs(dstSurf)); @@ -2124,8 +2132,6 @@ RasterImage::DrawFrameTo(imgFrame *aSrc, dst.SetPattern(srcPatt); dst.Paint(); - aDst->UnlockImageData(); - return NS_OK; } diff --git a/image/src/imgFrame.h b/image/src/imgFrame.h index fa27560b670..b6d434239cb 100644 --- a/image/src/imgFrame.h +++ b/image/src/imgFrame.h @@ -176,4 +176,33 @@ private: // data #endif }; +namespace mozilla { +namespace image { + // An RAII class to ensure it's easy to balance locks and unlocks on + // imgFrames. + class AutoFrameLocker + { + public: + AutoFrameLocker(imgFrame* frame) + : mFrame(frame) + , mSucceeded(NS_SUCCEEDED(frame->LockImageData())) + {} + + ~AutoFrameLocker() + { + if (mSucceeded) { + mFrame->UnlockImageData(); + } + } + + // Whether the lock request succeeded. + bool Succeeded() { return mSucceeded; } + + private: + imgFrame* mFrame; + bool mSucceeded; + }; +} +} + #endif /* imgFrame_h */