Bug 505385 - Part 8: Remove image reference from proxies, and delegate operations on images to the owning request or the status tracker. r=joe
authorJosh Matthews <josh@joshmatthews.net>
Fri, 12 Oct 2012 12:11:21 -0400
changeset 110232 8b08ff14b2460e3475c3a374f8d64b295df39bd0
parent 110231 ea55032b7fd937a45c15ca8eead9160d9d8f34da
child 110233 c401bda70ef9207e851c94d9cf87f2dd4a140166
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersjoe
bugs505385
milestone19.0a1
Bug 505385 - Part 8: Remove image reference from proxies, and delegate operations on images to the owning request or the status tracker. r=joe
image/src/imgRequest.cpp
image/src/imgRequestProxy.cpp
image/src/imgRequestProxy.h
--- a/image/src/imgRequest.cpp
+++ b/image/src/imgRequest.cpp
@@ -990,34 +990,34 @@ imgRequest::OnDataAvailable(nsIRequest *
       mContentType = newType;
 
       // 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 (mResniffMimeType) {
         NS_ABORT_IF_FALSE(mIsMultiPartChannel, "Resniffing a non-multipart image");
         imgStatusTracker* freshTracker = new imgStatusTracker(nullptr, this);
-        freshTracker->AdoptConsumers(mStatusTracker);
+        freshTracker->AdoptConsumers(&GetStatusTracker());
         mStatusTracker = freshTracker;
       }
 
       mResniffMimeType = false;
 
       /* now we have mimetype, so we can infer the image type that we want */
       if (mContentType.EqualsLiteral(SVG_MIMETYPE)) {
         mImage = new VectorImage(mStatusTracker.forget());
       } else {
         mImage = new RasterImage(mStatusTracker.forget());
       }
       mImage->SetInnerWindowID(mInnerWindowId);
 
       // Notify any imgRequestProxys that are observing us that we have an Image.
     nsTObserverArray<imgRequestProxy*>::ForwardIterator iter(GetStatusTracker().GetConsumers());
       while (iter.HasMore()) {
-        iter.GetNext()->SetImage(mImage);
+        iter.GetNext()->SetHasImage();
       }
 
       /* set our mimetype as a property */
       nsCOMPtr<nsISupportsCString> contentType(do_CreateInstance("@mozilla.org/supports-cstring;1"));
       if (contentType) {
         contentType->SetData(mContentType);
         mProperties->Set("type", contentType);
       }
--- a/image/src/imgRequestProxy.cpp
+++ b/image/src/imgRequestProxy.cpp
@@ -35,27 +35,27 @@ NS_INTERFACE_MAP_BEGIN(imgRequestProxy)
   NS_INTERFACE_MAP_ENTRY(nsISupportsPriority)
   NS_INTERFACE_MAP_ENTRY(nsISecurityInfoProvider)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsITimedChannel, TimedChannel() != nullptr)
 NS_INTERFACE_MAP_END
 
 imgRequestProxy::imgRequestProxy() :
   mOwner(nullptr),
   mURI(nullptr),
-  mImage(nullptr),
   mListener(nullptr),
   mLoadFlags(nsIRequest::LOAD_NORMAL),
   mLockCount(0),
   mAnimationConsumers(0),
   mCanceled(false),
   mIsInLoadGroup(false),
   mListenerIsStrongRef(false),
   mDecodeRequested(false),
   mDeferNotifications(false),
