Bug 1363036 - Part 2: Reject AddEntry / ReplaceEntry if the entry has been associated to another SHistory. Cleanup mHistoryTracker if root docshell changes. r?smaug draft
authorSamael Wang <freesamael@gmail.com>
Mon, 22 May 2017 15:36:06 +0800
changeset 584234 15d2400559754f1dc6b08d3f0f0fb7e8ddfb8acd
parent 582880 bde2a7f515979ad881b5e97183459f4d7ea2ba1e
child 630309 3bb2409aa9fd973c7391e547749e0861ee7d476b
push id60661
push userbmo:sawang@mozilla.com
push dateThu, 25 May 2017 02:45:41 +0000
reviewerssmaug
bugs1363036
milestone55.0a1
Bug 1363036 - Part 2: Reject AddEntry / ReplaceEntry if the entry has been associated to another SHistory. Cleanup mHistoryTracker if root docshell changes. r?smaug MozReview-Commit-ID: 9s4dQG18JUN
docshell/shistory/nsISHEntry.idl
docshell/shistory/nsSHEntry.cpp
docshell/shistory/nsSHistory.cpp
--- a/docshell/shistory/nsISHEntry.idl
+++ b/docshell/shistory/nsISHEntry.idl
@@ -330,19 +330,21 @@ interface nsISHEntry : nsISupports
 
     /**
      * Flag to indicate that the history entry was originally loaded in the
      * current process. This flag does not survive a browser process switch.
      */
     readonly attribute boolean loadedInThisProcess;
 
     /**
-     * Set the session history it belongs to. It's only set on root entries.
+     * The session history it belongs to. It's usually only set on root entries.
+     * SHEntry is strictly bound to the SHistory it belongs to; it should not be
+     * changed once set to a non-null value.
      */
-    [noscript] void setSHistory(in nsISHistory aSHistory);
+    [noscript] attribute nsISHistory SHistory;
 };
 
 [scriptable, uuid(bb66ac35-253b-471f-a317-3ece940f04c5)]
 interface nsISHEntryInternal : nsISupports
 {
     [notxpcom] void RemoveFromBFCacheAsync();
     [notxpcom] void RemoveFromBFCacheSync();
 
--- a/docshell/shistory/nsSHEntry.cpp
+++ b/docshell/shistory/nsSHEntry.cpp
@@ -970,16 +970,24 @@ nsSHEntry::GetLastTouched(uint32_t* aLas
 NS_IMETHODIMP
 nsSHEntry::SetLastTouched(uint32_t aLastTouched)
 {
   mShared->mLastTouched = aLastTouched;
   return NS_OK;
 }
 
 NS_IMETHODIMP
+nsSHEntry::GetSHistory(nsISHistory** aSHistory)
+{
+  nsCOMPtr<nsISHistory> shistory(do_QueryReferent(mShared->mSHistory));
+  shistory.forget(aSHistory);
+  return NS_OK;
+}
+
+NS_IMETHODIMP
 nsSHEntry::SetSHistory(nsISHistory* aSHistory)
 {
   nsWeakPtr shistory = do_GetWeakReference(aSHistory);
   // mSHistory can not be changed once it's set
   MOZ_ASSERT(!mShared->mSHistory || (mShared->mSHistory == shistory));
   mShared->mSHistory = shistory;
   return NS_OK;
 }
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -380,16 +380,25 @@ nsSHistory::Shutdown()
 /* Add an entry to the History list at mIndex and
  * increment the index to point to the new entry
  */
 NS_IMETHODIMP
 nsSHistory::AddEntry(nsISHEntry* aSHEntry, bool aPersist)
 {
   NS_ENSURE_ARG(aSHEntry);
 
+  nsCOMPtr<nsISHistory> shistoryOfEntry;
+  aSHEntry->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;
+  }
+
   aSHEntry->SetSHistory(this);
 
   // If we have a root docshell, update the docshell id of the root shentry to
   // match the id of that docshell
   if (mRootDocShell) {
     nsID docshellID = mRootDocShell->HistoryID();
     aSHEntry->SetDocshellID(&docshellID);
   }
@@ -862,16 +871,27 @@ nsSHistory::ReplaceEntry(int32_t aIndex,
   if (!mListRoot) {
     // Session History is not initialised.
     return NS_ERROR_FAILURE;
   }
 
   rv = GetTransactionAtIndex(aIndex, getter_AddRefs(currentTxn));
 
   if (currentTxn) {
+    nsCOMPtr<nsISHistory> shistoryOfEntry;
+    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;
 }
@@ -1933,16 +1953,24 @@ nsSHistory::SetRootDocShell(nsIDocShell*
   // Init mHistoryTracker on setting mRootDocShell so we can bind its event
   // target to the tabGroup.
   if (mRootDocShell) {
     nsCOMPtr<nsPIDOMWindowOuter> win = mRootDocShell->GetWindow();
     if (!win) {
       return NS_ERROR_UNEXPECTED;
     }
 
+    // Seamonkey moves shistory between <xul:browser>s when restoring a tab.
+    // Let's try not to break our friend too badly...
+    if (mHistoryTracker) {
+      NS_WARNING("Change the root docshell of a shistory is unsafe and "
+                 "potentially problematic.");
+      mHistoryTracker->AgeAllGenerations();
+    }
+
     RefPtr<mozilla::dom::TabGroup> tabGroup = win->TabGroup();
     mHistoryTracker = mozilla::MakeUnique<HistoryTracker>(
       this,
       mozilla::Preferences::GetUint(CONTENT_VIEWER_TIMEOUT_SECONDS,
                                     CONTENT_VIEWER_TIMEOUT_SECONDS_DEFAULT),
       tabGroup->EventTargetFor(mozilla::TaskCategory::Other));
   }