Bug 1364364 - Part 3: Clear dynamic subframe entries in RemoveFromBFCacheSync/Async. r=smaug
☠☠ backed out by f9c03c26c876 ☠ ☠
authorSamael Wang <freesamael@gmail.com>
Thu, 24 Aug 2017 15:17:39 +0800
changeset 428713 f23aa9861e9bf64e6225e35469d34e24fd2d820e
parent 428712 5d6bf02b495a923a6b26397ef0bba7f7d00ce57b
child 428714 6a0ec9eff8c3f4c71461f761b28055c2ca9c8a42
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1364364
milestone57.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 1364364 - Part 3: Clear dynamic subframe entries in RemoveFromBFCacheSync/Async. r=smaug MozReview-Commit-ID: F8OTxbWIp5O
docshell/shistory/nsSHEntryShared.cpp
--- a/docshell/shistory/nsSHEntryShared.cpp
+++ b/docshell/shistory/nsSHEntryShared.cpp
@@ -163,76 +163,76 @@ nsSHEntryShared::SetContentViewer(nsICon
   return NS_OK;
 }
 
 nsresult
 nsSHEntryShared::RemoveFromBFCacheSync()
 {
   MOZ_ASSERT(mContentViewer && mDocument, "we're not in the bfcache!");
 
+  // The call to DropPresentationState could drop the last reference, so hold
+  // |this| until RemoveDynEntriesForBFCacheEntry finishes.
+  RefPtr<nsSHEntryShared> kungFuDeathGrip = this;
+
+  // DropPresentationState would clear mContentViewer.
   nsCOMPtr<nsIContentViewer> viewer = mContentViewer;
   DropPresentationState();
 
-  // Warning! The call to DropPresentationState could have dropped the last
-  // reference to this object, so don't access members beyond here.
-
   if (viewer) {
     viewer->Destroy();
   }
 
+  // Now that we've dropped the viewer, we have to clear associated dynamic
+  // subframe entries.
+  nsCOMPtr<nsISHistoryInternal> shistory = do_QueryReferent(mSHistory);
+  if (shistory) {
+    shistory->RemoveDynEntriesForBFCacheEntry(this);
+  }
+
   return NS_OK;
 }
 
-class DestroyViewerEvent : public mozilla::Runnable
-{
-public:
-  DestroyViewerEvent(nsIContentViewer* aViewer, nsIDocument* aDocument)
-    : mozilla::Runnable("DestroyViewerEvent")
-    , mViewer(aViewer)
-    , mDocument(aDocument)
-  {
-  }
-
-  NS_IMETHOD Run() override
-  {
-    if (mViewer) {
-      mViewer->Destroy();
-    }
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsIContentViewer> mViewer;
-  nsCOMPtr<nsIDocument> mDocument;
-};
-
 nsresult
 nsSHEntryShared::RemoveFromBFCacheAsync()
 {
   MOZ_ASSERT(mContentViewer && mDocument, "we're not in the bfcache!");
 
-  // Release the reference to the contentviewer asynchronously so that the
-  // document doesn't get nuked mid-mutation.
-
+  // Check it again to play safe in release builds.
   if (!mDocument) {
     return NS_ERROR_UNEXPECTED;
   }
-  nsCOMPtr<nsIRunnable> evt = new DestroyViewerEvent(mContentViewer, mDocument);
-  nsresult rv = mDocument->Dispatch(mozilla::TaskCategory::Other, evt.forget());
+
+  // DropPresentationState would clear mContentViewer & mDocument. Capture and
+  // release the references asynchronously so that the document doesn't get
+  // nuked mid-mutation.
+  nsCOMPtr<nsIContentViewer> viewer = mContentViewer;
+  nsCOMPtr<nsIDocument> document = mDocument;
+  RefPtr<nsSHEntryShared> self = this;
+  nsresult rv = mDocument->Dispatch(mozilla::TaskCategory::Other,
+    NS_NewRunnableFunction("nsSHEntryShared::RemoveFromBFCacheAsync",
+    [self, viewer, document]() {
+      if (viewer) {
+        viewer->Destroy();
+      }
+
+      nsCOMPtr<nsISHistoryInternal> shistory = do_QueryReferent(self->mSHistory);
+      if (shistory) {
+        shistory->RemoveDynEntriesForBFCacheEntry(self);
+      }
+    }));
+
   if (NS_FAILED(rv)) {
-    NS_WARNING("failed to dispatch DestroyViewerEvent");
+    NS_WARNING("Failed to dispatch RemoveFromBFCacheAsync runnable.");
   } else {
     // Drop presentation. Only do this if we succeeded in posting the event
     // since otherwise the document could be torn down mid-mutation, causing
     // crashes.
     DropPresentationState();
   }
 
-  // Careful! The call to DropPresentationState could have dropped the last
-  // reference to this nsSHEntryShared, so don't access members beyond here.
-
   return NS_OK;
 }
 
 nsresult
 nsSHEntryShared::GetID(uint64_t* aID)
 {
   *aID = mID;
   return NS_OK;