Bug 449780. When doing a frameloader swap, clear the bfcache on both docshells, since we can't deal with all those presentations. r+sr=jst
authorBoris Zbarsky <bzbarsky@mit.edu>
Tue, 13 Jan 2009 14:32:30 -0500
changeset 23620 f0a8064d5a8e08bb9d80acf3608165fb47e5fb28
parent 23619 5a53d161e4b44c9d728b24960f41bc65131be715
child 23621 8dae102faa15b6ae4182428c012156dbcceeeb50
push idunknown
push userunknown
push dateunknown
bugs449780
milestone1.9.2a1pre
Bug 449780. When doing a frameloader swap, clear the bfcache on both docshells, since we can't deal with all those presentations. r+sr=jst
content/base/src/nsFrameLoader.cpp
docshell/shistory/public/nsISHistoryInternal.idl
docshell/shistory/src/nsSHistory.cpp
docshell/shistory/src/nsSHistory.h
docshell/test/chrome/Makefile.in
docshell/test/chrome/bug449780_window.xul
docshell/test/chrome/test_bug449780.xul
--- a/content/base/src/nsFrameLoader.cpp
+++ b/content/base/src/nsFrameLoader.cpp
@@ -68,16 +68,17 @@
 #include "nsIDOMEventTarget.h"
 #include "nsIFrame.h"
 #include "nsIFrameFrame.h"
 #include "nsDOMError.h"
 #include "nsPresShellIterator.h"
 #include "nsGUIEvent.h"
 #include "nsEventDispatcher.h"
 #include "nsISHistory.h"
+#include "nsISHistoryInternal.h"
 
 #include "nsIURI.h"
 #include "nsIURL.h"
 #include "nsNetUtil.h"
 
 #include "nsGkAtoms.h"
 #include "nsINameSpaceManager.h"
 
