Bug 1364364 - Part 2: Extract a part of EvictExpiredContentViewerForEntry to a new function FindTransactionForBFCache, and implement RemoveDynEntriesForEntry on top of it. r=smaug
☠☠ backed out by 34f2787cfb1f ☠ ☠
authorSamael Wang <freesamael@gmail.com>
Thu, 24 Aug 2017 14:32:23 +0800
changeset 428949 58fe37290a82a1c6096ccaca9f95efdb11451ff2
parent 428948 24edc3618b4571c4d4f1cbb375a704aaa3d489ff
child 428950 e07ddefd4a8e3a052e5a5dfc128f74067ed38e65
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 2: Extract a part of EvictExpiredContentViewerForEntry to a new function FindTransactionForBFCache, and implement RemoveDynEntriesForEntry on top of it. r=smaug MozReview-Commit-ID: EzK2U0c94v8
docshell/shistory/nsISHistoryInternal.idl
docshell/shistory/nsSHistory.cpp
docshell/shistory/nsSHistory.h
--- a/docshell/shistory/nsISHistoryInternal.idl
+++ b/docshell/shistory/nsISHistoryInternal.idl
@@ -116,14 +116,28 @@ interface nsISHistoryInternal: nsISuppor
     *        The container to start looking for dynamic entries. Only the
     *        dynamic descendants of the container will be removed. If not given,
     *        all dynamic entries at the index will be removed.
     */
    [noscript, notxpcom] void RemoveDynEntries(in long aIndex,
                                               in nsISHContainer aContainer);
 
    /**
+    * Similar to RemoveDynEntries, but instead of specifying an index, use the
+    * given BFCacheEntry to find the index and remove dynamic entries from the
+    * index.
+    *
+    * The method takes no effect if the bfcache entry is not or no longer hold
+    * by the SHistory instance.
+    *
+    * @param aEntry
+    *        The bfcache entry to look up for index to remove dynamic entries
+    *        from.
+    */
+   [noscript, notxpcom] void RemoveDynEntriesForBFCacheEntry(in nsIBFCacheEntry aEntry);
+
+   /**
     * Removes entries from the history if their docshellID is in
     * aIDs array.
     */
    [noscript, notxpcom] void RemoveEntries(in nsDocshellIDArray aIDs,
                                            in long aStartIndex);
 };
--- a/docshell/shistory/nsSHistory.cpp
+++ b/docshell/shistory/nsSHistory.cpp
@@ -208,17 +208,17 @@ nsSHistory::EvictContentViewerForTransac
   entry->GetAnyContentViewer(getter_AddRefs(ownerEntry),
                              getter_AddRefs(viewer));
   if (viewer) {
     NS_ASSERTION(ownerEntry, "Content viewer exists but its SHEntry is null");
 
     LOG_SHENTRY_SPEC(("Evicting content viewer 0x%p for "
                       "owning SHEntry 0x%p at %s.",
                       viewer.get(), ownerEntry.get(), _spec),
-                     ownerEntry);
+                      ownerEntry);
 
     // Drop the presentation state before destroying the viewer, so that
     // document teardown is able to correctly persist the state.
     ownerEntry->SetContentViewer(nullptr);
     ownerEntry->SyncPresentationState();
     viewer->Destroy();
   }
 
@@ -1285,18 +1285,23 @@ nsSHistory::GloballyEvictContentViewers(
   for (int32_t i = transactions.Length() - 1; i >= sHistoryMaxTotalViewers;
        --i) {
     (transactions[i].mSHistory)->
       EvictContentViewerForTransaction(transactions[i].mTransaction);
   }
 }
 
 nsresult
