Bug 1601980 - use MOZ_ATOMIC_BITFIELDS in imagelib to avoid races on flags. r=aosmond,decoder

Differential Revision: https://phabricator.services.mozilla.com/D91633
This commit is contained in:
Alexis Beingessner 2020-10-01 13:22:49 +00:00
parent 4484175a12
commit ab37cbed76
3 changed files with 94 additions and 101 deletions

View File

@ -77,18 +77,8 @@ RasterImage::RasterImage(nsIURI* aURI /* = nullptr */)
#ifdef DEBUG
mFramesNotified(0),
#endif
mSourceBuffer(MakeNotNull<SourceBuffer*>()),
mHasSize(false),
mTransient(false),
mSyncLoad(false),
mDiscardable(false),
mSomeSourceData(false),
mAllSourceData(false),
mHasBeenDecoded(false),
mPendingAnimation(false),
mAnimationFinished(false),
mWantFullDecode(false),
mHandledOrientation(StaticPrefs::image_honor_orientation_metadata()) {
mSourceBuffer(MakeNotNull<SourceBuffer*>()) {
SetHandledOrientation(StaticPrefs::image_honor_orientation_metadata());
}
//******************************************************************************
@ -122,10 +112,10 @@ nsresult RasterImage::Init(const char* aMimeType, uint32_t aFlags) {
!(aFlags & INIT_FLAG_DISCARDABLE));
// Store initialization data
mDiscardable = !!(aFlags & INIT_FLAG_DISCARDABLE);
mWantFullDecode = !!(aFlags & INIT_FLAG_DECODE_IMMEDIATELY);
mTransient = !!(aFlags & INIT_FLAG_TRANSIENT);
mSyncLoad = !!(aFlags & INIT_FLAG_SYNC_LOAD);
SetDiscardable(!!(aFlags & INIT_FLAG_DISCARDABLE));
SetWantFullDecode(!!(aFlags & INIT_FLAG_DECODE_IMMEDIATELY));
SetTransient(!!(aFlags & INIT_FLAG_TRANSIENT));
SetSyncLoad(!!(aFlags & INIT_FLAG_SYNC_LOAD));
// Use the MIME type to select a decoder type, and make sure there *is* a
// decoder for this MIME type.
@ -136,7 +126,7 @@ nsresult RasterImage::Init(const char* aMimeType, uint32_t aFlags) {
}
// Lock this image's surfaces in the SurfaceCache if we're not discardable.
if (!mDiscardable) {
if (!GetDiscardable()) {
mLockCount++;
SurfaceCache::LockImage(ImageKey(this));
}
@ -181,7 +171,7 @@ RasterImage::RequestRefresh(const TimeStamp& aTime) {
}
if (res.mAnimationFinished) {
mAnimationFinished = true;
SetAnimationFinished(true);
EvaluateAnimation();
}
}
@ -235,7 +225,7 @@ nsresult RasterImage::GetNativeSizes(nsTArray<IntSize>& aNativeSizes) const {
//******************************************************************************
size_t RasterImage::GetNativeSizesLength() const {
if (mError || !mHasSize) {
if (mError || !GetHasSize()) {
return 0;
}
@ -271,7 +261,7 @@ NS_IMETHODIMP_(Orientation)
RasterImage::GetOrientation() { return mOrientation; }
NS_IMETHODIMP_(bool)
RasterImage::HandledOrientation() { return mHandledOrientation; }
RasterImage::HandledOrientation() { return GetHandledOrientation(); }
//******************************************************************************
NS_IMETHODIMP
@ -344,7 +334,7 @@ LookupResult RasterImage::LookupFrame(const UnorientedIntSize& aSize,
LookupResult result =
LookupFrameInternal(requestedSize, aFlags, aPlaybackType, aMarkUsed);
if (!result && !mHasSize) {
if (!result && !GetHasSize()) {
// We can't request a decode without knowing our intrinsic size. Give up.
return LookupResult(MatchType::NOT_FOUND);
}
@ -392,7 +382,7 @@ LookupResult RasterImage::LookupFrame(const UnorientedIntSize& aSize,
// Sync decoding guarantees that we got the frame, but if it's owned by an
// async decoder that's currently running, the contents of the frame may not
// be available yet. Make sure we get everything.
if (mAllSourceData && syncDecode) {
if (GetAllSourceData() && syncDecode) {
result.Surface()->WaitUntilFinished();
}
@ -525,7 +515,7 @@ RasterImage::GetAnimated(bool* aAnimated) {
// images. This is true even though we check for animation during the
// metadata decode, because we may still discover animation only during the
// full decode for corrupt images.
if (!mHasBeenDecoded) {
if (!GetHasBeenDecoded()) {
return NS_ERROR_NOT_AVAILABLE;
}
@ -626,7 +616,7 @@ RasterImage::GetFrameInternal(const IntSize& aSize,
Tuple<ImgDrawResult, IntSize> RasterImage::GetImageContainerSize(
LayerManager* aManager, const IntSize& aRequestedSize, uint32_t aFlags) {
if (!mHasSize) {
if (!GetHasSize()) {
return MakeTuple(ImgDrawResult::NOT_READY, IntSize(0, 0));
}
@ -681,7 +671,7 @@ RasterImage::IsImageContainerAvailableAtSize(LayerManager* aManager,
// surface than mSize. If mSize > aRequestedSize, and mSize > maxTextureSize,
// we still want to use image containers if aRequestedSize <= maxTextureSize.
int32_t maxTextureSize = aManager->GetMaxTextureSize();
if (!mHasSize || aRequestedSize.IsEmpty() ||
if (!GetHasSize() || aRequestedSize.IsEmpty() ||
min(mSize.width, aRequestedSize.width) > maxTextureSize ||
min(mSize.height, aRequestedSize.height) > maxTextureSize) {
return false;
@ -735,7 +725,7 @@ bool RasterImage::SetMetadata(const ImageMetadata& aMetadata,
Orientation orientation = aMetadata.GetOrientation();
// If we already have a size, check the new size against the old one.
if (mHasSize &&
if (GetHasSize() &&
(metadataSize != ToUnoriented(mSize) || orientation != mOrientation)) {
NS_WARNING(
"Image changed size or orientation on redecode! "
@ -752,10 +742,10 @@ bool RasterImage::SetMetadata(const ImageMetadata& aMetadata,
mNativeSizes.AppendElement(
ToOriented(UnorientedIntSize::FromUnknownSize(nativeSize)));
}
mHasSize = true;
SetHasSize(true);
}
if (mHasSize && aMetadata.HasAnimation() && !mAnimationState) {
if (GetHasSize() && aMetadata.HasAnimation() && !mAnimationState) {
// We're becoming animated, so initialize animation stuff.
mAnimationState.emplace(mAnimationMode);
mFrameAnimator =
@ -823,16 +813,16 @@ nsresult RasterImage::StartAnimation() {
// If we're not ready to animate, then set mPendingAnimation, which will cause
// us to start animating if and when we do become ready.
mPendingAnimation =
!mAnimationState || mAnimationState->KnownFrameCount() < 1;
if (mPendingAnimation) {
SetPendingAnimation(!mAnimationState ||
mAnimationState->KnownFrameCount() < 1);
if (GetPendingAnimation()) {
return NS_OK;
}
// Don't bother to animate if we're displaying the first frame forever.
if (mAnimationState->GetCurrentAnimationFrameIndex() == 0 &&
mAnimationState->FirstFrameTimeout() == FrameTimeout::Forever()) {
mAnimationFinished = true;
SetAnimationFinished(true);
return NS_ERROR_ABORT;
}
@ -865,14 +855,14 @@ RasterImage::ResetAnimation() {
return NS_ERROR_FAILURE;
}
mPendingAnimation = false;
SetPendingAnimation(false);
if (mAnimationMode == kDontAnimMode || !mAnimationState ||
mAnimationState->GetCurrentAnimationFrameIndex() == 0) {
return NS_OK;
}
mAnimationFinished = false;
SetAnimationFinished(false);
if (mAnimating) {
StopAnimation();
@ -925,7 +915,7 @@ nsresult RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*,
MOZ_ASSERT(NS_IsMainThread());
// Record that we have all the data we're going to get now.
mAllSourceData = true;
SetAllSourceData(true);
// Let decoders know that there won't be any more data coming.
mSourceBuffer->Complete(aStatus);
@ -935,9 +925,9 @@ nsresult RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*,
// decoder could delay this image's load event quite a bit), or if this image
// is transient.
bool canSyncDecodeMetadata =
mSyncLoad || mTransient || DecodePool::NumberOfCores() < 2;
GetSyncLoad() || GetTransient() || DecodePool::NumberOfCores() < 2;
if (canSyncDecodeMetadata && !mHasSize) {
if (canSyncDecodeMetadata && !GetHasSize()) {
// We're loading this image synchronously, so it needs to be usable after
// this call returns. Since we haven't gotten our size yet, we need to do a
// synchronous metadata decode here.
@ -958,7 +948,7 @@ nsresult RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*,
Progress loadProgress = LoadCompleteProgress(aLastPart, mError, finalStatus);
if (!mHasSize && !mError) {
if (!GetHasSize() && !mError) {
// We don't have our size yet, so we'll fire the load event in SetSize().
MOZ_ASSERT(!canSyncDecodeMetadata,
"Firing load async after metadata sync decode?");
@ -972,9 +962,10 @@ nsresult RasterImage::OnImageDataComplete(nsIRequest*, nsISupports*,
}
void RasterImage::NotifyForLoadEvent(Progress aProgress) {
MOZ_ASSERT(mHasSize || mError, "Need to know size before firing load event");
MOZ_ASSERT(GetHasSize() || mError,
"Need to know size before firing load event");
MOZ_ASSERT(
!mHasSize || (mProgressTracker->GetProgress() & FLAG_SIZE_AVAILABLE),
!GetHasSize() || (mProgressTracker->GetProgress() & FLAG_SIZE_AVAILABLE),
"Should have notified that the size is available if we have it");
// If we encountered an error, make sure we notify for that as well.
@ -990,9 +981,9 @@ nsresult RasterImage::OnImageDataAvailable(nsIRequest*, nsISupports*,
nsIInputStream* aInputStream,
uint64_t, uint32_t aCount) {
nsresult rv = mSourceBuffer->AppendFromInputStream(aInputStream, aCount);
if (NS_SUCCEEDED(rv) && !mSomeSourceData) {
mSomeSourceData = true;
if (!mSyncLoad) {
if (NS_SUCCEEDED(rv) && !GetSomeSourceData()) {
SetSomeSourceData(true);
if (!GetSyncLoad()) {
// Create an async metadata decoder and verify we succeed in doing so.
rv = DecodeMetadata(DECODE_FLAGS_DEFAULT);
}
@ -1058,7 +1049,7 @@ void RasterImage::Discard() {
}
bool RasterImage::CanDiscard() {
return mAllSourceData &&
return GetAllSourceData() &&
// Can discard animated images if the pref is set
(!mAnimationState ||
StaticPrefs::image_mem_animated_discardable_AtStartup());
@ -1070,8 +1061,8 @@ RasterImage::StartDecoding(uint32_t aFlags, uint32_t aWhichFrame) {
return NS_ERROR_FAILURE;
}
if (!mHasSize) {
mWantFullDecode = true;
if (!GetHasSize()) {
SetWantFullDecode(true);
return NS_OK;
}
@ -1086,8 +1077,8 @@ bool RasterImage::StartDecodingWithResult(uint32_t aFlags,
return false;
}
if (!mHasSize) {
mWantFullDecode = true;
if (!GetHasSize()) {
SetWantFullDecode(true);
return false;
}
@ -1150,8 +1141,8 @@ LookupResult RasterImage::RequestDecodeForSizeInternal(
return result;
}
if (!mHasSize) {
mWantFullDecode = true;
if (!GetHasSize()) {
SetWantFullDecode(true);
return LookupResult(MatchType::NOT_FOUND);
}
@ -1159,7 +1150,7 @@ LookupResult RasterImage::RequestDecodeForSizeInternal(
// explicitly trading off flashing for responsiveness in the case that we're
// redecoding an image (see bug 845147).
bool shouldSyncDecodeIfFast =
!mHasBeenDecoded && (aFlags & FLAG_SYNC_DECODE_IF_FAST);
!GetHasBeenDecoded() && (aFlags & FLAG_SYNC_DECODE_IF_FAST);
uint32_t flags =
shouldSyncDecodeIfFast ? aFlags : aFlags & ~FLAG_SYNC_DECODE_IF_FAST;
@ -1202,8 +1193,8 @@ void RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
}
// If we don't have a size yet, we can't do any other decoding.
if (!mHasSize) {
mWantFullDecode = true;
if (!GetHasSize()) {
SetWantFullDecode(true);
return;
}
@ -1221,10 +1212,10 @@ void RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
if (aFlags & FLAG_ASYNC_NOTIFY) {
decoderFlags |= DecoderFlags::ASYNC_NOTIFY;
}
if (mTransient) {
if (GetTransient()) {
decoderFlags |= DecoderFlags::IMAGE_IS_TRANSIENT;
}
if (mHasBeenDecoded) {
if (GetHasBeenDecoded()) {
decoderFlags |= DecoderFlags::IS_REDECODE;
}
if ((aFlags & FLAG_SYNC_DECODE) || !(aFlags & FLAG_HIGH_QUALITY_SCALING)) {
@ -1290,7 +1281,7 @@ void RasterImage::Decode(const UnorientedIntSize& aSize, uint32_t aFlags,
mDecodeCount++;
// We're ready to decode; start the decoder.
aOutRanSync = LaunchDecodingTask(task, this, aFlags, mAllSourceData);
aOutRanSync = LaunchDecodingTask(task, this, aFlags, GetAllSourceData());
}
NS_IMETHODIMP
@ -1299,7 +1290,7 @@ RasterImage::DecodeMetadata(uint32_t aFlags) {
return NS_ERROR_FAILURE;
}
MOZ_ASSERT(!mHasSize, "Should not do unnecessary metadata decodes");
MOZ_ASSERT(!GetHasSize(), "Should not do unnecessary metadata decodes");
// Create a decoder.
RefPtr<IDecodingTask> task = DecoderFactory::CreateMetadataDecoder(
@ -1311,13 +1302,13 @@ RasterImage::DecodeMetadata(uint32_t aFlags) {
}
// We're ready to decode; start the decoder.
LaunchDecodingTask(task, this, aFlags, mAllSourceData);
LaunchDecodingTask(task, this, aFlags, GetAllSourceData());
return NS_OK;
}
void RasterImage::RecoverFromInvalidFrames(const UnorientedIntSize& aSize,
uint32_t aFlags) {
if (!mHasSize) {
if (!GetHasSize()) {
return;
}
@ -1359,7 +1350,7 @@ bool RasterImage::CanDownscaleDuringDecode(const UnorientedIntSize& aSize,
// Check basic requirements: downscale-during-decode is enabled, Skia is
// available, this image isn't transient, we have all the source data and know
// our size, and the flags allow us to do it.
if (!mHasSize || mTransient || !HaveSkia() ||
if (!GetHasSize() || GetTransient() || !HaveSkia() ||
!StaticPrefs::image_downscale_during_decode_enabled() ||
!(aFlags & imgIContainer::FLAG_HIGH_QUALITY_SCALING)) {
return false;
@ -1562,8 +1553,8 @@ RasterImage::UnlockImage() {
NS_IMETHODIMP
RasterImage::RequestDiscard() {
if (mDiscardable && // Enabled at creation time...
mLockCount == 0 && // ...not temporarily disabled...
if (GetDiscardable() && // Enabled at creation time...
mLockCount == 0 && // ...not temporarily disabled...
CanDiscard()) {
Discard();
}
@ -1630,7 +1621,7 @@ RasterImage::HandleErrorWorker::Run() {
bool RasterImage::ShouldAnimate() {
return ImageResource::ShouldAnimate() && mAnimationState &&
mAnimationState->KnownFrameCount() >= 1 && !mAnimationFinished;
mAnimationState->KnownFrameCount() >= 1 && !GetAnimationFinished();
}
#ifdef DEBUG
@ -1665,7 +1656,7 @@ void RasterImage::NotifyProgress(
}
// If we should start animating right now, do so.
if (mAnimationState && aFrameCount == Some(1u) && mPendingAnimation &&
if (mAnimationState && aFrameCount == Some(1u) && GetPendingAnimation() &&
ShouldAnimate()) {
StartAnimation();
}
@ -1716,12 +1707,12 @@ void RasterImage::NotifyDecodeComplete(
return;
}
MOZ_ASSERT(mError || mHasSize || !aMetadata.HasSize(),
MOZ_ASSERT(mError || GetHasSize() || !aMetadata.HasSize(),
"SetMetadata should've gotten a size");
if (!aStatus.mWasMetadataDecode && aStatus.mFinished) {
// Flag that we've been decoded before.
mHasBeenDecoded = true;
SetHasBeenDecoded(true);
}
// Send out any final notifications.
@ -1736,12 +1727,12 @@ void RasterImage::NotifyDecodeComplete(
}
// If we should start animating right now, do so.
if (mAnimationState && aFrameCount == Some(1u) && mPendingAnimation &&
if (mAnimationState && aFrameCount == Some(1u) && GetPendingAnimation() &&
ShouldAnimate()) {
StartAnimation();
}
if (mAnimationState && mHasBeenDecoded) {
if (mAnimationState && GetHasBeenDecoded()) {
// We've finished a full decode of all animation frames and our
// AnimationState has been notified about them all, so let it know not to
// expect anymore.
@ -1778,7 +1769,7 @@ void RasterImage::NotifyDecodeComplete(
if (aStatus.mHadError &&
(!mAnimationState || mAnimationState->KnownFrameCount() == 0)) {
DoError();
} else if (aStatus.mWasMetadataDecode && !mHasSize) {
} else if (aStatus.mWasMetadataDecode && !GetHasSize()) {
DoError();
}
@ -1791,8 +1782,8 @@ void RasterImage::NotifyDecodeComplete(
}
// If we were a metadata decode and a full decode was requested, do it.
if (mWantFullDecode) {
mWantFullDecode = false;
if (GetWantFullDecode()) {
SetWantFullDecode(false);
RequestDecodeForSize(mSize.ToUnknownSize(),
DECODE_FLAGS_DEFAULT | FLAG_HIGH_QUALITY_SCALING,
FRAME_CURRENT);

View File

@ -28,6 +28,7 @@
#include "ImageMetadata.h"
#include "ISurfaceProvider.h"
#include "Orientation.h"
#include "mozilla/AtomicBitfields.h"
#include "mozilla/Attributes.h"
#include "mozilla/Maybe.h"
#include "mozilla/MemoryReporting.h"
@ -397,7 +398,7 @@ class RasterImage final : public ImageResource,
* Orientation that indicates no transformation is needed.
*/
Orientation UsedOrientation() const {
return mHandledOrientation ? mOrientation : Orientation();
return GetHandledOrientation() ? mOrientation : Orientation();
}
// Functions to convert between oriented and unoriented pixels.
@ -462,35 +463,40 @@ class RasterImage final : public ImageResource,
NotNull<RefPtr<SourceBuffer>> mSourceBuffer;
// Boolean flags (clustered together to conserve space):
bool mHasSize : 1; // Has SetSize() been called?
bool mTransient : 1; // Is the image short-lived?
bool mSyncLoad : 1; // Are we loading synchronously?
bool mDiscardable : 1; // Is container discardable?
bool mSomeSourceData : 1; // Do we have some source data?
bool mAllSourceData : 1; // Do we have all the source data?
bool mHasBeenDecoded : 1; // Decoded at least once?
MOZ_ATOMIC_BITFIELDS(
mAtomicBitfields, 16,
((bool, HasSize, 1), // Has SetSize() been called?
(bool, Transient, 1), // Is the image short-lived?
(bool, SyncLoad, 1), // Are we loading synchronously?
(bool, Discardable, 1), // Is container discardable?
(bool, SomeSourceData, 1), // Do we have some source data?
(bool, AllSourceData, 1), // Do we have all the source data?
(bool, HasBeenDecoded, 1), // Decoded at least once?
// Whether we're waiting to start animation. If we get a StartAnimation() call
// but we don't yet have more than one frame, mPendingAnimation is set so that
// we know to start animation later if/when we have more frames.
bool mPendingAnimation : 1;
// Whether we're waiting to start animation. If we get a StartAnimation()
// call but we don't yet have more than one frame, mPendingAnimation is
// set so that we know to start animation later if/when we have more
// frames.
(bool, PendingAnimation, 1),
// Whether the animation can stop, due to running out
// of frames, or no more owning request
bool mAnimationFinished : 1;
// Whether the animation can stop, due to running out
// of frames, or no more owning request
(bool, AnimationFinished, 1),
// Whether, once we are done doing a metadata decode, we should immediately
// kick off a full decode.
bool mWantFullDecode : 1;
// Whether, once we are done doing a metadata decode, we should
// immediately kick off a full decode.
(bool, WantFullDecode, 1),
// Whether this RasterImage handled orientation of the image.
//
// This will be set based on the value of the image.honor-orientation-metadata
// pref at the time the RasterImage is created.
//
// NOTE(heycam): Once the image.honor-orientation-metadata pref is removed,
// this member (and the UsedOrientation() function) can also be removed.
bool mHandledOrientation : 1;
// Whether this RasterImage handled orientation of the image.
//
// This will be set based on the value of the
// image.honor-orientation-metadata pref at the time the RasterImage is
// created.
//
// NOTE(heycam): Once the image.honor-orientation-metadata pref is
// removed, this member (and the UsedOrientation() function) can also be
// removed.
(bool, HandledOrientation, 1)))
TimeStamp mDrawStartTime;

View File

@ -150,10 +150,6 @@ extern "C" const char* __tsan_default_suppressions() {
"race:ScriptPreloader::MaybeFinishOffThreadDecode\n"
"race:ScriptPreloader::DoFinishOffThreadDecode\n"
// Bug 1601980
"race:image::RasterImage::StartDecoding\n"
"race:image::RasterImage::OnImageDataAvailable\n"
// Bug 1606651
"race:nsPluginTag::nsPluginTag\n"
"race:nsFakePluginTag\n"