Bug 1363036 - Part 3: Ensure mSHistory is set for root entries in ReplaceEntry & after AbandonBFCacheEntry. r?smaug draft
authorSamael Wang <freesamael@gmail.com>
Mon, 22 May 2017 15:37:48 +0800
changeset 582334 bf6ff9eede9ae5f04beb8b872c01086cfbb0137d
parent 582333 4b069e77912dc2fb4722447bb2af29f701b9ef4d
child 629729 b4f987cdabe7f6e3e80bbc8f1285f31feb0f9fea
push id60039
push userbmo:sawang@mozilla.com
push dateMon, 22 May 2017 07:47:46 +0000
reviewerssmaug
bugs1363036
milestone55.0a1
Bug 1363036 - Part 3: Ensure mSHistory is set for root entries in ReplaceEntry & after AbandonBFCacheEntry. r?smaug MozReview-Commit-ID: 5wT5bOR0tu1
docshell/base/nsDocShell.cpp
docshell/shistory/nsSHEntryShared.cpp
docshell/shistory/nsSHistory.cpp
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -5341,16 +5341,20 @@ nsDocShell::LoadErrorPage(nsIURI* aURI, 
   mFailedURI = aURI;
   mFailedLoadType = mLoadType;
 
   if (mLSHE) {
     // Abandon mLSHE's BFCache entry and create a new one.  This way, if
     // we go back or forward to another SHEntry with the same doc
     // identifier, the error page won't persist.
     mLSHE->AbandonBFCacheEntry();
+
+    // AbandonBFCacheEntry would not copy SHistory reference. Set to our
+    // mSessionHistory since we're sure it still belongs to the same SHistory.
+    mLSHE->SetSHistory(mSessionHistory);
   }
 
   nsAutoCString url;
   nsAutoCString charset;
   if (aURI) {
     nsresult rv = aURI->GetSpec(url);
     NS_ENSURE_SUCCESS(rv, rv);
     rv = aURI->GetOriginCharset(charset);
@@ -11741,16 +11745,21 @@ nsDocShell::OnNewURI(nsIURI* aURI, nsICh
         uploadChannel->GetUploadStream(getter_AddRefs(inputStream));
       }
 
       // If the response status indicates an error, unlink this session
       // history entry from any entries sharing its document.
       nsresult rv = httpChannel->GetResponseStatus(&responseStatus);
       if (mLSHE && NS_SUCCEEDED(rv) && responseStatus >= 400) {
         mLSHE->AbandonBFCacheEntry();
+
+        // AbandonBFCacheEntry would not copy SHistory reference. Set to our
+        // mSessionHistory since we're sure it still belongs to the same
+        // SHistory.
+        mLSHE->SetSHistory(mSessionHistory);
       }
     }
   }
 
   // Determine if this type of load should update history.
   bool updateGHistory = !(aLoadType == LOAD_BYPASS_HISTORY ||
                           aLoadType == LOAD_ERROR_PAGE ||
                           aLoadType & LOAD_CMD_HISTORY);
@@ -12394,16 +12403,20 @@ nsDocShell::AddToSessionHistory(nsIURI* 
       shContainer->GetChildCount(&childCount);
       // Remove all children of this entry
       for (int32_t i = childCount - 1; i >= 0; i--) {
         nsCOMPtr<nsISHEntry> child;
         shContainer->GetChildAt(i, getter_AddRefs(child));
         shContainer->RemoveChild(child);
       }
       entry->AbandonBFCacheEntry();
+
+      // AbandonBFCacheEntry would not copy SHistory reference. Set to our
+      // mSessionHistory since we're sure it still belongs to the same SHistory.
+      entry->SetSHistory(mSessionHistory);
     }
   }
 
   // Create a new entry if necessary.
   if (!entry) {
     entry = do_CreateInstance(NS_SHENTRY_CONTRACTID);
 
     if (!entry) {
--- a/docshell/shistory/nsSHEntryShared.cpp
+++ b/docshell/shistory/nsSHEntryShared.cpp
@@ -58,16 +58,20 @@ nsSHEntryShared::~nsSHEntryShared()
 
 NS_IMPL_ISUPPORTS(nsSHEntryShared, nsIBFCacheEntry, nsIMutationObserver)
 
 already_AddRefed<nsSHEntryShared>
 nsSHEntryShared::Duplicate(nsSHEntryShared* aEntry)
 {
   RefPtr<nsSHEntryShared> newEntry = new nsSHEntryShared();
 
+  // Keep in mind that Duplicate can be from a SHEntry cloned from another
+  // docshell (eg. nsDocShell::LoadPage), so do not copy anything tie to the
+  // nsDocShell or nsSHistory instance, such as mSHistory.
+
   newEntry->mDocShellID = aEntry->mDocShellID;
   newEntry->mChildShells.AppendObjects(aEntry->mChildShells);
   newEntry->mTriggeringPrincipal = aEntry->mTriggeringPrincipal;
   newEntry->mPrincipalToInherit = aEntry->mPrincipalToInherit;
   newEntry->mContentType.Assign(aEntry->mContentType);
   newEntry->mIsFrameNavigation = aEntry->mIsFrameNavigation;
   newEntry->mSaveLayoutState = aEntry->mSaveLayoutState;
   newEntry->mSticky = aEntry->mSticky;
@@ -139,16 +143,26 @@ nsSHEntryShared::SetContentViewer(nsICon
   // non-null content viewer, the entry shouldn't have been tracked either.
   MOZ_ASSERT(!GetExpirationState()->IsTracked());
   mContentViewer = aViewer;
 
   if (mContentViewer) {
     // mSHistory is only set for root entries, but in general bfcache only
     // applies to root entries as well. BFCache for subframe navigation has been
     // disabled since 2005 in bug 304860.
+
+#ifdef DEBUG
+    // Check we have mSHistory set if it's a top-level entry. This helps
+    // ensuring that we remember to set mSHistory after duplicating a top-level
+    // entry.
+    nsCOMPtr<nsIDocShell> container;
+    mContentViewer->GetContainer(getter_AddRefs(container));
+    MOZ_ASSERT(mSHistory || !container->GetIsTopLevelContentDocShell());
+#endif
+
     nsCOMPtr<nsISHistoryInternal> shistory = do_QueryReferent(mSHistory);
     if (shistory) {
       shistory->AddToExpirationTracker(this);
     }
 
     nsCOMPtr<nsIDOMDocument> domDoc;
     mContentViewer->GetDOMDocument(getter_AddRefs(domDoc));
     // Store observed document in strong pointer in case it is removed from
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -878,16 +878,18 @@ nsSHistory::ReplaceEntry(int32_t aIndex,
     aReplaceEntry->GetSHistory(getter_AddRefs(shistoryOfEntry));
     if (shistoryOfEntry && shistoryOfEntry != this) {
       NS_WARNING("The entry has been associated to another nsISHistory instance. "
       "Try nsISHEntry.clone() and nsISHEntry.abandonBFCacheEntry() first "
       "if you're copying an entry from another nsISHistory.");
       return NS_ERROR_FAILURE;
     }
 
+    aReplaceEntry->SetSHistory(this);
+
     NOTIFY_LISTENERS(OnHistoryReplaceEntry, (aIndex));
 
     // Set the replacement entry in the transaction
     rv = currentTxn->SetSHEntry(aReplaceEntry);
     rv = currentTxn->SetPersist(true);
   }
   return rv;
 }