-nsSHistory::EvictExpiredContentViewerForEntry(nsIBFCacheEntry* aEntry)
+nsSHistory::FindTransactionForBFCache(nsIBFCacheEntry* aEntry,
+                                      nsISHTransaction** aResult,
+                                      int32_t* aResultIndex)
 {
+  *aResult = nullptr;
+  *aResultIndex = -1;
+
   int32_t startIndex = std::max(0, mIndex - nsISHistory::VIEWER_WINDOW);
   int32_t endIndex = std::min(mLength - 1, mIndex + nsISHistory::VIEWER_WINDOW);
   nsCOMPtr<nsISHTransaction> trans;
   GetTransactionAtIndex(startIndex, getter_AddRefs(trans));
 
   int32_t i;
   for (i = startIndex; trans && i <= endIndex; ++i) {
     nsCOMPtr<nsISHEntry> entry;
@@ -1306,25 +1311,39 @@ nsSHistory::EvictExpiredContentViewerFor
     if (entry->HasBFCacheEntry(aEntry)) {
       break;
     }
 
     nsCOMPtr<nsISHTransaction> temp = trans;
     temp->GetNext(getter_AddRefs(trans));
   }
   if (i > endIndex) {
-    return NS_OK;
+    return NS_ERROR_FAILURE;
   }
 
-  if (i == mIndex) {
+  trans.forget(aResult);
+  *aResultIndex = i;
+  return NS_OK;
+}
+
+nsresult
+nsSHistory::EvictExpiredContentViewerForEntry(nsIBFCacheEntry* aEntry)
+{
+  int32_t index;
+  nsCOMPtr<nsISHTransaction> trans;
+  FindTransactionForBFCache(aEntry, getter_AddRefs(trans), &index);
+
+  if (index == mIndex) {
     NS_WARNING("How did the current SHEntry expire?");
     return NS_OK;
   }
 
-  EvictContentViewerForTransaction(trans);
+  if (trans) {
+    EvictContentViewerForTransaction(trans);
+  }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsSHistory::AddToExpirationTracker(nsIBFCacheEntry* aEntry)
 {
   RefPtr<nsSHEntryShared> entry = static_cast<nsSHEntryShared*>(aEntry);
@@ -1581,16 +1600,30 @@ nsSHistory::RemoveDynEntries(int32_t aIn
     AutoTArray<nsID, 16> toBeRemovedEntries;
     GetDynamicChildren(container, toBeRemovedEntries, true);
     if (toBeRemovedEntries.Length()) {
       RemoveEntries(toBeRemovedEntries, aIndex);
     }
   }
 }
 
+void
+nsSHistory::RemoveDynEntriesForBFCacheEntry(nsIBFCacheEntry* aEntry)
+{
+  int32_t index;
+  nsCOMPtr<nsISHTransaction> trans;
+  FindTransactionForBFCache(aEntry, getter_AddRefs(trans), &index);
+  if (trans) {
+    nsCOMPtr<nsISHEntry> entry;
+    trans->GetSHEntry(getter_AddRefs(entry));
+    nsCOMPtr<nsISHContainer> container(do_QueryInterface(entry));
+    RemoveDynEntries(index, container);
+  }
+}
+
 NS_IMETHODIMP
 nsSHistory::UpdateIndex()
 {
   // Update the actual index with the right value.
   if (mIndex != mRequestedIndex && mRequestedIndex != -1) {
     mIndex = mRequestedIndex;
     NOTIFY_LISTENERS(OnIndexChanged, (mIndex))
   }
--- a/docshell/shistory/nsSHistory.h
+++ b/docshell/shistory/nsSHistory.h
@@ -91,16 +91,22 @@ private:
                         long aLoadType);
 
   nsresult LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd);
 
 #ifdef DEBUG
   nsresult PrintHistory();
 #endif
 
+  // Find the transaction for a given bfcache entry. It only looks up between
+  // the range where alive viewers may exist (i.e nsISHistory::VIEWER_WINDOW).
+  nsresult FindTransactionForBFCache(nsIBFCacheEntry* aEntry,
+                                     nsISHTransaction** aResult,
+                                     int32_t* aResultIndex);
+
   // Evict content viewers in this window which don't lie in the "safe" range
   // around aIndex.
   void EvictOutOfRangeWindowContentViewers(int32_t aIndex);
   void EvictContentViewerForTransaction(nsISHTransaction* aTrans);
   static void GloballyEvictContentViewers();
   static void GloballyEvictAllContentViewers();
 
   // Calculates a max number of total