Bug 1112972 - Part 1: Minor refactoring to prepare for MultipartImage. r=tn, a=sledru
authorSeth Fowler <seth@mozilla.com>
Wed, 07 Jan 2015 01:37:20 -0800
changeset 243671 28f4806f60ee
parent 243670 842d25881e21
child 243672 438fed84c7c3
push id4432
push userryanvm@gmail.com
push date2015-02-04 15:12 +0000
treeherdermozilla-beta@7422906b1a32 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn, sledru
bugs1112972
milestone36.0
Bug 1112972 - Part 1: Minor refactoring to prepare for MultipartImage. r=tn, a=sledru
image/src/ImageWrapper.h
image/src/ProgressTracker.cpp
image/src/ProgressTracker.h
image/src/imgRequest.cpp
--- a/image/src/ImageWrapper.h
+++ b/image/src/ImageWrapper.h
@@ -56,25 +56,31 @@ public:
   virtual void SetHasError() MOZ_OVERRIDE;
 
   virtual ImageURL* GetURI() MOZ_OVERRIDE;
 
 protected:
   explicit ImageWrapper(Image* aInnerImage)
     : mInnerImage(aInnerImage)
   {
-    NS_ABORT_IF_FALSE(aInnerImage, "Cannot wrap a null image");
+    MOZ_ASSERT(aInnerImage, "Need an image to wrap");
   }
 
   virtual ~ImageWrapper() { }
 
   /**
    * Returns a weak reference to the inner image wrapped by this ImageWrapper.
    */
-  Image* InnerImage() { return mInnerImage.get(); }
+  Image* InnerImage() const { return mInnerImage.get(); }
+
+  void SetInnerImage(Image* aInnerImage)
+  {
+    MOZ_ASSERT(aInnerImage, "Need an image to wrap");
+    mInnerImage = aInnerImage;
+  }
 
 private:
   nsRefPtr<Image> mInnerImage;
 };
 
 } // namespace image
 } // namespace mozilla
 
--- a/image/src/ProgressTracker.cpp
+++ b/image/src/ProgressTracker.cpp
@@ -24,20 +24,20 @@ namespace image {
 
 ProgressTrackerInit::ProgressTrackerInit(Image* aImage,
                                          ProgressTracker* aTracker)
 {
   MOZ_ASSERT(aImage);
 
   if (aTracker) {
     mTracker = aTracker;
-    mTracker->SetImage(aImage);
   } else {
-    mTracker = new ProgressTracker(aImage);
+    mTracker = new ProgressTracker();
   }
+  mTracker->SetImage(aImage);
   aImage->SetProgressTracker(mTracker);
   MOZ_ASSERT(mTracker);
 }
 
 ProgressTrackerInit::~ProgressTrackerInit()
 {
   mTracker->ResetImage();
 }
@@ -420,18 +420,17 @@ ProgressTracker::SyncNotify(IProgressObs
   }
 
   ObserverArray array;
   array.AppendElement(aObserver);
   SyncNotifyInternal(array, !!mImage, mProgress, r);
 }
 
 void
