diff --git a/image/src/Image.cpp b/image/src/Image.cpp index 5f19a8165f1..1f6c9a8cd52 100644 --- a/image/src/Image.cpp +++ b/image/src/Image.cpp @@ -63,6 +63,12 @@ ImageResource::ImageResource(ImageURL* aURI) : mError(false) { } +ImageResource::~ImageResource() +{ + // Ask our ProgressTracker to drop its weak reference to us. + mProgressTracker->ResetImage(); +} + // Translates a mimetype into a concrete decoder Image::eDecoderType Image::GetDecoderType(const char* aMimeType) diff --git a/image/src/Image.h b/image/src/Image.h index 6ee896a90e0..5d61a975314 100644 --- a/image/src/Image.h +++ b/image/src/Image.h @@ -289,6 +289,7 @@ public: protected: explicit ImageResource(ImageURL* aURI); + ~ImageResource(); // Shared functionality for implementors of imgIContainer. Every // implementation of attribute animationMode should forward here. diff --git a/image/src/ImageFactory.cpp b/image/src/ImageFactory.cpp index ea342bc2008..ddd1a3df25e 100644 --- a/image/src/ImageFactory.cpp +++ b/image/src/ImageFactory.cpp @@ -14,6 +14,7 @@ #include "nsMimeTypes.h" #include "nsIRequest.h" +#include "MultipartImage.h" #include "RasterImage.h" #include "VectorImage.h" #include "Image.h" @@ -149,12 +150,32 @@ ImageFactory::CreateAnonymousImage(const nsCString& aMimeType) nsRefPtr newImage = new RasterImage(); + nsRefPtr newTracker = new ProgressTracker(); + newTracker->SetImage(newImage); + newImage->SetProgressTracker(newTracker); + rv = newImage->Init(aMimeType.get(), Image::INIT_FLAG_NONE); NS_ENSURE_SUCCESS(rv, BadImage(newImage)); return newImage.forget(); } +/* static */ already_AddRefed +ImageFactory::CreateMultipartImage(Image* aFirstPart, + ProgressTracker* aProgressTracker) +{ + MOZ_ASSERT(aFirstPart); + MOZ_ASSERT(aProgressTracker); + + nsRefPtr newImage = new MultipartImage(aFirstPart); + aProgressTracker->SetImage(newImage); + newImage->SetProgressTracker(aProgressTracker); + + newImage->Init(); + + return newImage.forget(); +} + int32_t SaturateToInt32(int64_t val) { @@ -206,9 +227,13 @@ ImageFactory::CreateRasterImage(nsIRequest* aRequest, uint32_t aImageFlags, uint32_t aInnerWindowId) { + MOZ_ASSERT(aProgressTracker); + nsresult rv; - nsRefPtr newImage = new RasterImage(aProgressTracker, aURI); + nsRefPtr newImage = new RasterImage(aURI); + aProgressTracker->SetImage(newImage); + newImage->SetProgressTracker(aProgressTracker); rv = newImage->Init(aMimeType.get(), aImageFlags); NS_ENSURE_SUCCESS(rv, BadImage(newImage)); @@ -268,9 +293,13 @@ ImageFactory::CreateVectorImage(nsIRequest* aRequest, uint32_t aImageFlags, uint32_t aInnerWindowId) { + MOZ_ASSERT(aProgressTracker); + nsresult rv; - nsRefPtr newImage = new VectorImage(aProgressTracker, aURI); + nsRefPtr newImage = new VectorImage(aURI); + aProgressTracker->SetImage(newImage); + newImage->SetProgressTracker(aProgressTracker); rv = newImage->Init(aMimeType.get(), aImageFlags); NS_ENSURE_SUCCESS(rv, BadImage(newImage)); diff --git a/image/src/ImageFactory.h b/image/src/ImageFactory.h index 7ec187cf36c..c3367401727 100644 --- a/image/src/ImageFactory.h +++ b/image/src/ImageFactory.h @@ -18,6 +18,7 @@ namespace image { class Image; class ImageURL; +class MultipartImage; class ProgressTracker; class ImageFactory @@ -54,6 +55,18 @@ public: static already_AddRefed CreateAnonymousImage(const nsCString& aMimeType); + /** + * Creates a new multipart/x-mixed-replace image wrapper, and initializes it + * with the first part. Subsequent parts should be passed to the existing + * MultipartImage via MultipartImage::BeginTransitionToPart(). + * + * @param aFirstPart An image containing the first part of the multipart + * stream. + * @param aProgressTracker A progress tracker for the multipart image. + */ + static already_AddRefed + CreateMultipartImage(Image* aFirstPart, ProgressTracker* aProgressTracker); + private: // Factory functions that create specific types of image containers. static already_AddRefed diff --git a/image/src/MultipartImage.cpp b/image/src/MultipartImage.cpp index ddfb6b7d5ab..0a6b89d3d98 100644 --- a/image/src/MultipartImage.cpp +++ b/image/src/MultipartImage.cpp @@ -103,13 +103,18 @@ private: // Implementation /////////////////////////////////////////////////////////////////////////////// -MultipartImage::MultipartImage(Image* aImage, ProgressTracker* aTracker) - : ImageWrapper(aImage) +MultipartImage::MultipartImage(Image* aFirstPart) + : ImageWrapper(aFirstPart) , mDeferNotifications(false) { - MOZ_ASSERT(aTracker); - mProgressTrackerInit = new ProgressTrackerInit(this, aTracker); mNextPartObserver = new NextPartObserver(this); +} + +void +MultipartImage::Init() +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(mTracker, "Should've called SetProgressTracker() by now"); // Start observing the first part. nsRefPtr firstPartTracker = @@ -119,7 +124,11 @@ MultipartImage::MultipartImage(Image* aImage, ProgressTracker* aTracker) InnerImage()->IncrementAnimationConsumers(); } -MultipartImage::~MultipartImage() { } +MultipartImage::~MultipartImage() +{ + // Ask our ProgressTracker to drop its weak reference to us. + mTracker->ResetImage(); +} NS_IMPL_QUERY_INTERFACE_INHERITED0(MultipartImage, ImageWrapper) NS_IMPL_ADDREF(MultipartImage) diff --git a/image/src/MultipartImage.h b/image/src/MultipartImage.h index 9f990302a09..d3d3c6c9a4d 100644 --- a/image/src/MultipartImage.h +++ b/image/src/MultipartImage.h @@ -27,8 +27,6 @@ public: MOZ_DECLARE_REFCOUNTED_TYPENAME(MultipartImage) NS_DECL_ISUPPORTS - MultipartImage(Image* aImage, ProgressTracker* aTracker); - void BeginTransitionToPart(Image* aNextPart); // Overridden ImageWrapper methods: @@ -73,12 +71,15 @@ protected: virtual ~MultipartImage(); private: + friend class ImageFactory; friend class NextPartObserver; + explicit MultipartImage(Image* aFirstPart); + void Init(); + void FinishTransition(); nsRefPtr mTracker; - nsAutoPtr mProgressTrackerInit; nsRefPtr mNextPartObserver; nsRefPtr mNextPart; bool mDeferNotifications : 1; diff --git a/image/src/ProgressTracker.cpp b/image/src/ProgressTracker.cpp index bc74730c8da..3504eefbca1 100644 --- a/image/src/ProgressTracker.cpp +++ b/image/src/ProgressTracker.cpp @@ -22,26 +22,6 @@ using mozilla::WeakPtr; namespace mozilla { namespace image { -ProgressTrackerInit::ProgressTrackerInit(Image* aImage, - ProgressTracker* aTracker) -{ - MOZ_ASSERT(aImage); - - if (aTracker) { - mTracker = aTracker; - } else { - mTracker = new ProgressTracker(); - } - mTracker->SetImage(aImage); - aImage->SetProgressTracker(mTracker); - MOZ_ASSERT(mTracker); -} - -ProgressTrackerInit::~ProgressTrackerInit() -{ - mTracker->ResetImage(); -} - static void CheckProgressConsistency(Progress aProgress) { diff --git a/image/src/ProgressTracker.h b/image/src/ProgressTracker.h index a6128060a63..2b211c656f9 100644 --- a/image/src/ProgressTracker.h +++ b/image/src/ProgressTracker.h @@ -158,22 +158,21 @@ public: // probably be improved, but it's too scary to mess with at the moment. bool FirstObserverIs(IProgressObserver* aObserver); + // Resets our weak reference to our image. Image subclasses should call this + // in their destructor. + void ResetImage(); + private: typedef nsTObserverArray> ObserverArray; friend class AsyncNotifyRunnable; friend class AsyncNotifyCurrentStateRunnable; - friend class ProgressTrackerInit; + friend class ImageFactory; ProgressTracker(const ProgressTracker& aOther) = delete; - // This method should only be called once, and only on an ProgressTracker - // that was initialized without an image. ProgressTrackerInit automates this. + // Sets our weak reference to our image. Only ImageFactory should call this. void SetImage(Image* aImage); - // Resets our weak reference to our image, for when mImage is about to go out - // of scope. ProgressTrackerInit automates this. - void ResetImage(); - // Send some notifications that would be necessary to make |aObserver| believe // the request is finished downloading and decoding. We only send // FLAG_LOAD_COMPLETE and FLAG_ONLOAD_UNBLOCKED, and only if necessary. @@ -201,15 +200,6 @@ private: Progress mProgress; }; -class ProgressTrackerInit -{ -public: - ProgressTrackerInit(Image* aImage, ProgressTracker* aTracker); - ~ProgressTrackerInit(); -private: - ProgressTracker* mTracker; -}; - } // namespace image } // namespace mozilla diff --git a/image/src/RasterImage.cpp b/image/src/RasterImage.cpp index 34b428911bf..c001f489f2a 100644 --- a/image/src/RasterImage.cpp +++ b/image/src/RasterImage.cpp @@ -250,8 +250,7 @@ NS_IMPL_ISUPPORTS(RasterImage, imgIContainer, nsIProperties, #endif //****************************************************************************** -RasterImage::RasterImage(ProgressTracker* aProgressTracker, - ImageURL* aURI /* = nullptr */) : +RasterImage::RasterImage(ImageURL* aURI /* = nullptr */) : ImageResource(aURI), // invoke superclass's constructor mSize(0,0), mLockCount(0), @@ -273,8 +272,6 @@ RasterImage::RasterImage(ProgressTracker* aProgressTracker, mAnimationFinished(false), mWantFullDecode(false) { - mProgressTrackerInit = new ProgressTrackerInit(this, aProgressTracker); - Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(0); } diff --git a/image/src/RasterImage.h b/image/src/RasterImage.h index 30d6025ef36..0e9865d070e 100644 --- a/image/src/RasterImage.h +++ b/image/src/RasterImage.h @@ -424,9 +424,6 @@ private: // data TimeStamp mDrawStartTime; - // Initializes ProgressTracker and resets it on RasterImage destruction. - nsAutoPtr mProgressTrackerInit; - ////////////////////////////////////////////////////////////////////////////// // Scaling. @@ -474,8 +471,7 @@ private: // data bool CanDiscard(); protected: - explicit RasterImage(ProgressTracker* aProgressTracker = nullptr, - ImageURL* aURI = nullptr); + explicit RasterImage(ImageURL* aURI = nullptr); bool ShouldAnimate() override; diff --git a/image/src/VectorImage.cpp b/image/src/VectorImage.cpp index eb0550a4be6..bbaa25aa084 100644 --- a/image/src/VectorImage.cpp +++ b/image/src/VectorImage.cpp @@ -327,8 +327,7 @@ NS_IMPL_ISUPPORTS(VectorImage, //------------------------------------------------------------------------------ // Constructor / Destructor -VectorImage::VectorImage(ProgressTracker* aProgressTracker, - ImageURL* aURI /* = nullptr */) : +VectorImage::VectorImage(ImageURL* aURI /* = nullptr */) : ImageResource(aURI), // invoke superclass's constructor mLockCount(0), mIsInitialized(false), @@ -336,9 +335,7 @@ VectorImage::VectorImage(ProgressTracker* aProgressTracker, mIsDrawing(false), mHaveAnimations(false), mHasPendingInvalidation(false) -{ - mProgressTrackerInit = new ProgressTrackerInit(this, aProgressTracker); -} +{ } VectorImage::~VectorImage() { diff --git a/image/src/VectorImage.h b/image/src/VectorImage.h index 7f300427ace..b88c3de2901 100644 --- a/image/src/VectorImage.h +++ b/image/src/VectorImage.h @@ -69,8 +69,7 @@ public: void OnSVGDocumentError(); protected: - explicit VectorImage(ProgressTracker* aProgressTracker = nullptr, - ImageURL* aURI = nullptr); + explicit VectorImage(ImageURL* aURI = nullptr); virtual ~VectorImage(); virtual nsresult StartAnimation() override; @@ -111,9 +110,6 @@ private: bool mHasPendingInvalidation; // Invalidate observers next refresh // driver tick. - // Initializes ProgressTracker and resets it on RasterImage destruction. - nsAutoPtr mProgressTrackerInit; - friend class ImageFactory; }; diff --git a/image/src/imgRequest.cpp b/image/src/imgRequest.cpp index 8d749739f7c..52dd6dc6255 100644 --- a/image/src/imgRequest.cpp +++ b/image/src/imgRequest.cpp @@ -977,7 +977,8 @@ PrepareForNewPart(nsIRequest* aRequest, nsIInputStream* aInStr, uint32_t aCount, if (result.mIsFirstPart) { // First part for a multipart channel. Create the MultipartImage wrapper. MOZ_ASSERT(aProgressTracker, "Shouldn't have given away tracker yet"); - result.mImage = new MultipartImage(partImage, aProgressTracker); + result.mImage = + ImageFactory::CreateMultipartImage(partImage, aProgressTracker); } else { // Transition to the new part. auto multipartImage = static_cast(aExistingImage);