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 f9c03c26c876 ☠ ☠
authorSamael Wang <freesamael@gmail.com>
Thu, 24 Aug 2017 14:32:23 +0800
changeset 660369 5d6bf02b495a923a6b26397ef0bba7f7d00ce57b
parent 660368 c9420f4e27f7fce1f322e6323f00a3204066e899
child 660370 f23aa9861e9bf64e6225e35469d34e24fd2d820e
push id78390
push userbmo:emilio@crisal.io
push dateWed, 06 Sep 2017 23:04:15 +0000
reviewerssmaug
bugs1364364
milestone57.0a1
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