Backed out 7 changesets (bug 1364364) for causing bug 1399182.

Backed out changeset c517d8071dfb (bug 1364364) Backed out changeset 809036cfd7d9 (bug 1364364) Backed out changeset c394b06dc30c (bug 1364364) Backed out changeset c5a737bbfdeb (bug 1364364) Backed out changeset 21ee8f318a47 (bug 1364364) Backed out changeset 074475da0f2c (bug 1364364) Backed out changeset de6c153ec533 (bug 1364364) --HG-- rename : dom/indexedDB/test/bfcache_page1.html => dom/indexedDB/test/bfcache_iframe1.html rename : dom/indexedDB/test/bfcache_page2.html => dom/indexedDB/test/bfcache_iframe2.html rename : dom/media/webspeech/synth/test/file_bfcache_page1.html => dom/media/webspeech/synth/test/file_bfcache_frame.html rename : dom/media/webspeech/synth/test/file_bfcache_page2.html => dom/media/webspeech/synth/test/file_bfcache_frame2.html rename : dom/workers/test/WorkerDebugger_frozen_window1.html => dom/workers/test/WorkerDebugger_frozen_iframe1.html rename : dom/workers/test/WorkerDebugger_frozen_window2.html => dom/workers/test/WorkerDebugger_frozen_iframe2.html rename : dom/workers/test/suspend_window.html => dom/workers/test/suspend_iframe.html
dothq · Sep 12, 2017 · 68c8008 · 68c8008
1 parent ecf716b
commit 68c8008
Show file tree

Hide file tree

Showing 31 changed files with 278 additions and 389 deletions.
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -8314,11 +8314,16 @@ nsDocShell::CanSavePresentation(uint32_t aLoadType,
     return false;
   }
 
-  // Don't cache the content viewer if we're in a subframe.
-  nsCOMPtr<nsIDocShellTreeItem> root;
-  GetSameTypeParent(getter_AddRefs(root));
-  if (root && root != this) {
-    return false;  // this is a subframe load
+  // Don't cache the content viewer if we're in a subframe and the subframe
+  // pref is disabled.
+  bool cacheFrames =
+    Preferences::GetBool("browser.sessionhistory.cache_subframes", false);
+  if (!cacheFrames) {
+    nsCOMPtr<nsIDocShellTreeItem> root;
+    GetSameTypeParent(getter_AddRefs(root));
+    if (root && root != this) {
+      return false;  // this is a subframe load
+    }
   }
 
   // If the document does not want its presentation cached, then don't.

diff --git a/docshell/shistory/nsISHistoryInternal.idl b/docshell/shistory/nsISHistoryInternal.idl
@@ -120,20 +120,6 @@ interface nsISHistoryInternal: nsISupports
    [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.

diff --git a/docshell/shistory/nsSHEntryShared.cpp b/docshell/shistory/nsSHEntryShared.cpp
@@ -168,66 +168,66 @@ 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!");
 
-  // Check it again to play safe in release builds.
+  // Release the reference to the contentviewer asynchronously so that the
+  // document doesn't get nuked mid-mutation.
+
   if (!mDocument) {
     return NS_ERROR_UNEXPECTED;
   }
-
-  // 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);
-      }
-    }));
-
+  nsCOMPtr<nsIRunnable> evt = new DestroyViewerEvent(mContentViewer, mDocument);
+  nsresult rv = mDocument->Dispatch(mozilla::TaskCategory::Other, evt.forget());
   if (NS_FAILED(rv)) {
-    NS_WARNING("Failed to dispatch RemoveFromBFCacheAsync runnable.");
+    NS_WARNING("failed to dispatch DestroyViewerEvent");
   } 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;
 }
 

diff --git a/docshell/shistory/nsSHistory.cpp b/docshell/shistory/nsSHistory.cpp
@@ -213,7 +213,7 @@ nsSHistory::EvictContentViewerForTransaction(nsISHTransaction* aTrans)
     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.
@@ -588,7 +588,7 @@ nsSHistory::GetEntryAtIndex(int32_t aIndex, bool aModifyIndex,
 }
 
 /* Get the transaction at a given index */
-nsresult
+NS_IMETHODIMP
 nsSHistory::GetTransactionAtIndex(int32_t aIndex, nsISHTransaction** aResult)
 {
   nsresult rv;
@@ -1290,13 +1290,8 @@ nsSHistory::GloballyEvictContentViewers()
 }
 
 nsresult
-nsSHistory::FindTransactionForBFCache(nsIBFCacheEntry* aEntry,
-                                      nsISHTransaction** aResult,
-                                      int32_t* aResultIndex)
+nsSHistory::EvictExpiredContentViewerForEntry(nsIBFCacheEntry* aEntry)
 {
-  *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;
@@ -1316,29 +1311,15 @@ nsSHistory::FindTransactionForBFCache(nsIBFCacheEntry* aEntry,
     temp->GetNext(getter_AddRefs(trans));
   }
   if (i > endIndex) {
-    return NS_ERROR_FAILURE;
+    return NS_OK;
   }
 
-  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) {
+  if (i == mIndex) {
     NS_WARNING("How did the current SHEntry expire?");
     return NS_OK;
   }
 
-  if (trans) {
-    EvictContentViewerForTransaction(trans);
-  }
+  EvictContentViewerForTransaction(trans);
 
   return NS_OK;
 }
@@ -1605,20 +1586,6 @@ nsSHistory::RemoveDynEntries(int32_t aIndex, nsISHContainer* aContainer)
   }
 }
 
