Bug 1159409 - (Part 2) - Remove ProgressTrackerInit and register Images with a ProgressTracker in ImageFactory. r=tn

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)

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.

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<RasterImage> newImage = new RasterImage();
 
+  nsRefPtr<ProgressTracker> 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<MultipartImage>
+ImageFactory::CreateMultipartImage(Image* aFirstPart,
+                                   ProgressTracker* aProgressTracker)
+{
+  MOZ_ASSERT(aFirstPart);
+  MOZ_ASSERT(aProgressTracker);
+
+  nsRefPtr<MultipartImage> 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<RasterImage> newImage = new RasterImage(aProgressTracker, aURI);
+  nsRefPtr<RasterImage> 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<VectorImage> newImage = new VectorImage(aProgressTracker, aURI);
+  nsRefPtr<VectorImage> newImage = new VectorImage(aURI);
+  aProgressTracker->SetImage(newImage);
+  newImage->SetProgressTracker(aProgressTracker);
 
   rv = newImage->Init(aMimeType.get(), aImageFlags);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));

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<Image>
   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<MultipartImage>
+  CreateMultipartImage(Image* aFirstPart, ProgressTracker* aProgressTracker);
+
 private:
   // Factory functions that create specific types of image containers.
   static already_AddRefed<Image>

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<ProgressTracker> 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)

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<ProgressTracker> mTracker;
-  nsAutoPtr<ProgressTrackerInit> mProgressTrackerInit;
   nsRefPtr<NextPartObserver> mNextPartObserver;
   nsRefPtr<Image> mNextPart;
   bool mDeferNotifications : 1;

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)
 {

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<mozilla::WeakPtr<IProgressObserver>> 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
 

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);
 }
 

image/src/RasterImage.h

@@ -424,9 +424,6 @@ private: // data
 
   TimeStamp mDrawStartTime;
 
-  // Initializes ProgressTracker and resets it on RasterImage destruction.
-  nsAutoPtr<ProgressTrackerInit> 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;
 

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()
 {

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<ProgressTrackerInit> mProgressTrackerInit;
-
   friend class ImageFactory;
 };
 

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<MultipartImage*>(aExistingImage);