Bug 1364364 - Part 3: Clear dynamic subframe entries in RemoveFromBFCacheSync/Async. r?smaug
MozReview-Commit-ID: F8OTxbWIp5O
--- a/docshell/shistory/nsSHEntryShared.cpp
+++ b/docshell/shistory/nsSHEntryShared.cpp
@@ -45,17 +45,28 @@ nsSHEntryShared::nsSHEntryShared()
, mSticky(true)
, mDynamicallyCreated(false)
, mExpired(false)
{
}
nsSHEntryShared::~nsSHEntryShared()
{
+ // The destruction can be caused by either the entry is removed from session
+ // history and no one holds the reference, or the whole session history is on
+ // destruction. We want to ensure that we invoke
+ // shistory->RemoveFromExpirationTracker for the former case.
RemoveFromExpirationTracker();
+
+ // Calling RemoveDynEntriesForBFCacheEntry on destruction is unnecessary since
+ // there couldn't be any SHEntry holding this shared entry, and we noticed
+ // that calling RemoveDynEntriesForBFCacheEntry in the middle of
+ // nsSHistory::Release can cause a crash, so set mSHistory to null explicitly
+ // before RemoveFromBFCacheSync.
+ mSHistory = nullptr;
if (mContentViewer) {
RemoveFromBFCacheSync();
}
}
NS_IMPL_ISUPPORTS(nsSHEntryShared, nsIBFCacheEntry, nsIMutationObserver)
already_AddRefed<nsSHEntryShared>
@@ -163,76 +174,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;