Bug 1125490 (Part 1) - Make sure we request discarding for images in PresShell::Destroy. r=tn

dom/base/nsIImageLoadingContent.idl

@@ -37,7 +37,7 @@ interface nsIFrame;
  * interface to mirror this interface when changing it.
  */
 
-[scriptable, builtinclass, uuid(ce098f6c-baca-4178-a9aa-266e8bfe509b)]
+[scriptable, builtinclass, uuid(5794d12b-3195-4526-a814-a2181f6c71fe)]
 interface nsIImageLoadingContent : imgINotificationObserver
 {
   /**
@@ -170,9 +170,12 @@ 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 coount.
+   * visible. These methods increment, decrement, or return the visible count.
+   *
+   * @param aRequestDiscard Whether to attempt to discard the image if its
+   *                        visibility count is now zero.
    */
   [noscript, notxpcom] void IncrementVisibleCount();
-  [noscript, notxpcom] void DecrementVisibleCount();
+  [noscript, notxpcom] void DecrementVisibleCount(in boolean aRequestDiscard);
   [noscript, notxpcom] uint32_t GetVisibleCount();
 };

dom/base/nsImageLoadingContent.cpp

@@ -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();
+    DecrementVisibleCount(/* aRequestDiscard = */ false);
   }
 }
 
@@ -777,14 +777,15 @@ nsImageLoadingContent::IncrementVisibleCount()
 }
 
 void
-nsImageLoadingContent::DecrementVisibleCount()
+nsImageLoadingContent::DecrementVisibleCount(bool aRequestDiscard)
 {
   NS_ASSERTION(mVisibleCount > 0, "visible count should be positive here");
   mVisibleCount--;
 
   if (mVisibleCount == 0) {
-    UntrackImage(mCurrentRequest);
+    uint32_t flags = aRequestDiscard ? REQUEST_DISCARD : 0;
-    UntrackImage(mPendingRequest);
+    UntrackImage(mCurrentRequest, flags);
+    UntrackImage(mPendingRequest, flags);
   }
 }
 

image/test/browser/browser_bug666317.js

@@ -27,18 +27,6 @@ function currentRequest() {
   return img.getRequest(Ci.nsIImageLoadingContent.CURRENT_REQUEST);
 }
 
-function attachDiscardObserver(result) {
-  // Create the discard observer.
-  let observer = new ImageDiscardObserver(result);
-  let scriptedObserver = Cc["@mozilla.org/image/tools;1"]
-                           .getService(Ci.imgITools)
-                           .createScriptedObserver(observer);
-
-  // Clone the current imgIRequest with our new observer.
-  let request = currentRequest();
-  return request.clone(scriptedObserver);
-}
-
 function isImgDecoded() {
   let request = currentRequest();
   return request.imageStatus & Ci.imgIRequest.STATUS_FRAME_COMPLETE ? true : false;
@@ -71,7 +59,16 @@ function test() {
 function step2() {
   // Attach a discard listener and create a place to hold the result.
   var result = { wasDiscarded: false };
-  var clonedRequest = attachDiscardObserver(result);
+
+  // Create the discard observer.
+  var observer = new ImageDiscardObserver(result);
+  var scriptedObserver = Cc["@mozilla.org/image/tools;1"]
+                           .getService(Ci.imgITools)
+                           .createScriptedObserver(observer);
+
+  // Clone the current imgIRequest with our new observer.
+  var request = currentRequest();
+  var clonedRequest = request.clone(scriptedObserver);
 
   // Check that the image is decoded.
   forceDecodeImg();

layout/base/nsPresShell.cpp

@@ -1153,7 +1153,7 @@ PresShell::Destroy()
 
   mUpdateImageVisibilityEvent.Revoke();
 
-  ClearVisibleImagesList();
+  ClearVisibleImagesList(/* aRequestDiscard = */ true);
 
   if (mCaret) {
     mCaret->Terminate();
@@ -5820,7 +5820,7 @@ PresShell::MarkImagesInListVisible(const nsDisplayList& aList)
 static PLDHashOperator
 DecrementVisibleCount(nsRefPtrHashKey<nsIImageLoadingContent>* aEntry, void*)
 {
-  aEntry->GetKey()->DecrementVisibleCount();
+  aEntry->GetKey()->DecrementVisibleCount(/* aRequestDiscard = */ false);
   return PL_DHASH_NEXT;
 }
 
@@ -5844,7 +5844,7 @@ PresShell::ClearImageVisibilityVisited(nsView* aView, bool aClear)
   if (aClear) {
     PresShell* presShell = static_cast<PresShell*>(vm->GetPresShell());
     if (!presShell->mImageVisibilityVisited) {
-      presShell->ClearVisibleImagesList();
+      presShell->ClearVisibleImagesList(/* aRequestDiscard = */ false);
     }
     presShell->mImageVisibilityVisited = false;
   }
@@ -5853,10 +5853,20 @@ PresShell::ClearImageVisibilityVisited(nsView* aView, bool aClear)
   }
 }
 
-void
+static PLDHashOperator
-PresShell::ClearVisibleImagesList()
+DecrementVisibleCountAndDiscard(nsRefPtrHashKey<nsIImageLoadingContent>* aEntry,
+                                void*)
 {
-  mVisibleImages.EnumerateEntries(DecrementVisibleCount, nullptr);
+  aEntry->GetKey()->DecrementVisibleCount(/* aRequestDiscard = */ true);
+  return PL_DHASH_NEXT;
+}
+
+void
+PresShell::ClearVisibleImagesList(bool aRequestDiscard)
+{
+  auto enumerator = aRequestDiscard ? DecrementVisibleCountAndDiscard
+                                    : DecrementVisibleCount;
+  mVisibleImages.EnumerateEntries(enumerator, nullptr);
   mVisibleImages.Clear();
 }
 
@@ -5986,7 +5996,7 @@ PresShell::UpdateImageVisibility()
   // call update on that frame
   nsIFrame* rootFrame = GetRootFrame();
   if (!rootFrame) {
-    ClearVisibleImagesList();
+    ClearVisibleImagesList(/* aRequestDiscard = */ true);
     return;
   }
 
@@ -6145,7 +6155,7 @@ 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();
+    aImage->DecrementVisibleCount(/* aRequestDiscard = */ false);
   }
 }
 

layout/base/nsPresShell.h

@@ -731,7 +731,7 @@ protected:
 
   nsRevocableEventPtr<nsRunnableMethod<PresShell> > mUpdateImageVisibilityEvent;
 
-  void ClearVisibleImagesList();
+  void ClearVisibleImagesList(bool aRequestDiscard);
   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,7 @@ SVGFEImageFrame::DestroyFrom(nsIFrame* aDestructRoot)
 
   if (imageLoader) {
     imageLoader->FrameDestroyed(this);
-    imageLoader->DecrementVisibleCount();
+    imageLoader->DecrementVisibleCount(/* aRequestDiscard = */ false);
   }
 
   SVGFEImageFrameBase::DestroyFrom(aDestructRoot);