Bug 867755 - Add strong refcnting for derived classes of mozilla::image::Image r=seth

This commit is contained in:
Steve Workman 2013-09-28 11:28:44 -07:00
parent 71940cda4d
commit 38b912cd11
11 changed files with 149 additions and 58 deletions

View File

@ -11,8 +11,7 @@ namespace mozilla {
namespace image {
// Constructor
ImageResource::ImageResource(imgStatusTracker* aStatusTracker,
ImageURL* aURI) :
ImageResource::ImageResource(ImageURL* aURI) :
mURI(aURI),
mInnerWindowId(0),
mAnimationConsumers(0),
@ -21,12 +20,6 @@ ImageResource::ImageResource(imgStatusTracker* aStatusTracker,
mAnimating(false),
mError(false)
{
if (aStatusTracker) {
mStatusTracker = aStatusTracker;
mStatusTracker->SetImage(this);
} else {
mStatusTracker = new imgStatusTracker(this);
}
}
uint32_t

View File

@ -63,6 +63,7 @@ public:
uint32_t aFlags) = 0;
virtual already_AddRefed<imgStatusTracker> GetStatusTracker() = 0;
virtual void SetStatusTracker(imgStatusTracker* aStatusTracker) {}
/**
* The rectangle defining the location and size of the given frame.
@ -137,11 +138,16 @@ public:
class ImageResource : public Image
{
public:
virtual already_AddRefed<imgStatusTracker> GetStatusTracker() MOZ_OVERRIDE {
already_AddRefed<imgStatusTracker> GetStatusTracker() MOZ_OVERRIDE {
nsRefPtr<imgStatusTracker> statusTracker = mStatusTracker;
MOZ_ASSERT(statusTracker);
return statusTracker.forget();
}
void SetStatusTracker(imgStatusTracker* aStatusTracker) MOZ_OVERRIDE MOZ_FINAL {
MOZ_ASSERT(aStatusTracker);
MOZ_ASSERT(!mStatusTracker);
mStatusTracker = aStatusTracker;
}
virtual uint32_t SizeOfData() MOZ_OVERRIDE;
virtual void IncrementAnimationConsumers() MOZ_OVERRIDE;
@ -165,8 +171,7 @@ public:
virtual ImageURL* GetURI() MOZ_OVERRIDE { return mURI.get(); }
protected:
ImageResource(imgStatusTracker* aStatusTracker,
ImageURL* aURI);
ImageResource(ImageURL* aURI);
// Shared functionality for implementors of imgIContainer. Every
// implementation of attribute animationMode should forward here.

View File

@ -376,7 +376,7 @@ NS_IMPL_ISUPPORTS3(RasterImage, imgIContainer, nsIProperties,
//******************************************************************************
RasterImage::RasterImage(imgStatusTracker* aStatusTracker,
ImageURL* aURI /* = nullptr */) :
ImageResource(aStatusTracker, aURI), // invoke superclass's constructor
ImageResource(aURI), // invoke superclass's constructor
mSize(0,0),
mFrameDecodeFlags(DECODE_FLAGS_DEFAULT),
mMultipartDecodedFrame(nullptr),
@ -404,6 +404,8 @@ RasterImage::RasterImage(imgStatusTracker* aStatusTracker,
mPendingError(false),
mScaleRequest(nullptr)
{
mStatusTrackerInit = new imgStatusTrackerInit(this, aStatusTracker);
// Set up the discard tracker node.
mDiscardTrackerNode.img = this;
Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(0);
@ -449,6 +451,7 @@ RasterImage::~RasterImage()
}
delete mAnim;
mAnim = nullptr;
delete mMultipartDecodedFrame;
// Total statistics

View File

@ -717,6 +717,9 @@ private: // data
// will inform us when to let go of this pointer.
ScaleRequest* mScaleRequest;
// Initializes imgStatusTracker and resets it on RasterImage destruction.
nsAutoPtr<imgStatusTrackerInit> mStatusTrackerInit;
nsresult ShutdownDecoder(eShutdownIntent aIntent);
// Error handling.

View File

@ -303,13 +303,14 @@ NS_IMPL_ISUPPORTS3(VectorImage,
VectorImage::VectorImage(imgStatusTracker* aStatusTracker,
ImageURL* aURI /* = nullptr */) :
ImageResource(aStatusTracker, aURI), // invoke superclass's constructor
ImageResource(aURI), // invoke superclass's constructor
mIsInitialized(false),
mIsFullyLoaded(false),
mIsDrawing(false),
mHaveAnimations(false),
mHasPendingInvalidation(false)
{
mStatusTrackerInit = new imgStatusTrackerInit(this, aStatusTracker);
}
VectorImage::~VectorImage()

View File

@ -98,6 +98,9 @@ private:
bool mHasPendingInvalidation; // Invalidate observers next refresh
// driver tick.
// Initializes imgStatusTracker and resets it on RasterImage destruction.
nsAutoPtr<imgStatusTrackerInit> mStatusTrackerInit;
friend class ImageFactory;
};

View File

@ -800,7 +800,7 @@ imgRequest::OnDataAvailable(nsIRequest *aRequest, nsISupports *ctxt,
return NS_BINDING_ABORTED;
}
NS_ABORT_IF_FALSE(statusTracker->GetImage(), "Status tracker should have an image!");
NS_ABORT_IF_FALSE(statusTracker->HasImage(), "Status tracker should have an image!");
NS_ABORT_IF_FALSE(mImage, "imgRequest should have an image!");
if (mDecodeRequested)

View File

@ -26,7 +26,8 @@ class ProxyBehaviour
public:
virtual ~ProxyBehaviour() {}
virtual mozilla::image::Image* GetImage() const = 0;
virtual already_AddRefed<mozilla::image::Image> GetImage() const = 0;
virtual bool HasImage() const = 0;
virtual already_AddRefed<imgStatusTracker> GetStatusTracker() const = 0;
virtual imgRequest* GetOwner() const = 0;
virtual void SetOwner(imgRequest* aOwner) = 0;
@ -37,7 +38,8 @@ class RequestBehaviour : public ProxyBehaviour
public:
RequestBehaviour() : mOwner(nullptr), mOwnerHasImage(false) {}
virtual mozilla::image::Image* GetImage() const MOZ_OVERRIDE;
virtual already_AddRefed<mozilla::image::Image> GetImage() const MOZ_OVERRIDE;
virtual bool HasImage() const MOZ_OVERRIDE;
virtual already_AddRefed<imgStatusTracker> GetStatusTracker() const MOZ_OVERRIDE;
virtual imgRequest* GetOwner() const MOZ_OVERRIDE {
@ -49,7 +51,7 @@ class RequestBehaviour : public ProxyBehaviour
if (mOwner) {
nsRefPtr<imgStatusTracker> ownerStatusTracker = GetStatusTracker();
mOwnerHasImage = ownerStatusTracker && ownerStatusTracker->GetImage();
mOwnerHasImage = ownerStatusTracker && ownerStatusTracker->HasImage();
} else {
mOwnerHasImage = false;
}
@ -67,7 +69,7 @@ class RequestBehaviour : public ProxyBehaviour
bool mOwnerHasImage;
};
mozilla::image::Image*
already_AddRefed<mozilla::image::Image>
RequestBehaviour::GetImage() const
{
if (!mOwnerHasImage)
@ -200,7 +202,7 @@ nsresult imgRequestProxy::ChangeOwner(imgRequest *aNewOwner)
// Were we decoded before?
bool wasDecoded = false;
nsRefPtr<imgStatusTracker> statusTracker = GetStatusTracker();
if (GetImage() &&
if (statusTracker->HasImage() &&
statusTracker->GetImageStatus() & imgIRequest::STATUS_FRAME_COMPLETE) {
wasDecoded = true;
}
@ -381,8 +383,9 @@ NS_IMETHODIMP
imgRequestProxy::LockImage()
{
mLockCount++;
if (GetImage())
return GetImage()->LockImage();
nsRefPtr<Image> image = GetImage();
if (image)
return image->LockImage();
return NS_OK;
}
@ -393,8 +396,9 @@ imgRequestProxy::UnlockImage()
NS_ABORT_IF_FALSE(mLockCount > 0, "calling unlock but no locks!");
mLockCount--;
if (GetImage())
return GetImage()->UnlockImage();
nsRefPtr<Image> image = GetImage();
if (image)
return image->UnlockImage();
return NS_OK;
}
@ -402,8 +406,9 @@ imgRequestProxy::UnlockImage()
NS_IMETHODIMP
imgRequestProxy::RequestDiscard()
{
if (GetImage())
return GetImage()->RequestDiscard();
nsRefPtr<Image> image = GetImage();
if (image)
return image->RequestDiscard();
return NS_OK;
}
@ -411,8 +416,9 @@ NS_IMETHODIMP
imgRequestProxy::IncrementAnimationConsumers()
{
mAnimationConsumers++;
if (GetImage())
GetImage()->IncrementAnimationConsumers();
nsRefPtr<Image> image = GetImage();
if (image)
image->IncrementAnimationConsumers();
return NS_OK;
}
@ -427,8 +433,9 @@ imgRequestProxy::DecrementAnimationConsumers()
// early, but not the observer.)
if (mAnimationConsumers > 0) {
mAnimationConsumers--;
if (GetImage())
GetImage()->DecrementAnimationConsumers();
nsRefPtr<Image> image = GetImage();
if (image)
image->DecrementAnimationConsumers();
}
return NS_OK;
}
@ -479,20 +486,24 @@ NS_IMETHODIMP imgRequestProxy::SetLoadFlags(nsLoadFlags flags)
/** imgIRequest methods **/
/* attribute imgIContainer image; */
NS_IMETHODIMP imgRequestProxy::GetImage(imgIContainer * *aImage)
NS_IMETHODIMP imgRequestProxy::GetImage(imgIContainer **aImage)
{
NS_ENSURE_TRUE(aImage, NS_ERROR_NULL_POINTER);
// It's possible that our owner has an image but hasn't notified us of it -
// that'll happen if we get Canceled before the owner instantiates its image
// (because Canceling unregisters us as a listener on mOwner). If we're
// in that situation, just grab the image off of mOwner.
imgIContainer* imageToReturn = GetImage();
nsRefPtr<Image> image = GetImage();
nsCOMPtr<imgIContainer> imageToReturn;
if (image)
imageToReturn = do_QueryInterface(image);
if (!imageToReturn && GetOwner())
imageToReturn = GetOwner()->mImage.get();
imageToReturn = GetOwner()->mImage;
if (!imageToReturn)
return NS_ERROR_FAILURE;
NS_ADDREF(*aImage = imageToReturn);
imageToReturn.swap(*aImage);
return NS_OK;
}
@ -557,7 +568,8 @@ imgRequestProxy* NewStaticProxy(imgRequestProxy* aThis)
{
nsCOMPtr<nsIPrincipal> currentPrincipal;
aThis->GetImagePrincipal(getter_AddRefs(currentPrincipal));
return new imgRequestProxyStatic(aThis->GetImage(), currentPrincipal);
nsRefPtr<Image> image = aThis->GetImage();
return new imgRequestProxyStatic(image, currentPrincipal);
}
NS_IMETHODIMP imgRequestProxy::Clone(imgINotificationObserver* aObserver,
@ -906,7 +918,7 @@ nsresult
imgRequestProxy::GetStaticRequest(imgRequestProxy** aReturn)
{
*aReturn = nullptr;
mozilla::image::Image* image = GetImage();
nsRefPtr<Image> image = GetImage();
bool animated;
if (!image || (NS_SUCCEEDED(image->GetAnimated(&animated)) && !animated)) {
@ -951,7 +963,7 @@ void imgRequestProxy::NotifyListener()
} else {
// We don't have an imgRequest, so we can only notify the clone of our
// current state, but we still have to do that asynchronously.
NS_ABORT_IF_FALSE(GetImage(),
NS_ABORT_IF_FALSE(HasImage(),
"if we have no imgRequest, we should have an Image");
statusTracker->NotifyCurrentState(this);
}
@ -973,7 +985,7 @@ imgRequestProxy::SetHasImage()
{
nsRefPtr<imgStatusTracker> statusTracker = GetStatusTracker();
MOZ_ASSERT(statusTracker);
Image* image = statusTracker->GetImage();
nsRefPtr<Image> image = statusTracker->GetImage();
MOZ_ASSERT(image);
// Force any private status related to the owner to reflect
@ -995,12 +1007,27 @@ imgRequestProxy::GetStatusTracker() const
return mBehaviour->GetStatusTracker();
}
mozilla::image::Image*
already_AddRefed<mozilla::image::Image>
imgRequestProxy::GetImage() const
{
return mBehaviour->GetImage();
}
bool
RequestBehaviour::HasImage() const
{
if (!mOwnerHasImage)
return false;
nsRefPtr<imgStatusTracker> statusTracker = GetStatusTracker();
return statusTracker ? statusTracker->HasImage() : false;
}
bool
imgRequestProxy::HasImage() const
{
return mBehaviour->HasImage();
}
imgRequest*
imgRequestProxy::GetOwner() const
{
@ -1014,11 +1041,17 @@ class StaticBehaviour : public ProxyBehaviour
public:
StaticBehaviour(mozilla::image::Image* aImage) : mImage(aImage) {}
virtual mozilla::image::Image* GetImage() const MOZ_OVERRIDE {
virtual already_AddRefed<mozilla::image::Image>
GetImage() const MOZ_OVERRIDE {
nsRefPtr<mozilla::image::Image> image = mImage;
return image.forget();
}
virtual bool HasImage() const MOZ_OVERRIDE {
return mImage;
}
virtual already_AddRefed<imgStatusTracker> GetStatusTracker() const MOZ_OVERRIDE {
virtual already_AddRefed<imgStatusTracker> GetStatusTracker() const MOZ_OVERRIDE {
return mImage->GetStatusTracker();
}

View File

@ -180,7 +180,8 @@ protected:
return GetOwner()->mTimedChannel;
}
mozilla::image::Image* GetImage() const;
already_AddRefed<mozilla::image::Image> GetImage() const;
bool HasImage() const;
imgRequest* GetOwner() const;
nsresult PerformClone(imgINotificationObserver* aObserver,

View File

@ -43,7 +43,7 @@ public:
MOZ_ASSERT(NS_IsMainThread(),
"Use imgStatusTracker::mConsumers on main thread only");
LOG_SCOPE(GetImgLog(), "imgStatusTrackerNotifyingObserver::OnStartDecode");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnStartDecode callback before we've created our image");
mTracker->RecordStartDecode();
@ -74,9 +74,12 @@ public:
"Use imgStatusTracker::mConsumers on main thread only");
LOG_SCOPE(GetImgLog(), "imgStatusTrackerNotifyingObserver::OnStartContainer");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnStartContainer callback before we've created our image");
mTracker->RecordStartContainer(mTracker->GetImage());
{
nsRefPtr<Image> image = mTracker->GetImage();
mTracker->RecordStartContainer(image);
}
nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(mTracker->mConsumers);
while (iter.HasMore()) {
@ -87,7 +90,7 @@ public:
virtual void OnStartFrame()
{
LOG_SCOPE(GetImgLog(), "imgStatusTrackerNotifyingObserver::OnStartFrame");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnStartFrame callback before we've created our image");
mTracker->RecordStartFrame();
@ -101,7 +104,7 @@ public:
MOZ_ASSERT(NS_IsMainThread(),
"Use imgStatusTracker::mConsumers on main thread only");
LOG_SCOPE(GetImgLog(), "imgStatusTrackerNotifyingObserver::FrameChanged");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"FrameChanged callback before we've created our image");
mTracker->RecordFrameChanged(dirtyRect);
@ -117,7 +120,7 @@ public:
MOZ_ASSERT(NS_IsMainThread(),
"Use imgStatusTracker::mConsumers on main thread only");
LOG_SCOPE(GetImgLog(), "imgStatusTrackerNotifyingObserver::OnStopFrame");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnStopFrame callback before we've created our image");
mTracker->RecordStopFrame();
@ -135,7 +138,7 @@ public:
MOZ_ASSERT(NS_IsMainThread(),
"Use imgStatusTracker::mConsumers on main thread only");
LOG_SCOPE(GetImgLog(), "imgStatusTrackerNotifyingObserver::OnStopDecode");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnStopDecode callback before we've created our image");
bool preexistingError = mTracker->GetImageStatus() == imgIRequest::STATUS_ERROR;
@ -165,7 +168,7 @@ public:
{
MOZ_ASSERT(NS_IsMainThread(),
"Use imgStatusTracker::mConsumers on main thread only");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnDiscard callback before we've created our image");
mTracker->RecordDiscard();
@ -180,7 +183,7 @@ public:
{
MOZ_ASSERT(NS_IsMainThread(),
"Use imgStatusTracker::mConsumers on main thread only");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnUnlockedDraw callback before we've created our image");
mTracker->RecordUnlockedDraw();
@ -194,7 +197,7 @@ public:
{
MOZ_ASSERT(NS_IsMainThread(),
"Use imgStatusTracker::mConsumers on main thread only");
NS_ABORT_IF_FALSE(mTracker->GetImage(),
NS_ABORT_IF_FALSE(mTracker->HasImage(),
"OnImageIsAnimated callback before we've created our image");
mTracker->RecordImageIsAnimated();
@ -315,7 +318,7 @@ public:
LOG_SCOPE(GetImgLog(), "imgStatusTrackerObserver::OnUnlockedDraw");
nsRefPtr<imgStatusTracker> tracker = mTracker.get();
if (!tracker) { return; }
NS_ABORT_IF_FALSE(tracker->GetImage(),
NS_ABORT_IF_FALSE(tracker->HasImage(),
"OnUnlockedDraw callback before we've created our image");
tracker->RecordUnlockedDraw();
}
@ -375,6 +378,26 @@ imgStatusTracker::imgStatusTracker(const imgStatusTracker& aOther)
imgStatusTracker::~imgStatusTracker()
{}
imgStatusTrackerInit::imgStatusTrackerInit(mozilla::image::Image* aImage,
imgStatusTracker* aTracker)
{
MOZ_ASSERT(aImage);
if (aTracker) {
mTracker = aTracker;
mTracker->SetImage(aImage);
} else {
mTracker = new imgStatusTracker(aImage);
}
aImage->SetStatusTracker(mTracker);
MOZ_ASSERT(mTracker);
}
imgStatusTrackerInit::~imgStatusTrackerInit()
{
mTracker->ResetImage();
}
void
imgStatusTracker::SetImage(Image* aImage)
{
@ -383,6 +406,13 @@ imgStatusTracker::SetImage(Image* aImage)
mImage = aImage;
}
void
imgStatusTracker::ResetImage()
{
NS_ABORT_IF_FALSE(mImage, "Resetting image when it's already null!");
mImage = nullptr;
}
bool
imgStatusTracker::IsLoading() const
{
@ -447,8 +477,8 @@ imgStatusTracker::Notify(imgRequestProxy* proxy)
{
MOZ_ASSERT(NS_IsMainThread(), "imgRequestProxy is not threadsafe");
#ifdef PR_LOGGING
if (GetImage() && GetImage()->GetURI()) {
nsRefPtr<ImageURL> uri(GetImage()->GetURI());
if (mImage && mImage->GetURI()) {
nsRefPtr<ImageURL> uri(mImage->GetURI());
nsAutoCString spec;
uri->GetSpec(spec);
LOG_FUNC_WITH_PARAM(GetImgLog(), "imgStatusTracker::Notify async", "uri", spec.get());
@ -1206,11 +1236,11 @@ imgStatusTracker::FireFailureNotification()
// Some kind of problem has happened with image decoding.
// Report the URI to net:failed-to-process-uri-conent observers.
if (GetImage()) {
if (mImage) {
// Should be on main thread, so ok to create a new nsIURI.
nsCOMPtr<nsIURI> uri;
{
nsRefPtr<ImageURL> threadsafeUriData = GetImage()->GetURI();
nsRefPtr<ImageURL> threadsafeUriData = mImage->GetURI();
uri = threadsafeUriData ? threadsafeUriData->ToIURI() : nullptr;
}
if (uri) {

View File

@ -124,6 +124,10 @@ public:
// without an image.
void SetImage(mozilla::image::Image* aImage);
// Image resetter, for when mImage is about to go out of scope. mImage is a
// weak reference, and thus must be set to null when it's object is deleted.
void ResetImage();
// Inform this status tracker that it is associated with a multipart image.
void SetIsMultipart() { mIsMultipart = true; }
@ -264,7 +268,11 @@ public:
bool IsMultipart() const { return mIsMultipart; }
// Weak pointer getters - no AddRefs.
inline mozilla::image::Image* GetImage() const { return mImage; }
inline already_AddRefed<mozilla::image::Image> GetImage() const {
nsRefPtr<mozilla::image::Image> image = mImage;
return image.forget();
}
inline bool HasImage() { return mImage; }
inline imgDecoderObserver* GetDecoderObserver() { return mTrackerObserver.get(); }
@ -292,6 +300,7 @@ private:
friend class imgRequestNotifyRunnable;
friend class imgStatusTrackerObserver;
friend class imgStatusTrackerNotifyingObserver;
friend class imgStatusTrackerInit;
imgStatusTracker(const imgStatusTracker& aOther);
// Main thread only because it deals with the observer service.
@ -309,7 +318,7 @@ private:
// frames are assumed to be fully valid.)
nsIntRect mInvalidRect;
// Weak pointer to the image. The image owns the status tracker.
// This weak ref should be set null when the image goes out of scope.
mozilla::image::Image* mImage;
// List of proxies attached to the image. Each proxy represents a consumer
@ -326,4 +335,14 @@ private:
bool mHasBeenDecoded : 1;
};
class imgStatusTrackerInit
{
public:
imgStatusTrackerInit(mozilla::image::Image* aImage,
imgStatusTracker* aTracker);
~imgStatusTrackerInit();
private:
imgStatusTracker* mTracker;
};
#endif