-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()
 {
@@ -1736,7 +1703,7 @@ nsSHistory::LoadNextPossibleEntry(int32_t aNewIndex, long aLoadType,
   return NS_ERROR_FAILURE;
 }
 
-nsresult
+NS_IMETHODIMP
 nsSHistory::LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd)
 {
   if (!mRootDocShell) {

diff --git a/docshell/shistory/nsSHistory.h b/docshell/shistory/nsSHistory.h
@@ -83,25 +83,20 @@ class nsSHistory final : public mozilla::LinkedListElement<nsSHistory>,
   friend class nsSHEnumerator;
   friend class nsSHistoryObserver;
 
-  nsresult GetTransactionAtIndex(int32_t aIndex, nsISHTransaction** aResult);
+  // Could become part of nsIWebNavigation
+  NS_IMETHOD GetTransactionAtIndex(int32_t aIndex, nsISHTransaction** aResult);
   nsresult LoadDifferingEntries(nsISHEntry* aPrevEntry, nsISHEntry* aNextEntry,
                                 nsIDocShell* aRootDocShell, long aLoadType,
                                 bool& aDifferenceFound);
   nsresult InitiateLoad(nsISHEntry* aFrameEntry, nsIDocShell* aFrameDS,
                         long aLoadType);
 
-  nsresult LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd);
+  NS_IMETHOD 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);

diff --git a/docshell/test/chrome/bug608669.xul b/docshell/test/chrome/bug608669.xul
@@ -1,14 +1,6 @@
 <?xml version="1.0"?>
 <?xml-stylesheet href="chrome://global/skin" type="text/css"?>
 <window title="Mozilla Bug 608669 - Blank page"
-  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-  onload="notifyOpener();">
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
   <description flex="1" value="This window is intentionally left blank"/>
-  <script type="application/javascript">
-  function notifyOpener() {
-    if (opener) {
-      opener.postMessage("load", "*");
-    }
-  }
-  </script>
 </window>
diff --git a/docshell/test/chrome/test_bug608669.xul b/docshell/test/chrome/test_bug608669.xul
@@ -18,6 +18,27 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=608669
   <!-- test code goes here -->
   <script type="application/javascript"><![CDATA[
 
+var gOrigMaxTotalViewers = undefined;
+function setCachePref(enabled) {
+  var prefBranch = Components.classes["@mozilla.org/preferences-service;1"]
+                             .getService(Components.interfaces.nsIPrefBranch);
+  if (enabled) {
+    is(typeof gOrigMaxTotalViewers, "undefined", "don't double-enable bfcache");
+    prefBranch.setBoolPref("browser.sessionhistory.cache_subframes", true);
+    gOrigMaxTotalViewers = prefBranch.getIntPref("browser.sessionhistory.max_total_viewers");
+    prefBranch.setIntPref("browser.sessionhistory.max_total_viewers", 10);
+  }
+  else {
+    is(typeof gOrigMaxTotalViewers, "number", "don't double-disable bfcache");
+    prefBranch.setIntPref("browser.sessionhistory.max_total_viewers", gOrigMaxTotalViewers);
+    gOrigMaxTotalViewers = undefined;
+    try {
+      prefBranch.clearUserPref("browser.sessionhistory.cache_subframes");
+    } catch (e) { /* Pref didn't exist, meh */ }
+  }
+}
+
+
 /** Test for Bug 608669 **/
 SimpleTest.waitForExplicitFinish();
 
@@ -30,6 +51,10 @@ function nextTest() {
 }
 
 function* doTest() {
+  var container = document.getElementById('container');
+
+  setCachePref(true);
+
   var notificationCount = 0;
   var observer = {
     observe: function(aSubject, aTopic, aData) {
@@ -48,31 +73,44 @@ function* doTest() {
 
   is(notificationCount, 0, "initial count");
 
-  // create a new window
-  var testWin = window.open("", "bug 608669", "chrome,width=600,height=600");
-  testWin.x = "y";
-  is(notificationCount, 1, "after created window");
-
-  // Try loading in the window
-  testWin.location = "bug608669.xul";
-  window.onmessage = nextTest;
+  // create a new iframe
+  var iframe = document.createElement("iframe");
+  container.appendChild(iframe);
+  iframe.contentWindow.x = "y";
+  is(notificationCount, 1, "after created iframe");
+  
+  // Try loading in an iframe
+  iframe.setAttribute("src", "bug608669.xul");
+  iframe.onload = nextTest;
   yield undefined;
   is(notificationCount, 1, "after first load");
-  is(testWin.x, "y", "reused window");
+  is(iframe.contentWindow.x, "y", "reused window");
 
-  // Try loading again in the window
-  testWin.location = "bug608669.xul?x";
-  window.onmessage = nextTest;
+  // Try loading again in an iframe
+  iframe.setAttribute("src", "bug608669.xul?x");
+  iframe.onload = nextTest;
   yield undefined;
   is(notificationCount, 2, "after second load");
-  is("x" in testWin, false, "didn't reuse window");
+  is("x" in iframe.contentWindow, false, "didn't reuse window");
 
-  testWin.close();
+  // Open a new window using window.open
+  popup = window.open("bug608669.xul", "bug 608669",
+                      "chrome,width=600,height=600");
+  popup.onload = nextTest;
+  yield undefined;
+  is(notificationCount, 3, "after window.open load");
+  popup.close();
 
+  setCachePref(false);
   os.removeObserver(observer, "chrome-document-global-created");
   os.removeObserver(observer, "content-document-global-created");
   SimpleTest.finish();
 }
 
+
+
   ]]></script>
+  <vbox id="container" flex="1">
+    <description>Below will an iframe be added</description>
+  </vbox>
 </window>