Bug 1125490 (Part 2) - Use an enumeration instead of a boolean to request discarding in nsIImageLoadingContent. r=tn

dom/base/nsIImageLoadingContent.idl

@@ -172,10 +172,15 @@ interface nsIImageLoadingContent : imgINotificationObserver
    * A visible count is stored, if it is non-zero then this image is considered
    * visible. These methods increment, decrement, or return the visible count.
    *
-   * @param aRequestDiscard Whether to attempt to discard the image if its
+   * @param aNonvisibleAction What to do if the image's visibility count is now
-   *                        visibility count is now zero.
+   *                          zero. If ON_NONVISIBLE_NO_ACTION, nothing will be
+   *                          done. If ON_NONVISIBLE_REQUEST_DISCARD, the image
+   *                          will be asked to discard its surfaces if possible.
    */
   [noscript, notxpcom] void IncrementVisibleCount();
-  [noscript, notxpcom] void DecrementVisibleCount(in boolean aRequestDiscard);
+  [noscript, notxpcom] void DecrementVisibleCount(in uint32_t aNonvisibleAction);
   [noscript, notxpcom] uint32_t GetVisibleCount();
+
+  const long ON_NONVISIBLE_NO_ACTION = 0;
+  const long ON_NONVISIBLE_REQUEST_DISCARD = 1;
 };

dom/base/nsImageLoadingContent.cpp

@@ -105,8 +105,8 @@ nsImageLoadingContent::DestroyImageLoadingContent()
 {
   // Cancel our requests so they won't hold stale refs to us
   // NB: Don't ask to discard the images here.
-  ClearCurrentRequest(NS_BINDING_ABORTED, 0);
+  ClearCurrentRequest(NS_BINDING_ABORTED, ON_NONVISIBLE_NO_ACTION);
-  ClearPendingRequest(NS_BINDING_ABORTED, 0);
+  ClearPendingRequest(NS_BINDING_ABORTED, ON_NONVISIBLE_NO_ACTION);
 }
 
 nsImageLoadingContent::~nsImageLoadingContent()
@@ -554,7 +554,7 @@ nsImageLoadingContent::FrameDestroyed(nsIFrame* aFrame)
   if (aFrame->HasAnyStateBits(NS_FRAME_IN_POPUP)) {
     // We assume all images in popups are visible, so this decrement balances
     // out the increment in FrameCreated above.
-    DecrementVisibleCount(/* aRequestDiscard = */ false);
+    DecrementVisibleCount(ON_NONVISIBLE_NO_ACTION);
   }
 }
 
@@ -777,15 +777,14 @@ nsImageLoadingContent::IncrementVisibleCount()
 }
 
 void
-nsImageLoadingContent::DecrementVisibleCount(bool aRequestDiscard)
+nsImageLoadingContent::DecrementVisibleCount(uint32_t aNonvisibleAction)
 {
   NS_ASSERTION(mVisibleCount > 0, "visible count should be positive here");
   mVisibleCount--;
 
   if (mVisibleCount == 0) {
-    uint32_t flags = aRequestDiscard ? REQUEST_DISCARD : 0;
+    UntrackImage(mCurrentRequest, aNonvisibleAction);
-    UntrackImage(mCurrentRequest, flags);
+    UntrackImage(mPendingRequest, aNonvisibleAction);
-    UntrackImage(mPendingRequest, flags);
   }
 }
 
@@ -1097,8 +1096,8 @@ void
 nsImageLoadingContent::CancelImageRequests(bool aNotify)
 {
   AutoStateChanger changer(this, aNotify);
-  ClearPendingRequest(NS_BINDING_ABORTED, REQUEST_DISCARD);
+  ClearPendingRequest(NS_BINDING_ABORTED, ON_NONVISIBLE_REQUEST_DISCARD);
-  ClearCurrentRequest(NS_BINDING_ABORTED, REQUEST_DISCARD);
+  ClearCurrentRequest(NS_BINDING_ABORTED, ON_NONVISIBLE_REQUEST_DISCARD);
 }
 
 nsresult