@@ -489,47 +490,46 @@ nsFrameLoader::SwapWithOtherLoader(nsFra
   // frameloaders that don't correspond to root same-type docshells,
   // unless both roots have session history disabled.
   nsCOMPtr<nsIDocShellTreeItem> ourTreeItem = do_QueryInterface(ourDochell);
   nsCOMPtr<nsIDocShellTreeItem> otherTreeItem =
     do_QueryInterface(otherDocshell);
   nsCOMPtr<nsIDocShellTreeItem> ourRootTreeItem, otherRootTreeItem;
   ourTreeItem->GetSameTypeRootTreeItem(getter_AddRefs(ourRootTreeItem));
   otherTreeItem->GetSameTypeRootTreeItem(getter_AddRefs(otherRootTreeItem));
-  if (ourRootTreeItem != ourTreeItem || otherRootTreeItem != otherTreeItem) {
-    nsCOMPtr<nsIWebNavigation> ourRootWebnav =
-      do_QueryInterface(ourRootTreeItem);
-    nsCOMPtr<nsIWebNavigation> otherRootWebnav =
-      do_QueryInterface(otherRootTreeItem);
+  nsCOMPtr<nsIWebNavigation> ourRootWebnav =
+    do_QueryInterface(ourRootTreeItem);
+  nsCOMPtr<nsIWebNavigation> otherRootWebnav =
+    do_QueryInterface(otherRootTreeItem);
+
+  if (!ourRootWebnav || !otherRootWebnav) {
+    return NS_ERROR_NOT_IMPLEMENTED;
+  }
 
-    if (!ourRootWebnav || !otherRootWebnav) {
-      return NS_ERROR_NOT_IMPLEMENTED;
-    }
+  nsCOMPtr<nsISHistory> ourHistory;
+  nsCOMPtr<nsISHistory> otherHistory;
+  ourRootWebnav->GetSessionHistory(getter_AddRefs(ourHistory));
+  otherRootWebnav->GetSessionHistory(getter_AddRefs(otherHistory));
 
-    nsCOMPtr<nsISHistory> ourHistory;
-    nsCOMPtr<nsISHistory> otherHistory;
-    ourRootWebnav->GetSessionHistory(getter_AddRefs(ourHistory));
-    otherRootWebnav->GetSessionHistory(getter_AddRefs(otherHistory));
-
-    if (ourHistory || otherHistory) {
-      return NS_ERROR_NOT_IMPLEMENTED;
-    }
+  if ((ourRootTreeItem != ourTreeItem || otherRootTreeItem != otherTreeItem) &&
+      (ourHistory || otherHistory)) {
+    return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   // Also make sure that the two docshells are the same type. Otherwise
   // swapping is certainly not safe.
   PRInt32 ourType = nsIDocShellTreeItem::typeChrome;
   PRInt32 otherType = nsIDocShellTreeItem::typeChrome;
   ourTreeItem->GetItemType(&ourType);
   otherTreeItem->GetItemType(&otherType);
   if (ourType != otherType) {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
-  // One more twist here.  Setting up the right treeowners in a heterogenous
+  // One more twist here.  Setting up the right treeowners in a heterogeneous
   // tree is a bit of a pain.  So make sure that if ourType is not
   // nsIDocShellTreeItem::typeContent then all of our descendants are the same
   // type as us.
   if (ourType != nsIDocShellTreeItem::typeContent &&
       (!AllDescendantsOfType(ourTreeItem, ourType) ||
        !AllDescendantsOfType(otherTreeItem, otherType))) {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
@@ -687,16 +687,28 @@ nsFrameLoader::SwapWithOtherLoader(nsFra
   ourWindow->SetFrameElementInternal(otherFrameElement);
   otherWindow->SetFrameElementInternal(ourFrameElement);
 
   mOwnerContent = otherContent;
   aOther->mOwnerContent = ourContent;
 
   aFirstToSwap.swap(aSecondToSwap);
 
+  // Drop any cached content viewers in the two session histories.
+  nsCOMPtr<nsISHistoryInternal> ourInternalHistory =
+    do_QueryInterface(ourHistory);
+  nsCOMPtr<nsISHistoryInternal> otherInternalHistory =
+    do_QueryInterface(otherHistory);
+  if (ourInternalHistory) {
+    ourInternalHistory->EvictAllContentViewers();
+  }
+  if (otherInternalHistory) {
+    otherInternalHistory->EvictAllContentViewers();
+  }
+
   // We shouldn't have changed frames, but be really careful about it
   if (ourFrame == ourShell->GetPrimaryFrameFor(ourContent) &&
       otherFrame == otherShell->GetPrimaryFrameFor(otherContent)) {
     ourFrameFrame->EndSwapDocShells(otherFrame);
   }
 
   ourParentDocument->FlushPendingNotifications(Flush_Layout);
   otherParentDocument->FlushPendingNotifications(Flush_Layout);
--- a/docshell/shistory/public/nsISHistoryInternal.idl
+++ b/docshell/shistory/public/nsISHistoryInternal.idl
@@ -47,17 +47,17 @@ interface nsIDocShell;
 %{C++
 #define NS_SHISTORY_INTERNAL_CID \
 { 0x9c47c121, 0x1c6e, 0x4d8f, \
   { 0xb9, 0x04, 0x3a, 0xc9, 0x68, 0x11, 0x6e, 0x88 } }
 
 #define NS_SHISTORY_INTERNAL_CONTRACTID "@mozilla.org/browser/shistory-internal;1"
 %}
 
-[scriptable, uuid(9c47c121-1c6e-4d8f-b904-3ac968116e88)]
+[scriptable, uuid(7ca0fd71-437c-48ad-985d-11ce9e2429b4)]
 interface nsISHistoryInternal: nsISupports
 {
   /**
    * Add a new Entry to the History List
    * @param aEntry - The entry to add
    * @param aPersist - If true this specifies that the entry should persist
    * in the list.  If false, this means that when new entries are added
    * this element will not appear in the session history list.
@@ -100,9 +100,14 @@ interface nsISHistoryInternal: nsISuppor
    */
    void evictContentViewers(in long previousIndex, in long index);
    
    /**
     * Evict the content viewer associated with a session history entry
     * that has timed out.
     */
    void evictExpiredContentViewerForEntry(in nsISHEntry aEntry);
+
+   /**
+    * Evict all the content viewers in this session history
+    */
+   void evictAllContentViewers();
 };
--- a/docshell/shistory/src/nsSHistory.cpp
+++ b/docshell/shistory/src/nsSHistory.cpp
@@ -113,17 +113,17 @@ nsSHistoryObserver::Observe(nsISupports 
                       &nsSHistory::sHistoryMaxTotalViewers);
     if (nsSHistory::sHistoryMaxTotalViewers < 0) {
       nsSHistory::sHistoryMaxTotalViewers = nsSHistory::CalcMaxTotalViewers();
     }
 
     nsSHistory::EvictGlobalContentViewer();
   } else if (!strcmp(aTopic, NS_CACHESERVICE_EMPTYCACHE_TOPIC_ID) ||
              !strcmp(aTopic, "memory-pressure")) {
-    nsSHistory::EvictAllContentViewers();
+    nsSHistory::EvictAllContentViewersGlobally();
   }
 
   return NS_OK;
 }
 
 //*****************************************************************************
 //***    nsSHistory: Object Management
 //*****************************************************************************
@@ -659,16 +659,27 @@ nsSHistory::EvictContentViewers(PRInt32 
 {
   // Check our per SHistory object limit in the currently navigated SHistory
   EvictWindowContentViewers(aPreviousIndex, aIndex);
   // Check our total limit across all SHistory objects
   EvictGlobalContentViewer();
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsSHistory::EvictAllContentViewers()
+{
+  // XXXbz we don't actually do a good job of evicting things as we should, so
+  // we might have viewers quite far from mIndex.  So just evict everything.
+  EvictContentViewersInRange(0, mLength);
+  return NS_OK;
+}
+
+
+
 //*****************************************************************************
 //    nsSHistory: nsIWebNavigation
 //*****************************************************************************
 
 NS_IMETHODIMP
 nsSHistory::GetCanGoBack(PRBool * aCanGoBack)
 {
   NS_ENSURE_ARG_POINTER(aCanGoBack);
@@ -1033,17 +1044,17 @@ nsSHistory::EvictExpiredContentViewerFor
 
 // Evicts all content viewers in all history objects.  This is very
 // inefficient, because it requires a linear search through all SHistory
 // objects for each viewer to be evicted.  However, this method is called
 // infrequently -- only when the disk or memory cache is cleared.
 
 //static
 void
-nsSHistory::EvictAllContentViewers()
+nsSHistory::EvictAllContentViewersGlobally()
 {
   PRInt32 maxViewers = sHistoryMaxTotalViewers;
   sHistoryMaxTotalViewers = 0;
   EvictGlobalContentViewer();
   sHistoryMaxTotalViewers = maxViewers;
 }
 
 NS_IMETHODIMP
--- a/docshell/shistory/src/nsSHistory.h
+++ b/docshell/shistory/src/nsSHistory.h
@@ -94,20 +94,22 @@ protected:
    nsresult InitiateLoad(nsISHEntry * aFrameEntry, nsIDocShell * aFrameDS, long aLoadType);
 
    NS_IMETHOD LoadEntry(PRInt32 aIndex, long aLoadType, PRUint32 histCmd);
 	
 #ifdef DEBUG
    nsresult PrintHistory();
 #endif
 
+  // Evict the viewers at indices between aStartIndex and aEndIndex,
+  // including aStartIndex but not aEndIndex.
   void EvictContentViewersInRange(PRInt32 aStartIndex, PRInt32 aEndIndex);
   void EvictWindowContentViewers(PRInt32 aFromIndex, PRInt32 aToIndex);
   static void EvictGlobalContentViewer();
-  static void EvictAllContentViewers();
+  static void EvictAllContentViewersGlobally();
 
   // Calculates a max number of total
   // content viewers to cache, based on amount of total memory
   static PRUint32 CalcMaxTotalViewers();
 
 protected:
   nsCOMPtr<nsISHTransaction> mListRoot;
   PRInt32 mIndex;
--- a/docshell/test/chrome/Makefile.in
+++ b/docshell/test/chrome/Makefile.in
@@ -66,16 +66,18 @@ include $(topsrcdir)/config/rules.mk
 		bug215405_window.xul \
 		test_bug364461.xul \
 		bug364461_window.xul \
 		test_bug396519.xul \
 		bug396519_window.xul \
 		test_bug428288.html \
 		test_bug449778.xul \
 		bug449778_window.xul \
+		test_bug449780.xul \
+		bug449780_window.xul \
 		test_bug454235.xul \
 		bug454235-subframe.xul \
 		test_bug456980.xul \
 		$(NULL)
 
 libs:: $(_HTTP_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
 
new file mode 100644
--- /dev/null
+++ b/docshell/test/chrome/bug449780_window.xul
@@ -0,0 +1,78 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<window title="Mozilla Bug 449780" onload="doTheTest()"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+
+  <hbox id="parent">
+  </hbox>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+    var imports = [ "SimpleTest", "is", "isnot", "ok", "snapshotWindow",
+                    "compareSnapshots", "onerror" ];
+    for each (var import in imports) {
+      window[import] = window.opener.wrappedJSObject[import];
+    }
+
+    function $(id) {
+      return document.getElementById(id);
+    }
+
+    function addBrowser(parent, id, width, height) {
+      var b =
+        document.createElementNS("http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul", "browser");
+      b.setAttribute("type", "content");
+      b.setAttribute("id", id);
+      b.setAttribute("width", width);
+      b.setAttribute("height", height);
+      $(parent).appendChild(b);
+    }
+    addBrowser("parent", "f1", 300, 200);
+    addBrowser("parent", "f2", 300, 200);
+
+    /** Test for Bug 449780 **/
+    var doc1 = "data:text/html,<html><body>This is a test</body></html>";
+    var doc2 = "data:text/html,<html><body>This is a second test</body></html>";
+
+    function getDOM(id) {
+      return $(id).contentDocument.documentElement.innerHTML;
+    }
+
+    var tester = (function() {
+      var origDOM = getDOM("f1");
+      $("f1").contentDocument.body.textContent = "Modified";
+      var modifiedDOM = getDOM("f1");
+      isnot(origDOM, modifiedDOM, "DOM should be different");
+      $("f1").contentWindow.location.href = doc2;
+      yield;
+
+      $("f1").goBack();
+      yield;
+
+      is(getDOM("f1"), modifiedDOM, "Should have been bfcached");
+      $("f1").goForward();
+      yield;
+
+      // Ignore the notifications during swap
+      $("f1").removeEventListener("pageshow", testDriver, false);
+      $("f1").swapDocShells($("f2"));
+      $("f2").addEventListener("pageshow", testDriver, false);
+      $("f2").goBack();
+      yield;
+
+      is(getDOM("f2"), origDOM, "Should have not have been bfcached");
+      window.close();
+      SimpleTest.finish();
+      yield;
+    })();
+
+    function testDriver() {
+      setTimeout(function() { tester.next() }, 0);
+    }
+
+    function doTheTest() {
+      $("f1").addEventListener("pageshow", testDriver, false);
+      $("f1").setAttribute("src", doc1);
+    }
+  ]]></script>
+</window>
new file mode 100644
--- /dev/null
+++ b/docshell/test/chrome/test_bug449780.xul
@@ -0,0 +1,33 @@
+<?xml version="1.0"?>
+<?xml-stylesheet href="chrome://global/skin" type="text/css"?>
+<?xml-stylesheet href="chrome://mochikit/content/tests/SimpleTest/test.css"
+                 type="text/css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=449780
+-->
+<window title="Mozilla Bug 449780"
+  xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript"
+          src="chrome://mochikit/content/MochiKit/packed.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/WindowSnapshot.js"></script>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=449780"
+     target="_blank">Mozilla Bug 396519</a>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript"><![CDATA[
+    SimpleTest.waitForExplicitFinish();
+
+    addLoadEvent(function() {
+      window.open("bug449780_window.xul", "bug449780",
+                  "chrome,width=800,height=800");
+    });
+
+  ]]></script>
+</window>