Bug 1125490 (Part 1) - Make sure we request discarding for images in PresShell::Destroy. r=tn
☠☠ backed out by 457883ac8776 ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Mon, 26 Jan 2015 22:53:21 -0800
changeset 225931 bbd77af91d820b079fc5f922a6942f68f34d117d
parent 225930 00f4c6c6f553dcb9becb6ef4acb445f1cd588685
child 225932 b44e10f8362384e88218432f52f7551d65307ce2
push id54703
push usermfowler@mozilla.com
push dateTue, 27 Jan 2015 06:53:33 +0000
treeherdermozilla-inbound@b44e10f83623 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1125490
milestone38.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 1125490 (Part 1) - Make sure we request discarding for images in PresShell::Destroy. r=tn
dom/base/nsIImageLoadingContent.idl
dom/base/nsImageLoadingContent.cpp
image/test/browser/browser_bug666317.js
layout/base/nsPresShell.cpp
layout/base/nsPresShell.h
layout/svg/SVGFEImageFrame.cpp
--- a/dom/base/nsIImageLoadingContent.idl
+++ b/dom/base/nsIImageLoadingContent.idl
@@ -32,17 +32,17 @@ interface nsIFrame;
  * missed.  We should NOT freeze this interface without considering
  * this issue.  (It could be that the image status on imgIRequest is
  * sufficient, when combined with the imageBlockingStatus information.)
  *
  * Please make sure to update the MozImageLoadingContent WebIDL
  * 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
 {
   /**
    * Request types.  Image loading content nodes attempt to do atomic
    * image changes when the image url is changed.  This means that
    * when the url changes the new image load will start, but the old
    * image will remain the "current" request until the new image is
    * fully loaded.  At that point, the old "current" request will be
@@ -165,14 +165,17 @@ interface nsIImageLoadingContent : imgIN
    * The intrinsic size and width of this content. May differ from actual image
    * size due to things like responsive image density.
    */
   readonly attribute unsigned long    naturalWidth;
   readonly attribute unsigned long    naturalHeight;
 
   /**
    * 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();
 };
--- a/dom/base/nsImageLoadingContent.cpp
+++ b/dom/base/nsImageLoadingContent.cpp
@@ -549,17 +549,17 @@ nsImageLoadingContent::FrameDestroyed(ns
   nsIPresShell* presShell = presContext ? presContext->GetPresShell() : nullptr;
   if (presShell) {
     presShell->RemoveImageFromVisibleList(this);
   }
 
   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);
   }
 }
 
 /* static */
 nsContentPolicyType
 nsImageLoadingContent::PolicyTypeForLoad(ImageLoadType aImageLoadType)
 {
   if (aImageLoadType == eImageLoadType_Imageset) {
@@ -772,24 +772,25 @@ nsImageLoadingContent::IncrementVisibleC
   mVisibleCount++;
   if (mVisibleCount == 1) {
     TrackImage(mCurrentRequest);
     TrackImage(mPendingRequest);
   }
 }
 
 void
-nsImageLoadingContent::DecrementVisibleCount()
+nsImageLoadingContent::DecrementVisibleCount(bool aRequestDiscard)
 {
   NS_ASSERTION(mVisibleCount > 0, "visible count should be positive here");
   mVisibleCount--;
 
   if (mVisibleCount == 0) {
-    UntrackImage(mCurrentRequest);
-    UntrackImage(mPendingRequest);
+    uint32_t flags = aRequestDiscard ? REQUEST_DISCARD : 0;
+    UntrackImage(mCurrentRequest, flags);
+    UntrackImage(mPendingRequest, flags);
   }
 }
 
 uint32_t
 nsImageLoadingContent::GetVisibleCount()
 {
   return mVisibleCount;
 }
--- a/image/test/browser/browser_bug666317.js
+++ b/image/test/browser/browser_bug666317.js
@@ -22,28 +22,16 @@ function ImageDiscardObserver(result) {
 
 function currentRequest() {
   let img = gBrowser.getBrowserForTab(newTab).contentWindow
             .document.getElementById('testImg');
   img.QueryInterface(Ci.nsIImageLoadingContent);
   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;
 }
 
 // Ensure that the image is decoded by drawing it to a canvas.
 function forceDecodeImg() {
   let doc = gBrowser.getBrowserForTab(newTab).contentWindow.document;
@@ -66,17 +54,26 @@ function test() {
   // Run step2 after the tab loads.
   gBrowser.getBrowserForTab(newTab)
           .addEventListener("pageshow", step2 );
 }
 
 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();
   ok(isImgDecoded(), 'Image should initially be decoded.');
 
   // Focus the old tab, then fire a memory-pressure notification.  This should
   // cause the decoded image in the new tab to be discarded.
   gBrowser.selectedTab = oldTab;
--- a/layout/base/nsPresShell.cpp
+++ b/layout/base/nsPresShell.cpp
@@ -1148,17 +1148,17 @@ PresShell::Destroy()
     mDelayedPaintTimer->Cancel();
     mDelayedPaintTimer = nullptr;
   }
 
   mSynthMouseMoveEvent.Revoke();
 
   mUpdateImageVisibilityEvent.Revoke();
 
-  ClearVisibleImagesList();
+  ClearVisibleImagesList(/* aRequestDiscard = */ true);
 
   if (mCaret) {
     mCaret->Terminate();
     mCaret = nullptr;
   }
 
   if (mSelection) {
     mSelection->DisconnectFromPresShell();
@@ -5815,17 +5815,17 @@ PresShell::MarkImagesInListVisible(const
       }
     }
   }
 }
 
 static PLDHashOperator
 DecrementVisibleCount(nsRefPtrHashKey<nsIImageLoadingContent>* aEntry, void*)
 {
-  aEntry->GetKey()->DecrementVisibleCount();
+  aEntry->GetKey()->DecrementVisibleCount(/* aRequestDiscard = */ false);
   return PL_DHASH_NEXT;
 }
 
 void
 PresShell::RebuildImageVisibilityDisplayList(const nsDisplayList& aList)
 {
   MOZ_ASSERT(!mImageVisibilityVisited, "already visited?");
   mImageVisibilityVisited = true;
@@ -5839,29 +5839,39 @@ PresShell::RebuildImageVisibilityDisplay
 
 /* static */ void
 PresShell::ClearImageVisibilityVisited(nsView* aView, bool aClear)
 {
   nsViewManager* vm = aView->GetViewManager();
   if (aClear) {
     PresShell* presShell = static_cast<PresShell*>(vm->GetPresShell());
     if (!presShell->mImageVisibilityVisited) {
-      presShell->ClearVisibleImagesList();
+      presShell->ClearVisibleImagesList(/* aRequestDiscard = */ false);
     }
     presShell->mImageVisibilityVisited = false;
   }
   for (nsView* v = aView->GetFirstChild(); v; v = v->GetNextSibling()) {
     ClearImageVisibilityVisited(v, v->GetViewManager() != vm);
   }
 }
 
-void
-PresShell::ClearVisibleImagesList()
-{
-  mVisibleImages.EnumerateEntries(DecrementVisibleCount, nullptr);
+static PLDHashOperator
+DecrementVisibleCountAndDiscard(nsRefPtrHashKey<nsIImageLoadingContent>* aEntry,
+                                void*)
+{
+  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();
 }
 
 void
 PresShell::MarkImagesInSubtreeVisible(nsIFrame* aFrame, const nsRect& aRect)
 {
   MOZ_ASSERT(aFrame->PresContext()->PresShell() == this, "wrong presshell");
 
@@ -5981,17 +5991,17 @@ PresShell::UpdateImageVisibility()
 
   if (mHaveShutDown || mIsDestroying) {
     return;
   }
 
   // call update on that frame
   nsIFrame* rootFrame = GetRootFrame();
   if (!rootFrame) {
-    ClearVisibleImagesList();
+    ClearVisibleImagesList(/* aRequestDiscard = */ true);
     return;
   }
 
   RebuildImageVisibility();
   ClearImageVisibilityVisited(rootFrame->GetView(), true);
 
 #ifdef DEBUG_IMAGE_VISIBILITY_DISPLAY_LIST
   // This can be used to debug the frame walker by comparing beforeImageList and
@@ -6140,17 +6150,17 @@ PresShell::RemoveImageFromVisibleList(ns
     MOZ_ASSERT(mVisibleImages.Count() == 0, "shouldn't have any images in the table");
     return;
   }
 
   uint32_t count = mVisibleImages.Count();
   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);
   }
 }
 
 class nsAutoNotifyDidPaint
 {
 public:
   nsAutoNotifyDidPaint(PresShell* aShell, uint32_t aFlags)
     : mShell(aShell), mFlags(aFlags)
--- a/layout/base/nsPresShell.h
+++ b/layout/base/nsPresShell.h
@@ -726,17 +726,17 @@ protected:
   virtual void PausePainting() MOZ_OVERRIDE;
   virtual void ResumePainting() MOZ_OVERRIDE;
 
   void UpdateImageVisibility();
   void UpdateActivePointerState(mozilla::WidgetGUIEvent* aEvent);
 
   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);
 
   void EvictTouches();
 
   // Methods for dispatching KeyboardEvent and BeforeAfterKeyboardEvent.
   void HandleKeyboardEvent(nsINode* aTarget,
--- a/layout/svg/SVGFEImageFrame.cpp
+++ b/layout/svg/SVGFEImageFrame.cpp
@@ -77,17 +77,17 @@ NS_IMPL_FRAMEARENA_HELPERS(SVGFEImageFra
 /* virtual */ void
 SVGFEImageFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
   nsCOMPtr<nsIImageLoadingContent> imageLoader =
     do_QueryInterface(SVGFEImageFrameBase::mContent);
 
   if (imageLoader) {
     imageLoader->FrameDestroyed(this);
-    imageLoader->DecrementVisibleCount();
+    imageLoader->DecrementVisibleCount(/* aRequestDiscard = */ false);
   }
 
   SVGFEImageFrameBase::DestroyFrom(aDestructRoot);
 }
 
 void
 SVGFEImageFrame::Init(nsIContent*       aContent,
                       nsContainerFrame* aParent,