-ProgressTracker::EmulateRequestFinished(IProgressObserver* aObserver,
-                                        nsresult aStatus)
+ProgressTracker::EmulateRequestFinished(IProgressObserver* aObserver)
 {
   MOZ_ASSERT(NS_IsMainThread(),
              "SyncNotifyState and mObservers are not threadsafe");
   nsRefPtr<IProgressObserver> kungFuDeathGrip(aObserver);
 
   if (mProgress & FLAG_ONLOAD_BLOCKED && !(mProgress & FLAG_ONLOAD_UNBLOCKED)) {
     aObserver->UnblockOnload();
   }
@@ -443,29 +442,28 @@ ProgressTracker::EmulateRequestFinished(
 
 void
 ProgressTracker::AddObserver(IProgressObserver* aObserver)
 {
   MOZ_ASSERT(NS_IsMainThread());
   mObservers.AppendElementUnlessExists(aObserver);
 }
 
-// XXX - The last argument should go away.
 bool
-ProgressTracker::RemoveObserver(IProgressObserver* aObserver, nsresult aStatus)
+ProgressTracker::RemoveObserver(IProgressObserver* aObserver)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // Remove the observer from the list.
   bool removed = mObservers.RemoveElement(aObserver);
 
   // Observers can get confused if they don't get all the proper teardown
   // notifications. Part ways on good terms.
   if (removed && !aObserver->NotificationsDeferred()) {
-    EmulateRequestFinished(aObserver, aStatus);
+    EmulateRequestFinished(aObserver);
   }
 
   // Make sure we don't give callbacks to an observer that isn't interested in
   // them any more.
   AsyncNotifyRunnable* runnable =
     static_cast<AsyncNotifyRunnable*>(mRunnable.get());
 
   if (aObserver->NotificationsDeferred() && runnable) {
--- a/image/src/ProgressTracker.h
+++ b/image/src/ProgressTracker.h
@@ -72,21 +72,18 @@ inline Progress LoadCompleteProgress(boo
 class ProgressTracker : public mozilla::SupportsWeakPtr<ProgressTracker>
 {
   virtual ~ProgressTracker() { }
 
 public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(ProgressTracker)
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ProgressTracker)
 
-  // aImage is the image that will be passed to the observers in SyncNotify()
-  // and EmulateRequestFinished(), and must be alive as long as this instance
-  // is, because we hold a weak reference to it.
-  explicit ProgressTracker(Image* aImage)
-    : mImage(aImage)
+  ProgressTracker()
+    : mImage(nullptr)
     , mProgress(NoProgress)
   { }
 
   bool HasImage() const { return mImage; }
   already_AddRefed<Image> GetImage() const
   {
     nsRefPtr<Image> image = mImage;
     return image.forget();
@@ -154,17 +151,17 @@ public:
   // Because this may result in recursive notifications, no decoding locks may
   // be held.  Called on the main thread only.
   void SyncNotifyProgress(Progress aProgress,
                           const nsIntRect& aInvalidRect = nsIntRect());
 
   // We manage a set of observers that are using an image and thus concerned
   // with its loading progress. Weak pointers.
   void AddObserver(IProgressObserver* aObserver);
-  bool RemoveObserver(IProgressObserver* aObserver, nsresult aStatus);
+  bool RemoveObserver(IProgressObserver* aObserver);
   size_t ObserverCount() const {
     MOZ_ASSERT(NS_IsMainThread(), "Use mObservers on main thread only");
     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.
@@ -190,17 +187,17 @@ private:
 
   // 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, nsresult aStatus);
+  void EmulateRequestFinished(IProgressObserver* aObserver);
 
   // Main thread only because it deals with the observer service.
   void FireFailureNotification();
 
   // Main thread only, since notifications are expected on the main thread, and
   // mObservers is not threadsafe.
   static void SyncNotifyInternal(ObserverArray& aObservers,
                                  bool aHasImage, Progress aProgress,
--- a/image/src/imgRequest.cpp
+++ b/image/src/imgRequest.cpp
@@ -57,17 +57,17 @@ NS_IMPL_ISUPPORTS(imgRequest,
                   nsIStreamListener, nsIRequestObserver,
                   nsIThreadRetargetableStreamListener,
                   nsIChannelEventSink,
                   nsIInterfaceRequestor,
                   nsIAsyncVerifyRedirectCallback)
 
 imgRequest::imgRequest(imgLoader* aLoader)
  : mLoader(aLoader)
- , mProgressTracker(new ProgressTracker(nullptr))
+ , mProgressTracker(new ProgressTracker())
  , mValidator(nullptr)
  , mInnerWindowId(0)
  , mCORSMode(imgIRequest::CORS_NONE)
  , mReferrerPolicy(mozilla::net::RP_Default)
  , mImageErrorCode(NS_OK)
  , mDecodeRequested(false)
  , mIsMultiPartChannel(false)
  , mGotData(false)
@@ -199,17 +199,17 @@ nsresult imgRequest::RemoveProxy(imgRequ
   // have animation consumers.
   proxy->ClearAnimationConsumers();
 
   // Let the status tracker do its thing before we potentially call Cancel()
   // below, because Cancel() may result in OnStopRequest being called back
   // before Cancel() returns, leaving the image in a different state then the
   // one it was in at this point.
   nsRefPtr<ProgressTracker> progressTracker = GetProgressTracker();
-  if (!progressTracker->RemoveObserver(proxy, aStatus))
+  if (!progressTracker->RemoveObserver(proxy))
     return NS_OK;
 
   if (progressTracker->ObserverCount() == 0) {
     // If we have no observers, there's nothing holding us alive. If we haven't
     // been cancelled and thus removed from the cache, tell the image loader so
     // we can be evicted from the cache.
     if (mCacheEntry) {
       NS_ABORT_IF_FALSE(mURI, "Removing last observer without key uri.");
@@ -892,17 +892,17 @@ imgRequest::OnDataAvailable(nsIRequest *
 
       // If we've resniffed our MIME type and it changed, we need to create a
       // new status tracker to give to the image, because we don't have one of
       // our own any more.
       if (resniffMimeType) {
         MOZ_ASSERT(mIsMultiPartChannel, "Resniffing a non-multipart image");
 
         // Initialize a new status tracker.
-        nsRefPtr<ProgressTracker> freshTracker = new ProgressTracker(nullptr);
+        nsRefPtr<ProgressTracker> freshTracker = new ProgressTracker();
         freshTracker->SetIsMultipart();
 
         // Replace the old status tracker with it.
         nsRefPtr<ProgressTracker> oldProgressTracker = GetProgressTracker();
         freshTracker->AdoptObservers(oldProgressTracker);
         mProgressTracker = freshTracker.forget();
       }