@@ -1110,8 +1109,8 @@ nsImageLoadingContent::UseAsPrimaryRequest(imgRequestProxy* aRequest,
   AutoStateChanger changer(this, aNotify);
 
   // Get rid if our existing images
-  ClearPendingRequest(NS_BINDING_ABORTED, REQUEST_DISCARD);
+  ClearPendingRequest(NS_BINDING_ABORTED, ON_NONVISIBLE_REQUEST_DISCARD);
-  ClearCurrentRequest(NS_BINDING_ABORTED, REQUEST_DISCARD);
+  ClearCurrentRequest(NS_BINDING_ABORTED, ON_NONVISIBLE_REQUEST_DISCARD);
 
   // Clone the request we were given.
   nsRefPtr<imgRequestProxy>& req = PrepareNextRequest(aImageLoadType);
@@ -1228,7 +1227,7 @@ nsImageLoadingContent::SetBlockedRequest(nsIURI* aURI, int16_t aContentDecision)
   // reason "image source changed". However, apparently there's some abuse
   // over in nsImageFrame where the displaying of the "broken" icon for the
   // next image depends on the cancel reason of the previous image. ugh.
-  ClearPendingRequest(NS_ERROR_IMAGE_BLOCKED, REQUEST_DISCARD);
+  ClearPendingRequest(NS_ERROR_IMAGE_BLOCKED, ON_NONVISIBLE_REQUEST_DISCARD);
 
   // For the blocked case, we only want to cancel the existing current request
   // if size is not available. bz says the web depends on this behavior.
@@ -1236,7 +1235,7 @@ nsImageLoadingContent::SetBlockedRequest(nsIURI* aURI, int16_t aContentDecision)
 
     mImageBlockingStatus = aContentDecision;
     uint32_t keepFlags = mCurrentRequestFlags & REQUEST_IS_IMAGESET;
-    ClearCurrentRequest(NS_ERROR_IMAGE_BLOCKED, REQUEST_DISCARD);
+    ClearCurrentRequest(NS_ERROR_IMAGE_BLOCKED, ON_NONVISIBLE_REQUEST_DISCARD);
 
     // We still want to remember what URI we were and if it was an imageset,
     // despite not having an actual request. These are both cleared as part of
@@ -1254,7 +1253,8 @@ nsImageLoadingContent::PrepareCurrentRequest(ImageLoadType aImageLoadType)
   mImageBlockingStatus = nsIContentPolicy::ACCEPT;
 
   // Get rid of anything that was there previously.
-  ClearCurrentRequest(NS_ERROR_IMAGE_SRC_CHANGED, REQUEST_DISCARD);
+  ClearCurrentRequest(NS_ERROR_IMAGE_SRC_CHANGED,
+                      ON_NONVISIBLE_REQUEST_DISCARD);
 
   if (mNewRequestsWillNeedAnimationReset) {
     mCurrentRequestFlags |= REQUEST_NEEDS_ANIMATION_RESET;
@@ -1272,7 +1272,8 @@ nsRefPtr<imgRequestProxy>&
 nsImageLoadingContent::PreparePendingRequest(ImageLoadType aImageLoadType)
 {
   // Get rid of anything that was there previously.
-  ClearPendingRequest(NS_ERROR_IMAGE_SRC_CHANGED, REQUEST_DISCARD);
+  ClearPendingRequest(NS_ERROR_IMAGE_SRC_CHANGED,
+                      ON_NONVISIBLE_REQUEST_DISCARD);
 
   if (mNewRequestsWillNeedAnimationReset) {
     mPendingRequestFlags |= REQUEST_NEEDS_ANIMATION_RESET;
@@ -1337,7 +1338,7 @@ nsImageLoadingContent::MakePendingRequestCurrent()
 
 void
 nsImageLoadingContent::ClearCurrentRequest(nsresult aReason,
-                                           uint32_t aFlags)
+                                           uint32_t aNonvisibleAction)
 {
   if (!mCurrentRequest) {
     // Even if we didn't have a current request, we might have been keeping
@@ -1355,7 +1356,7 @@ nsImageLoadingContent::ClearCurrentRequest(nsresult aReason,
                                         &mCurrentRequestRegistered);
 
   // Clean up the request.
-  UntrackImage(mCurrentRequest, aFlags);
+  UntrackImage(mCurrentRequest, aNonvisibleAction);
   mCurrentRequest->CancelAndForgetObserver(aReason);
   mCurrentRequest = nullptr;
   mCurrentRequestFlags = 0;
@@ -1363,7 +1364,7 @@ nsImageLoadingContent::ClearCurrentRequest(nsresult aReason,
 
 void
 nsImageLoadingContent::ClearPendingRequest(nsresult aReason,
-                                           uint32_t aFlags)
+                                           uint32_t aNonvisibleAction)
 {
   if (!mPendingRequest)
     return;
@@ -1373,7 +1374,7 @@ nsImageLoadingContent::ClearPendingRequest(nsresult aReason,
   nsLayoutUtils::DeregisterImageRequest(GetFramePresContext(), mPendingRequest,
                                         &mPendingRequestRegistered);
 
-  UntrackImage(mPendingRequest, aFlags);
+  UntrackImage(mPendingRequest, aNonvisibleAction);
   mPendingRequest->CancelAndForgetObserver(aReason);
   mPendingRequest = nullptr;
   mPendingRequestFlags = 0;
@@ -1473,7 +1474,9 @@ nsImageLoadingContent::TrackImage(imgIRequest* aImage)
 }
 
 void
-nsImageLoadingContent::UntrackImage(imgIRequest* aImage, uint32_t aFlags /* = 0 */)
+nsImageLoadingContent::UntrackImage(imgIRequest* aImage,
+                                    uint32_t aNonvisibleAction
+                                      /* = ON_NONVISIBLE_NO_ACTION */)
 {
   if (!aImage)
     return;
@@ -1490,9 +1493,10 @@ nsImageLoadingContent::UntrackImage(imgIRequest* aImage, uint32_t aFlags /* = 0
     if (doc && (mCurrentRequestFlags & REQUEST_IS_TRACKED)) {
       mCurrentRequestFlags &= ~REQUEST_IS_TRACKED;
       doc->RemoveImage(mCurrentRequest,
-                       (aFlags & REQUEST_DISCARD) ? nsIDocument::REQUEST_DISCARD : 0);
+                       (aNonvisibleAction == ON_NONVISIBLE_REQUEST_DISCARD)
-    }
+                         ? nsIDocument::REQUEST_DISCARD
-    else if (aFlags & REQUEST_DISCARD) {
+                         : 0);
+    } else if (aNonvisibleAction == ON_NONVISIBLE_REQUEST_DISCARD) {
       // If we're not in the document we may still need to be discarded.
       aImage->RequestDiscard();
     }
@@ -1501,9 +1505,10 @@ nsImageLoadingContent::UntrackImage(imgIRequest* aImage, uint32_t aFlags /* = 0
     if (doc && (mPendingRequestFlags & REQUEST_IS_TRACKED)) {
       mPendingRequestFlags &= ~REQUEST_IS_TRACKED;
       doc->RemoveImage(mPendingRequest,
-                       (aFlags & REQUEST_DISCARD) ? nsIDocument::REQUEST_DISCARD : 0);
+                       (aNonvisibleAction == ON_NONVISIBLE_REQUEST_DISCARD)
-    }
+                         ? nsIDocument::REQUEST_DISCARD
-    else if (aFlags & REQUEST_DISCARD) {
+                         : 0);
+    } else if (aNonvisibleAction == ON_NONVISIBLE_REQUEST_DISCARD) {
       // If we're not in the document we may still need to be discarded.
       aImage->RequestDiscard();
     }

dom/base/nsImageLoadingContent.h

@@ -316,9 +316,12 @@ protected:
 
   /**
    * Cancels and nulls-out the "current" and "pending" requests if they exist.
+   * 
+   * @param aNonvisibleAction An action to take if the image is no longer
+   *                          visible as a result; see |UntrackImage|.
    */
-  void ClearCurrentRequest(nsresult aReason, uint32_t aFlags);
+  void ClearCurrentRequest(nsresult aReason, uint32_t aNonvisibleAction);
-  void ClearPendingRequest(nsresult aReason, uint32_t aFlags);
+  void ClearPendingRequest(nsresult aReason, uint32_t aNonvisibleAction);
 
   /**
    * Retrieve a pointer to the 'registered with the refresh driver' flag for
@@ -347,14 +350,14 @@ protected:
    *
    * No-op if aImage is null.
    *
-   * REQUEST_DISCARD passed to UntrackImage means we request the discard of the
+   * @param aNonvisibleAction What to do if the image's visibility count is now
-   * decoded data of the image.
+   *                          zero. If ON_NONVISIBLE_NO_ACTION, nothing will be
+   *                          done. If ON_NONVISIBLE_REQUEST_DISCARD, the image
+   *                          will be asked to discard its surfaces if possible.
    */
   void TrackImage(imgIRequest* aImage);
-  enum {
+  void UntrackImage(imgIRequest* aImage,
-    REQUEST_DISCARD = 0x1
+                    uint32_t aNonvisibleAction = ON_NONVISIBLE_NO_ACTION);
-  };
-  void UntrackImage(imgIRequest* aImage, uint32_t aFlags = 0);
 
   /* MEMBERS */
   nsRefPtr<imgRequestProxy> mCurrentRequest;

layout/base/nsPresShell.cpp

@@ -1153,7 +1153,7 @@ PresShell::Destroy()
 
   mUpdateImageVisibilityEvent.Revoke();
 
-  ClearVisibleImagesList(/* aRequestDiscard = */ true);
+  ClearVisibleImagesList(nsIImageLoadingContent::ON_NONVISIBLE_REQUEST_DISCARD);
 
   if (mCaret) {
     mCaret->Terminate();
@@ -5820,7 +5820,8 @@ PresShell::MarkImagesInListVisible(const nsDisplayList& aList)
 static PLDHashOperator
 DecrementVisibleCount(nsRefPtrHashKey<nsIImageLoadingContent>* aEntry, void*)
 {
-  aEntry->GetKey()->DecrementVisibleCount(/* aRequestDiscard = */ false);
+  aEntry->GetKey()->DecrementVisibleCount(
+    nsIImageLoadingContent::ON_NONVISIBLE_NO_ACTION);
   return PL_DHASH_NEXT;
 }
 
@@ -5844,7 +5845,8 @@ PresShell::ClearImageVisibilityVisited(nsView* aView, bool aClear)
   if (aClear) {
     PresShell* presShell = static_cast<PresShell*>(vm->GetPresShell());
     if (!presShell->mImageVisibilityVisited) {
-      presShell->ClearVisibleImagesList(/* aRequestDiscard = */ false);
+      presShell->ClearVisibleImagesList(
+        nsIImageLoadingContent::ON_NONVISIBLE_NO_ACTION);
     }
     presShell->mImageVisibilityVisited = false;
   }
@@ -5857,15 +5859,18 @@ static PLDHashOperator
 DecrementVisibleCountAndDiscard(nsRefPtrHashKey<nsIImageLoadingContent>* aEntry,
                                 void*)
 {
-  aEntry->GetKey()->DecrementVisibleCount(/* aRequestDiscard = */ true);
+  aEntry->GetKey()->DecrementVisibleCount(
+    nsIImageLoadingContent::ON_NONVISIBLE_REQUEST_DISCARD);
   return PL_DHASH_NEXT;
 }
 
 void
-PresShell::ClearVisibleImagesList(bool aRequestDiscard)
+PresShell::ClearVisibleImagesList(uint32_t aNonvisibleAction)
 {
-  auto enumerator = aRequestDiscard ? DecrementVisibleCountAndDiscard
+  auto enumerator
-                                    : DecrementVisibleCount;
+    = aNonvisibleAction == nsIImageLoadingContent::ON_NONVISIBLE_REQUEST_DISCARD
+    ? DecrementVisibleCountAndDiscard
+    : DecrementVisibleCount;
   mVisibleImages.EnumerateEntries(enumerator, nullptr);
   mVisibleImages.Clear();
 }
@@ -5996,7 +6001,8 @@ PresShell::UpdateImageVisibility()
   // call update on that frame
   nsIFrame* rootFrame = GetRootFrame();
   if (!rootFrame) {
-    ClearVisibleImagesList(/* aRequestDiscard = */ true);
+    ClearVisibleImagesList(
+      nsIImageLoadingContent::ON_NONVISIBLE_REQUEST_DISCARD);
     return;
   }
 
@@ -6155,7 +6161,8 @@ PresShell::RemoveImageFromVisibleList(nsIImageLoadingContent* aImage)
   mVisibleImages.RemoveEntry(aImage);
   if (mVisibleImages.Count() < count) {
     // aImage was in the hashtable, so we need to decrement its visible count
-    aImage->DecrementVisibleCount(/* aRequestDiscard = */ false);
+    aImage->DecrementVisibleCount(
+      nsIImageLoadingContent::ON_NONVISIBLE_NO_ACTION);
   }
 }
 

layout/base/nsPresShell.h

@@ -731,7 +731,7 @@ protected:
 
   nsRevocableEventPtr<nsRunnableMethod<PresShell> > mUpdateImageVisibilityEvent;
 
-  void ClearVisibleImagesList(bool aRequestDiscard);
+  void ClearVisibleImagesList(uint32_t aNonvisibleAction);
   static void ClearImageVisibilityVisited(nsView* aView, bool aClear);
   static void MarkImagesInListVisible(const nsDisplayList& aList);
   void MarkImagesInSubtreeVisible(nsIFrame* aFrame, const nsRect& aRect);

layout/svg/SVGFEImageFrame.cpp

@@ -82,7 +82,8 @@ SVGFEImageFrame::DestroyFrom(nsIFrame* aDestructRoot)
 
   if (imageLoader) {
     imageLoader->FrameDestroyed(this);
-    imageLoader->DecrementVisibleCount(/* aRequestDiscard = */ false);
+    imageLoader
+      ->DecrementVisibleCount(nsIImageLoadingContent::ON_NONVISIBLE_NO_ACTION);
   }
 
   SVGFEImageFrameBase::DestroyFrom(aDestructRoot);