Bug 786444 - Part 1 - Add an RAII class to make locking imgFrame more foolproof, use it, and be sure to lock imgFrame more correctly. r=jlebar

--HG--
extra : rebase_source : 255805f32aa06b3719a1442388b7b759c1c16c89
This commit is contained in:
Joe Drew 2012-09-26 11:33:06 -04:00
parent b44769c993
commit 555228332f
2 changed files with 48 additions and 13 deletions

View File

@ -999,15 +999,15 @@ RasterImage::InternalAddFrameHelper(uint32_t framenum, imgFrame *aFrame,
nsAutoPtr<imgFrame> 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<imgFrame> 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<gfxPattern> srcPatt;
aSrc->GetPattern(getter_AddRefs(srcPatt));
aDst->LockImageData();
nsRefPtr<gfxASurface> dstSurf;
aDst->GetSurface(getter_AddRefs(dstSurf));
@ -2124,8 +2132,6 @@ RasterImage::DrawFrameTo(imgFrame *aSrc,
dst.SetPattern(srcPatt);
dst.Paint();
aDst->UnlockImageData();
return NS_OK;
}

View File

@ -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 */