Bug 1159409 - (Part 2) - Remove ProgressTrackerInit and register Images with a ProgressTracker in ImageFactory. r=tn
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 28 Apr 2015 16:32:02 -0700
changeset 273124 dfa398350b4a44bea8cfae8b52a4069d934b9d03
parent 273123 21735c9f4fae8e03b463ea1da85781b77546a6c4
child 273125 8e4ecd2f7a60c788135128352e3215acc85413f3
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1159409
milestone40.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1159409 - (Part 2) - Remove ProgressTrackerInit and register Images with a ProgressTracker in ImageFactory. r=tn
image/src/Image.cpp
image/src/Image.h
image/src/ImageFactory.cpp
image/src/ImageFactory.h
image/src/MultipartImage.cpp
image/src/MultipartImage.h
image/src/ProgressTracker.cpp
image/src/ProgressTracker.h
image/src/RasterImage.cpp
image/src/RasterImage.h
image/src/VectorImage.cpp
image/src/VectorImage.h
image/src/imgRequest.cpp
--- a/image/src/Image.cpp
+++ b/image/src/Image.cpp
@@ -58,16 +58,22 @@ ImageResource::ImageResource(ImageURL* a
   mInnerWindowId(0),
   mAnimationConsumers(0),
   mAnimationMode(kNormalAnimMode),
   mInitialized(false),
   mAnimating(false),
   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)
 {
   // By default we don't know
   eDecoderType rv = eDecoderType_unknown;
 
   // PNG
--- a/image/src/Image.h
+++ b/image/src/Image.h
@@ -284,16 +284,17 @@ public:
   /*
    * Returns a non-AddRefed pointer to the URI associated with this image.
    * Illegal to use off-main-thread.
    */
   virtual ImageURL* GetURI() override { return mURI.get(); }
 
 protected:
   explicit ImageResource(ImageURL* aURI);
+  ~ImageResource();
 
   // Shared functionality for implementors of imgIContainer. Every
   // implementation of attribute animationMode should forward here.
   nsresult GetAnimationModeInternal(uint16_t* aAnimationMode);
   nsresult SetAnimationModeInternal(uint16_t aAnimationMode);
 
   /**
    * Helper for RequestRefresh.
--- a/image/src/ImageFactory.cpp
+++ b/image/src/ImageFactory.cpp
@@ -9,16 +9,17 @@
 #include "mozilla/Likely.h"
 
 #include "nsIHttpChannel.h"
 #include "nsIFileChannel.h"
 #include "nsIFile.h"
 #include "nsMimeTypes.h"
 #include "nsIRequest.h"
 
+#include "MultipartImage.h"
 #include "RasterImage.h"
 #include "VectorImage.h"
 #include "Image.h"
 #include "nsMediaFragmentURIParser.h"
 #include "nsContentUtils.h"
 #include "nsIScriptSecurityManager.h"
 
 #include "ImageFactory.h"
@@ -144,22 +145,42 @@ BadImage(nsRefPtr<T>& image)
 
 /* static */ already_AddRefed<Image>
 ImageFactory::CreateAnonymousImage(const nsCString& aMimeType)
 {
   nsresult rv;
 
   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)
 {
   if (val > INT_MAX) {
     return INT_MAX;
   }
   if (val < INT_MIN) {
     return INT_MIN;
@@ -201,19 +222,23 @@ GetContentSize(nsIRequest* aRequest)
 /* static */ already_AddRefed<Image>
 ImageFactory::CreateRasterImage(nsIRequest* aRequest,
                                 ProgressTracker* aProgressTracker,
                                 const nsCString& aMimeType,
                                 ImageURL* aURI,
                                 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));
 
   newImage->SetInnerWindowID(aInnerWindowId);
 
   uint32_t len = GetContentSize(aRequest);
 
@@ -263,19 +288,23 @@ ImageFactory::CreateRasterImage(nsIReque
 /* static */ already_AddRefed<Image>
 ImageFactory::CreateVectorImage(nsIRequest* aRequest,
                                 ProgressTracker* aProgressTracker,
                                 const nsCString& aMimeType,
                                 ImageURL* aURI,
                                 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));
 
   newImage->SetInnerWindowID(aInnerWindowId);
 
   rv = newImage->OnStartRequest(aRequest, nullptr);
   NS_ENSURE_SUCCESS(rv, BadImage(newImage));
--- a/image/src/ImageFactory.h
+++ b/image/src/ImageFactory.h
@@ -13,16 +13,17 @@
 class nsCString;
 class nsIRequest;
 
 namespace mozilla {
 namespace image {
 
 class Image;
 class ImageURL;
+class MultipartImage;
 class ProgressTracker;
 
 class ImageFactory
 {
 public:
   /**
    * Registers vars with Preferences. Should only be called on the main thread.
    */
@@ -49,16 +50,28 @@ public:
    * Creates a new image which isn't associated with a URI or loaded through
    * the usual image loading mechanism.
    *
    * @param aMimeType      The mimetype of the image.
    */
   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>
   CreateRasterImage(nsIRequest* aRequest,
                     ProgressTracker* aProgressTracker,
                     const nsCString& aMimeType,
                     ImageURL* aURI,
                     uint32_t aImageFlags,
--- a/image/src/MultipartImage.cpp
+++ b/image/src/MultipartImage.cpp
@@ -98,33 +98,42 @@ private:
   nsRefPtr<Image> mImage;
 };
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // 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 =
     InnerImage()->GetProgressTracker();
   firstPartTracker->AddObserver(this);
   InnerImage()->RequestDecode();
   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)
 NS_IMPL_RELEASE(MultipartImage)
 
 void
 MultipartImage::BeginTransitionToPart(Image* aNextPart)
 {
--- a/image/src/MultipartImage.h
+++ b/image/src/MultipartImage.h
@@ -22,18 +22,16 @@ class NextPartObserver;
 class MultipartImage
   : public ImageWrapper
   , public IProgressObserver
 {
 public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(MultipartImage)
   NS_DECL_ISUPPORTS
 
-  MultipartImage(Image* aImage, ProgressTracker* aTracker);
-
   void BeginTransitionToPart(Image* aNextPart);
 
   // Overridden ImageWrapper methods:
   virtual already_AddRefed<imgIContainer> Unwrap() override;
   virtual already_AddRefed<ProgressTracker> GetProgressTracker() override;
   virtual void SetProgressTracker(ProgressTracker* aTracker) override;
   virtual nsresult OnImageDataAvailable(nsIRequest* aRequest,
                                         nsISupports* aContext,
@@ -68,22 +66,25 @@ public:
   // methods to do nothing.
   virtual void BlockOnload() override { }
   virtual void UnblockOnload() override { }
 
 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;
 };
 
 } // namespace image
 } // namespace mozilla
 
--- a/image/src/ProgressTracker.cpp
+++ b/image/src/ProgressTracker.cpp
@@ -17,36 +17,16 @@
 #include "mozilla/Assertions.h"
 #include "mozilla/Services.h"
 
 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)
 {
   // Check preconditions for every progress bit.
 
   if (aProgress & FLAG_SIZE_AVAILABLE) {
     // No preconditions.
   }
--- a/image/src/ProgressTracker.h
+++ b/image/src/ProgressTracker.h
@@ -153,32 +153,31 @@ public:
     return mObservers.Length();
   }
 
   // This is intentionally non-general because its sole purpose is to support
   // some obscure network priority logic in imgRequest. That stuff could
   // 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.
   void EmulateRequestFinished(IProgressObserver* aObserver);
 
   // Main thread only because it deals with the observer service.
   void FireFailureNotification();
 
@@ -196,21 +195,12 @@ private:
   // List of observers attached to the image. Each observer represents a
   // consumer using the image. Array and/or individual elements should only be
   // accessed on the main thread.
   ObserverArray mObservers;
 
   Progress mProgress;
 };
 
-class ProgressTrackerInit
-{
-public:
-  ProgressTrackerInit(Image* aImage, ProgressTracker* aTracker);
-  ~ProgressTrackerInit();
-private:
-  ProgressTracker* mTracker;
-};
-
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_src_ProgressTracker_h
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -245,18 +245,17 @@ static nsCOMPtr<nsIThread> sScaleWorkerT
 #ifndef DEBUG
 NS_IMPL_ISUPPORTS(RasterImage, imgIContainer, nsIProperties)
 #else
 NS_IMPL_ISUPPORTS(RasterImage, imgIContainer, nsIProperties,
                   imgIContainerDebug)
 #endif
 
 //******************************************************************************
-RasterImage::RasterImage(ProgressTracker* aProgressTracker,
-                         ImageURL* aURI /* = nullptr */) :
+RasterImage::RasterImage(ImageURL* aURI /* = nullptr */) :
   ImageResource(aURI), // invoke superclass's constructor
   mSize(0,0),
   mLockCount(0),
   mDecodeCount(0),
   mRequestedSampleSize(0),
   mLastImageContainerDrawResult(DrawResult::NOT_READY),
 #ifdef DEBUG
   mFramesNotified(0),
@@ -268,18 +267,16 @@ RasterImage::RasterImage(ProgressTracker
   mTransient(false),
   mDiscardable(false),
   mHasSourceData(false),
   mHasBeenDecoded(false),
   mPendingAnimation(false),
   mAnimationFinished(false),
   mWantFullDecode(false)
 {
-  mProgressTrackerInit = new ProgressTrackerInit(this, aProgressTracker);
-
   Telemetry::GetHistogramById(Telemetry::IMAGE_DECODE_COUNT)->Add(0);
 }
 
 //******************************************************************************
 RasterImage::~RasterImage()
 {
   // Make sure our SourceBuffer is marked as complete. This will ensure that any
   // outstanding decoders terminate.
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -419,19 +419,16 @@ private: // data
   bool                       mAnimationFinished:1;
 
   // Whether, once we are done doing a size decode, we should immediately kick
   // off a full decode.
   bool                       mWantFullDecode:1;
 
   TimeStamp mDrawStartTime;
 
-  // Initializes ProgressTracker and resets it on RasterImage destruction.
-  nsAutoPtr<ProgressTrackerInit> mProgressTrackerInit;
-
 
   //////////////////////////////////////////////////////////////////////////////
   // Scaling.
   //////////////////////////////////////////////////////////////////////////////
 
   // Initiates an HQ scale for the given frame, if possible.
   void RequestScale(imgFrame* aFrame, uint32_t aFlags, const nsIntSize& aSize);
 
@@ -469,18 +466,17 @@ private: // data
 
     nsRefPtr<RasterImage> mImage;
   };
 
   // Helpers
   bool CanDiscard();
 
 protected:
-  explicit RasterImage(ProgressTracker* aProgressTracker = nullptr,
-                       ImageURL* aURI = nullptr);
+  explicit RasterImage(ImageURL* aURI = nullptr);
 
   bool ShouldAnimate() override;
 
   friend class ImageFactory;
 };
 
 inline NS_IMETHODIMP
 RasterImage::GetAnimationMode(uint16_t* aAnimationMode) {
--- a/image/src/VectorImage.cpp
+++ b/image/src/VectorImage.cpp
@@ -322,28 +322,25 @@ SVGDrawingCallback::operator()(gfxContex
 NS_IMPL_ISUPPORTS(VectorImage,
                   imgIContainer,
                   nsIStreamListener,
                   nsIRequestObserver)
 
 //------------------------------------------------------------------------------
 // Constructor / Destructor
 
-VectorImage::VectorImage(ProgressTracker* aProgressTracker,
-                         ImageURL* aURI /* = nullptr */) :
+VectorImage::VectorImage(ImageURL* aURI /* = nullptr */) :
   ImageResource(aURI), // invoke superclass's constructor
   mLockCount(0),
   mIsInitialized(false),
   mIsFullyLoaded(false),
   mIsDrawing(false),
   mHaveAnimations(false),
   mHasPendingInvalidation(false)
-{
-  mProgressTrackerInit = new ProgressTrackerInit(this, aProgressTracker);
-}
+{ }
 
 VectorImage::~VectorImage()
 {
   CancelAllListeners();
   SurfaceCache::RemoveImage(ImageKey(this));
 }
 
 //------------------------------------------------------------------------------
--- a/image/src/VectorImage.h
+++ b/image/src/VectorImage.h
@@ -64,18 +64,17 @@ public:
   // Callback for SVGParseCompleteListener.
   void OnSVGDocumentParsed();
 
   // Callbacks for SVGLoadEventListener.
   void OnSVGDocumentLoaded();
   void OnSVGDocumentError();
 
 protected:
-  explicit VectorImage(ProgressTracker* aProgressTracker = nullptr,
-                       ImageURL* aURI = nullptr);
+  explicit VectorImage(ImageURL* aURI = nullptr);
   virtual ~VectorImage();
 
   virtual nsresult StartAnimation() override;
   virtual nsresult StopAnimation() override;
   virtual bool     ShouldAnimate() override;
 
   void CreateSurfaceAndShow(const SVGDrawingParameters& aParams);
   void Show(gfxDrawable* aDrawable, const SVGDrawingParameters& aParams);
@@ -106,19 +105,16 @@ private:
   bool           mIsFullyLoaded;          // Has the SVG document finished
                                           // loading?
   bool           mIsDrawing;              // Are we currently drawing?
   bool           mHaveAnimations;         // Is our SVG content SMIL-animated?
                                           // (Only set after mIsFullyLoaded.)
   bool           mHasPendingInvalidation; // Invalidate observers next refresh
                                           // driver tick.
 
-  // Initializes ProgressTracker and resets it on RasterImage destruction.
-  nsAutoPtr<ProgressTrackerInit> mProgressTrackerInit;
-
   friend class ImageFactory;
 };
 
 inline NS_IMETHODIMP VectorImage::GetAnimationMode(uint16_t* aAnimationMode) {
   return GetAnimationModeInternal(aAnimationMode);
 }
 
 inline NS_IMETHODIMP VectorImage::SetAnimationMode(uint16_t aAnimationMode) {
--- a/image/src/imgRequest.cpp
+++ b/image/src/imgRequest.cpp
@@ -972,17 +972,18 @@ PrepareForNewPart(nsIRequest* aRequest, 
     nsRefPtr<Image> partImage =
       ImageFactory::CreateImage(aRequest, progressTracker, result.mContentType,
                                 aURI, /* aIsMultipart = */ true,
                                 aInnerWindowId);
 
     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);
       multipartImage->BeginTransitionToPart(partImage);
     }
   } else {
     MOZ_ASSERT(!aExistingImage, "New part for non-multipart channel?");
     MOZ_ASSERT(aProgressTracker, "Shouldn't have given away tracker yet");