Bug 1125490 (Part 1) - Make sure we request discarding for images in PresShell::Destroy. r=tn
☠☠ backed out by 1f0d37966eac ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Sat, 24 Jan 2015 23:14:10 -0800
changeset 225604 fa3e911c2941490786e742a7e2f8840ace73301e
parent 225603 e43c8f946c95d975cbdcfc5bcb99661985159b6a
child 225605 042523d55ed89d57ecd55a3f67702df963546ac9
push id28170
push userphilringnalda@gmail.com
push dateSun, 25 Jan 2015 19:44:07 +0000
treeherdermozilla-central@fa91879c8428 [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,