-  mSentStartContainer(false)
+  mSentStartContainer(false),
+  mOwnerHasImage(false)
 {
   /* member initializers and constructor code */
 
 }
 
 imgRequestProxy::~imgRequestProxy()
 {
   /* destructor code */
@@ -95,26 +95,27 @@ nsresult imgRequestProxy::Init(imgStatus
   NS_PRECONDITION(!mOwner && !mListener, "imgRequestProxy is already initialized");
 
   LOG_SCOPE_WITH_PARAM(gImgLog, "imgRequestProxy::Init", "request", aStatusTracker->GetRequest());
 
   NS_ABORT_IF_FALSE(mAnimationConsumers == 0, "Cannot have animation before Init");
 
   mStatusTracker = aStatusTracker;
   mOwner = aStatusTracker->GetRequest();
+  mOwnerHasImage = !!aStatusTracker->GetImage();
+  MOZ_ASSERT_IF(mOwner, mStatusTracker == &mOwner->GetStatusTracker());
   mListener = aObserver;
   // Make sure to addref mListener before the AddProxy call below, since
   // that call might well want to release it if the imgRequest has
   // already seen OnStopRequest.
   if (mListener) {
     mListenerIsStrongRef = true;
     NS_ADDREF(mListener);
   }
   mLoadGroup = aLoadGroup;
-  mImage = aStatusTracker->GetImage();
   mURI = aURI;
 
   // Note: AddProxy won't send all the On* notifications immediately
   if (mOwner)
     mOwner->AddProxy(this);
 
   return NS_OK;
 }
@@ -128,51 +129,48 @@ nsresult imgRequestProxy::ChangeOwner(im
   uint32_t oldLockCount = mLockCount;
   while (mLockCount)
     UnlockImage();
 
   // If we're holding animation requests, undo them.
   uint32_t oldAnimationConsumers = mAnimationConsumers;
   ClearAnimationConsumers();
 
-  // Even if we are cancelled, we MUST change our image, because the image
-  // holds our status, and the status must always be correct.
-  mImage = aNewOwner->mImage;
+  nsRefPtr<imgRequest> oldOwner = mOwner;
+  mOwner = aNewOwner;
+  mStatusTracker = &aNewOwner->GetStatusTracker();
 
   // If we were locked, apply the locks here
   for (uint32_t i = 0; i < oldLockCount; i++)
     LockImage();
 
   if (mCanceled) {
     // If we had animation requests, restore them before exiting
     // (otherwise we restore them later below)
     for (uint32_t i = 0; i < oldAnimationConsumers; i++)
       IncrementAnimationConsumers();
 
     return NS_OK;
   }
 
   // Were we decoded before?
   bool wasDecoded = false;
-  if (mImage &&
-      (mImage->GetStatusTracker().GetImageStatus() &
-       imgIRequest::STATUS_FRAME_COMPLETE)) {
+  if (GetImage() &&
+      (GetStatusTracker().GetImageStatus() & imgIRequest::STATUS_FRAME_COMPLETE)) {
     wasDecoded = true;
   }
 
-  mOwner->RemoveProxy(this, NS_IMAGELIB_CHANGING_OWNER);
+  oldOwner->RemoveProxy(this, NS_IMAGELIB_CHANGING_OWNER);
 
   // If we had animation requests, restore them here. Note that we
   // do this *after* RemoveProxy, which clears out animation consumers
   // (see bug 601723).
   for (uint32_t i = 0; i < oldAnimationConsumers; i++)
     IncrementAnimationConsumers();
 
-  mOwner = aNewOwner;
-
   mOwner->AddProxy(this);
 
   // If we were decoded, or if we'd previously requested a decode, request a
   // decode on the new image
   if (wasDecoded || mDecodeRequested)
     mOwner->StartDecoding();
 
   return NS_OK;
@@ -325,65 +323,64 @@ imgRequestProxy::RequestDecode()
 }
 
 
 /* void lockImage (); */
 NS_IMETHODIMP
 imgRequestProxy::LockImage()
 {
   mLockCount++;
-  if (mImage)
-    return mImage->LockImage();
+  if (GetImage())
+    return GetImage()->LockImage();
   return NS_OK;
 }
 
 /* void unlockImage (); */
 NS_IMETHODIMP
 imgRequestProxy::UnlockImage()
 {
   NS_ABORT_IF_FALSE(mLockCount > 0, "calling unlock but no locks!");
 
   mLockCount--;
-  if (mImage)
-    return mImage->UnlockImage();
+  if (GetImage())
+    return GetImage()->UnlockImage();
   return NS_OK;
 }
 
 /* void requestDiscard (); */
 NS_IMETHODIMP
 imgRequestProxy::RequestDiscard()
 {
-  if (mImage) {
-    return mImage->RequestDiscard();
-  }
+  if (GetImage())
+    return GetImage()->RequestDiscard();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 imgRequestProxy::IncrementAnimationConsumers()
 {
   mAnimationConsumers++;
-  if (mImage)
-    mImage->IncrementAnimationConsumers();
+  if (GetImage())
+    GetImage()->IncrementAnimationConsumers();
   return NS_OK;
 }
 
 NS_IMETHODIMP
 imgRequestProxy::DecrementAnimationConsumers()
 {
   // We may get here if some responsible code called Increment,
   // then called us, but we have meanwhile called ClearAnimationConsumers
   // because we needed to get rid of them earlier (see
   // imgRequest::RemoveProxy), and hence have nothing left to
   // decrement. (In such a case we got rid of the animation consumers
   // early, but not the observer.)
   if (mAnimationConsumers > 0) {
     mAnimationConsumers--;
-    if (mImage)
-      mImage->DecrementAnimationConsumers();
+    if (GetImage())
+      GetImage()->DecrementAnimationConsumers();
   }
   return NS_OK;
 }
 
 void
 imgRequestProxy::ClearAnimationConsumers()
 {
   while (mAnimationConsumers > 0)
@@ -430,17 +427,17 @@ NS_IMETHODIMP imgRequestProxy::SetLoadFl
 
 /* attribute imgIContainer image; */
 NS_IMETHODIMP imgRequestProxy::GetImage(imgIContainer * *aImage)
 {
   // 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 = mImage ? mImage : mOwner->mImage;
+  imgIContainer* imageToReturn = GetImage() ? GetImage() : mOwner->mImage.get();
 
   if (!imageToReturn)
     return NS_ERROR_FAILURE;
 
   NS_ADDREF(*aImage = imageToReturn);
 
   return NS_OK;
 }
@@ -819,43 +816,44 @@ void imgRequestProxy::NullOutListener()
     mListener = nullptr;
   }
 }
 
 NS_IMETHODIMP
 imgRequestProxy::GetStaticRequest(imgIRequest** aReturn)
 {
   *aReturn = nullptr;
+  mozilla::image::Image* image = GetImage();
 
   bool animated;
-  if (!mImage || (NS_SUCCEEDED(mImage->GetAnimated(&animated)) && !animated)) {
+  if (!image || (NS_SUCCEEDED(image->GetAnimated(&animated)) && !animated)) {
     // Early exit - we're not animated, so we don't have to do anything.
     NS_ADDREF(*aReturn = this);
     return NS_OK;
   }
 
   // We are animated. We need to extract the current frame from this image.
   int32_t w = 0;
   int32_t h = 0;
-  mImage->GetWidth(&w);
-  mImage->GetHeight(&h);
+  image->GetWidth(&w);
+  image->GetHeight(&h);
   nsIntRect rect(0, 0, w, h);
   nsCOMPtr<imgIContainer> currentFrame;
-  nsresult rv = mImage->ExtractFrame(imgIContainer::FRAME_CURRENT, rect,
-                                     imgIContainer::FLAG_SYNC_DECODE,
-                                     getter_AddRefs(currentFrame));
+  nsresult rv = image->ExtractFrame(imgIContainer::FRAME_CURRENT, rect,
+                                    imgIContainer::FLAG_SYNC_DECODE,
+                                    getter_AddRefs(currentFrame));
   if (NS_FAILED(rv))
     return rv;
 
   nsRefPtr<Image> frame = static_cast<Image*>(currentFrame.get());
 
   // Create a static imgRequestProxy with our new extracted frame.
   nsCOMPtr<nsIPrincipal> currentPrincipal;
   GetImagePrincipal(getter_AddRefs(currentPrincipal));
-  nsRefPtr<imgRequestProxy> req = new imgRequestProxyStatic(currentPrincipal);
+  nsRefPtr<imgRequestProxy> req = new imgRequestProxyStatic(frame, currentPrincipal);
   req->Init(&frame->GetStatusTracker(), nullptr, mURI, nullptr);
 
   NS_ADDREF(*aReturn = req);
 
   return NS_OK;
 }
 
 void imgRequestProxy::NotifyListener()
@@ -866,65 +864,77 @@ void imgRequestProxy::NotifyListener()
   // need to check mCanceled everywhere too.
 
   if (mOwner) {
     // Send the notifications to our listener asynchronously.
     GetStatusTracker().Notify(mOwner, this);
   } 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(mImage,
+    NS_ABORT_IF_FALSE(GetImage(),
                       "if we have no imgRequest, we should have an Image");
-    mImage->GetStatusTracker().NotifyCurrentState(this);
+    GetStatusTracker().NotifyCurrentState(this);
   }
 }
 
 void imgRequestProxy::SyncNotifyListener()
 {
   // It would be nice to notify the observer directly in the status tracker
   // instead of through the proxy, but there are several places we do extra
   // processing when we receive notifications (like OnStopRequest()), and we
   // need to check mCanceled everywhere too.
 
   GetStatusTracker().SyncNotify(this);
 }
 
 void
-imgRequestProxy::SetImage(Image* aImage)
+imgRequestProxy::SetHasImage()
 {
-  NS_ABORT_IF_FALSE(aImage,  "Setting null image");
-  NS_ABORT_IF_FALSE(!mImage || mOwner->GetMultipart(),
-                    "Setting image when we already have one");
+  Image* image = GetStatusTracker().GetImage();
 
-  mImage = aImage;
+  mOwnerHasImage = true;
 
   // Apply any locks we have
   for (uint32_t i = 0; i < mLockCount; ++i)
-    mImage->LockImage();
+    image->LockImage();
 
   // Apply any animation consumers we have
   for (uint32_t i = 0; i < mAnimationConsumers; i++)
-    mImage->IncrementAnimationConsumers();
+    image->IncrementAnimationConsumers();
 }
 
 imgStatusTracker&
-imgRequestProxy::GetStatusTracker()
+imgRequestProxy::GetStatusTracker() const
 {
   // NOTE: It's possible that our mOwner has an Image that it didn't notify
   // us about, if we were Canceled before its Image was constructed.
   // (Canceling removes us as an observer, so mOwner has no way to notify us).
   // That's why this method uses mOwner->GetStatusTracker() instead of just
   // mOwner->mStatusTracker -- we might have a null mImage and yet have an
   // mOwner with a non-null mImage (and a null mStatusTracker pointer).
-  return mImage ? mImage->GetStatusTracker() : mOwner->GetStatusTracker();
+  return *mStatusTracker;
+}
+
+mozilla::image::Image*
+imgRequestProxy::GetImage() const
+{
+  if (!mOwnerHasImage)
+    return nullptr;
+  return GetStatusTracker().GetImage();
 }
 
 ////////////////// imgRequestProxyStatic methods
 
 NS_IMETHODIMP imgRequestProxyStatic::GetImagePrincipal(nsIPrincipal **aPrincipal)
 {
   if (!mPrincipal)
     return NS_ERROR_FAILURE;
 
   NS_ADDREF(*aPrincipal = mPrincipal);
 
   return NS_OK;
 }
+
+mozilla::image::Image*
+imgRequestProxyStatic::GetImage() const
+{
+  return mImage;
+}
--- a/image/src/imgRequestProxy.h
+++ b/image/src/imgRequestProxy.h
@@ -88,19 +88,18 @@ public:
   {
     return mDeferNotifications;
   }
   void SetNotificationsDeferred(bool aDeferNotifications)
   {
     mDeferNotifications = aDeferNotifications;
   }
 
-  // Setter for our |mImage| pointer, for imgRequest to use, once it
-  // instantiates an Image.
-  void SetImage(mozilla::image::Image* aImage);
+  // XXXbholley - This eventually gets folded into the new notification API.
+  void SetHasImage();
 
   // Removes all animation consumers that were created with
   // IncrementAnimationConsumers. This is necessary since we need
   // to do it before the proxy itself is destroyed. See
   // imgRequest::RemoveProxy
   void ClearAnimationConsumers();
 
 protected:
@@ -164,25 +163,27 @@ protected:
   void DoRemoveFromLoadGroup() {
     RemoveFromLoadGroup(true);
   }
 
   // Return the imgStatusTracker associated with mOwner and/or mImage. It may
   // live either on mOwner or mImage, depending on whether
   //   (a) we have an mOwner at all
   //   (b) whether mOwner has instantiated its image yet
-  imgStatusTracker& GetStatusTracker();
+  imgStatusTracker& GetStatusTracker() const;
 
   nsITimedChannel* TimedChannel()
   {
     if (!mOwner)
       return nullptr;
     return mOwner->mTimedChannel;
   }
 
+  virtual mozilla::image::Image* GetImage() const;
+
 public:
   NS_FORWARD_SAFE_NSITIMEDCHANNEL(TimedChannel())
 
 private:
   friend class imgCacheValidator;
 
   // We maintain the following invariant:
   // The proxy is registered at most with a single imgRequest as an observer,
@@ -193,20 +194,16 @@ private:
   nsRefPtr<imgRequest> mOwner;
 
   // Weak pointer to the status tracker.
   imgStatusTracker* mStatusTracker;
 
   // The URI of our request.
   nsCOMPtr<nsIURI> mURI;
 
-  // The image we represent. Is null until data has been received, and is then
-  // set by imgRequest.
-  nsRefPtr<mozilla::image::Image> mImage;
-
   // mListener is only promised to be a weak ref (see imgILoader.idl),
   // but we actually keep a strong ref to it until we've seen our
   // first OnStopRequest.
   imgIDecoderObserver* mListener;
   nsCOMPtr<nsILoadGroup> mLoadGroup;
 
   nsLoadFlags mLoadFlags;
   uint32_t    mLockCount;
@@ -218,28 +215,44 @@ private:
 
   // Whether we want to defer our notifications by the non-virtual Observer
   // interfaces as image loads proceed.
   bool mDeferNotifications;
 
   // We only want to send OnStartContainer once for each proxy, but we might
   // get multiple OnStartContainer calls.
   bool mSentStartContainer;
+
+  protected:
+    bool mOwnerHasImage;
 };
 
 // Used for static image proxies for which no requests are available, so
 // certain behaviours must be overridden to compensate.
 class imgRequestProxyStatic : public imgRequestProxy
 {
 
 public:
-  imgRequestProxyStatic(nsIPrincipal* aPrincipal) : mPrincipal(aPrincipal) {};
+  imgRequestProxyStatic(mozilla::image::Image* aImage,
+                        nsIPrincipal* aPrincipal)
+                       : mImage(aImage)
+                       , mPrincipal(aPrincipal)
+  {
+    mOwnerHasImage = true;
+  };
 
   NS_IMETHOD GetImagePrincipal(nsIPrincipal** aPrincipal);
 
 protected:
+  // Our image. We have to hold a strong reference here, because that's normally
+  // the job of the underlying request.
+  nsRefPtr<mozilla::image::Image> mImage;
+
   // Our principal. We have to cache it, rather than accessing the underlying
   // request on-demand, because static proxies don't have an underlying request.
   nsCOMPtr<nsIPrincipal> mPrincipal;
 
+private:
+  mozilla::image::Image* GetImage() const MOZ_OVERRIDE;
+  using imgRequestProxy::GetImage;
 };
 
 #endif // imgRequestProxy_h__