bug 305236: Don't store the current document in the bfcache if we're going to reuse the current inner window for another page. In this case, the current document never was making it into session history anyway. r=bryner sr=jst

docshell/base/nsDocShell.cpp

@@ -4810,7 +4810,9 @@ nsDocShell::CreateAboutBlankContentViewer()
 }
 
 PRBool
-nsDocShell::CanSavePresentation(PRUint32 aLoadType, nsIRequest *aNewRequest)
+nsDocShell::CanSavePresentation(PRUint32 aLoadType,
+                                nsIRequest *aNewRequest,
+                                nsIDocument *aNewDocument)
 {
     if (!mOSHE)
         return PR_FALSE; // no entry to save into
@@ -4838,6 +4840,9 @@ nsDocShell::CanSavePresentation(PRUint32 aLoadType, nsIRequest *aNewRequest)
     if (!pWin || pWin->IsLoading())
         return PR_FALSE;
 
+    if (pWin->WouldReuseInnerWindow(aNewDocument))
+        return PR_FALSE;
+
     // Avoid doing the work of saving the presentation state in the case where
     // the content viewer cache is disabled.
     PRInt32 maxViewers = 0;
@@ -5166,7 +5171,7 @@ nsDocShell::RestoreFromHistory()
         nsIRequest *request = nsnull;
         if (doc)
             request = doc->GetChannel();
-        mSavingOldViewer = CanSavePresentation(mLoadType, request);
+        mSavingOldViewer = CanSavePresentation(mLoadType, request, doc);
     }
 
     nsCOMPtr<nsIMarkupDocumentViewer> oldMUDV(do_QueryInterface(mContentViewer));
@@ -5444,7 +5449,10 @@ nsDocShell::CreateContentViewer(const char *aContentType,
         // at the time we initiated the new load.  We need to check whether
         // it's still safe to do so, since there may have been DOM mutations
         // or new requests initiated.
-        mSavingOldViewer = CanSavePresentation(mLoadType, request);
+        nsCOMPtr<nsIDOMDocument> domDoc;
+        viewer->GetDOMDocument(getter_AddRefs(domDoc));
+        nsCOMPtr<nsIDocument> doc = do_QueryInterface(domDoc);
+        mSavingOldViewer = CanSavePresentation(mLoadType, request, doc);
     }
 
     FirePageHideNotification(!mSavingOldViewer);
@@ -6352,7 +6360,9 @@ nsDocShell::InternalLoad(nsIURI * aURI,
     // This is necessary so that we can catch any pending requests.
     // Since the new request has not been created yet, we pass null for the
     // new request parameter.
-    PRBool savePresentation = CanSavePresentation(aLoadType, nsnull);
+    // Also pass nsnull for the document, since it doesn't affect the return
+    // value for our purposes here.
+    PRBool savePresentation = CanSavePresentation(aLoadType, nsnull, nsnull);
 
     // Don't stop current network activity for javascript: URL's since
     // they might not result in any data, and thus nothing should be

docshell/base/nsDocShell.h

@@ -1,4 +1,5 @@
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*-
+ * vim: set ts=4 sw=4 et tw=80:
  *
  * ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
@@ -491,8 +492,11 @@ protected:
     // |aLoadType| should be the load type that will replace the current
     // presentation.  |aNewRequest| should be the request for the document to
     // be loaded in place of the current document, or null if such a request
-    // has not been created yet.
-    PRBool CanSavePresentation(PRUint32 aLoadType, nsIRequest *aNewRequest);
+    // has not been created yet. |aNewDocument| should be the document that will
+    // replace the current document.
+    PRBool CanSavePresentation(PRUint32 aLoadType,
+                               nsIRequest *aNewRequest,
+                               nsIDocument *aNewDocument);
 
     // Captures the state of the supporting elements of the presentation
     // (the "window" object, docshell tree, meta-refresh loads, and security

dom/public/base/nsPIDOMWindow.h

@@ -1,4 +1,5 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 sw=2 et tw=80: */
 /* ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
  *
@@ -72,11 +73,12 @@ enum OpenAllowValue {
 
 class nsIDocShell;
 class nsIFocusController;
+class nsIDocument;
 struct nsTimeout;
 
 #define NS_PIDOMWINDOW_IID \
-{ 0x26c9769c, 0xecfc, 0x46b9, \
- { 0x9f, 0x62, 0x8e, 0x9b, 0x1a, 0x6c, 0x0d, 0x28 } }
+{ 0x1f9346e6, 0x3814, 0x4c3b, \
+  { 0x9d, 0x04, 0x0b, 0x93, 0x86, 0x53, 0x2b, 0xbf } }
 
 class nsPIDOMWindow : public nsIDOMWindowInternal
 {
@@ -285,6 +287,8 @@ public:
     return !IsInnerWindow();
   }
 
+  virtual PRBool WouldReuseInnerWindow(nsIDocument *aNewDocument) = 0;
+
 protected:
   // The nsPIDOMWindow constructor. The aOuterWindow argument should
   // be null if and only if the created window itself is an outer

dom/src/base/nsGlobalWindow.cpp

@@ -536,6 +536,89 @@ nsGlobalWindow::GetContext()
   return mContext;
 }
 
+PRBool
+nsGlobalWindow::WouldReuseInnerWindow(nsIDocument *aNewDocument)
+{
+  return WouldReuseInnerWindow(aNewDocument, PR_TRUE);
+}
+
+PRBool
+nsGlobalWindow::WouldReuseInnerWindow(nsIDocument *aNewDocument, PRBool useDocURI)
+{
+  // We reuse the inner window when:
+  // a. We are currently at about:blank
+  // b. At least one of the following conditions are true:
+  // -- We are not currently a content window (i.e., we're currently a chrome
+  //    window).
+  // -- The new document is the same as the old document. This means that we're
+  //    getting called from document.open().
+  // -- The new URI has the same origin as the script opener uri for our current
+  //    window.
+
+  nsCOMPtr<nsIDocument> curDoc(do_QueryInterface(mDocument));
+  if (!curDoc || !aNewDocument) {
+    return PR_FALSE;
+  }
+
+  nsCOMPtr<nsIURI> newURI;
+  if (useDocURI) {
+    newURI = aNewDocument->GetDocumentURI();
+  } else {
+    nsCOMPtr<nsIWebNavigation> webNav(do_QueryInterface(mDocShell));
+
+    if (webNav) {
+      webNav->GetCurrentURI(getter_AddRefs(newURI));
+    }
+  }
+
+  nsIURI* curURI = curDoc->GetDocumentURI();
+  if (!curURI || !newURI) {
+    return PR_FALSE;
+  }
+
+  PRBool isAbout;
+  if (NS_FAILED(curURI->SchemeIs("about", &isAbout)) || !isAbout) {
+    return PR_FALSE;
+  }
+
+  nsCAutoString uri;
+  curURI->GetSpec(uri);
+  if (!uri.EqualsLiteral("about:blank")) {
+    return PR_FALSE;
+  }
+  
+  // Great, we're an about:blank document, check for one of the other
+  // conditions.
+  if (curDoc == aNewDocument) {
+    // aClearScopeHint is false.
+    return PR_TRUE;
+  }
+
+  if (mOpenerScriptURL) {
+    if (sSecMan) {
+      PRBool isSameOrigin = PR_FALSE;
+      sSecMan->SecurityCompareURIs(mOpenerScriptURL, newURI, &isSameOrigin);
+      if (isSameOrigin) {
+        // The origin is the same.
+        return PR_TRUE;
+      }
+    }
+  }
+
+  nsCOMPtr<nsIDocShellTreeItem> treeItem(do_QueryInterface(mDocShell));
+
+  if (treeItem) {
+    PRInt32 itemType = nsIDocShellTreeItem::typeContent;
+    treeItem->GetItemType(&itemType);
+
+    // If we're a chrome window, then we want to reuse the inner window.
+    return itemType == nsIDocShellTreeItem::typeChrome;
+  }
+
+  // No treeItem: don't reuse the current inner window.
+  return PR_FALSE;
+}
+
 void
 nsGlobalWindow::SetOpenerScriptURL(nsIURI* aURI)
 {
@@ -793,58 +876,14 @@ nsGlobalWindow::SetNewDocument(nsIDOMDocument* aDocument,
   // we don't ever call SetNewDocument(nsnull), so no need to null
   // check xpc here.
   nsIXPConnect *xpc = nsContentUtils::XPConnect();
-  PRBool reUseInnerWindow = PR_FALSE;
 
-  if (oldDoc) {
-    nsIURI *oldURL = oldDoc->GetDocumentURI();
+  PRBool reUseInnerWindow = WouldReuseInnerWindow(newDoc, PR_FALSE);
 
-    if (oldURL) {
-      nsCOMPtr<nsIDocShellTreeItem> treeItem(do_QueryInterface(mDocShell));
-      PRBool isContentWindow = PR_FALSE;
-
-      if (treeItem) {
-        PRInt32 itemType = nsIDocShellTreeItem::typeContent;
-        treeItem->GetItemType(&itemType);
-
-        isContentWindow = itemType != nsIDocShellTreeItem::typeChrome;
-      }
-
-      PRBool isSameOrigin = PR_FALSE;
-      PRBool isAboutBlank = PR_FALSE;
-      PRBool isAbout;
-      if (NS_SUCCEEDED(oldURL->SchemeIs("about", &isAbout)) && isAbout) {
-        nsCAutoString url;
-        oldURL->GetSpec(url);
-        isAboutBlank = url.EqualsLiteral("about:blank");
-
-        if (isAboutBlank && mOpenerScriptURL) {
-          nsCOMPtr<nsIWebNavigation> webNav(do_QueryInterface(mDocShell));
-          if (webNav) {
-            nsCOMPtr<nsIURI> newURI;
-            webNav->GetCurrentURI(getter_AddRefs(newURI));
-            if (newURI && sSecMan) {
-              // XXXjst: Uh, don't we want to compare the new URI
-              // against the calling URI (JS_GetScopeChain()?) and not
-              // the opener script URL?
-              sSecMan->SecurityCompareURIs(mOpenerScriptURL, newURI,
-                                           &isSameOrigin);
-            }
-          }
-        }
-      }
-
-      reUseInnerWindow =
-        isAboutBlank && (!isContentWindow || !aClearScopeHint ||
-                         isSameOrigin);
-
-      // XXXjst: Remove the aRemoveEventListeners argument, it's
-      // always passed the same value as aClearScopeHint.
-
-      // Don't remove event listeners in similar conditions
-      aRemoveEventListeners = aRemoveEventListeners &&
-        (!isAboutBlank || (isContentWindow && !isSameOrigin));
-    }
-  }
+  // XXX We used to share event listeners between inner windows in special
+  // circumstances (that were remarkably close to the conditions that we set
+  // reUseInnerWindow in) but that left dangling pointers to the old (destroyed)
+  // inner window (bug 303765). Setting this here should be a no-op.
+  aRemoveEventListeners = !reUseInnerWindow;
 
   // Remember the old document's principal.
   nsIPrincipal *oldPrincipal = nsnull;

dom/src/base/nsGlobalWindow.h

@@ -222,6 +222,8 @@ public:
   virtual NS_HIDDEN_(nsresult) SaveWindowState(nsISupports **aState);
   virtual NS_HIDDEN_(nsresult) RestoreWindowState(nsISupports *aState);
 
+  virtual NS_HIDDEN_(PRBool) WouldReuseInnerWindow(nsIDocument *aNewDocument);
+
   // nsIDOMViewCSS
   NS_DECL_NSIDOMVIEWCSS
 
@@ -292,6 +294,8 @@ protected:
                           PRBool aClearScopeHint,
                           PRBool aIsInternalCall);
 
+  PRBool WouldReuseInnerWindow(nsIDocument *aNewDocument, PRBool useDocURI);
+
   // Get the parent, returns null if this is a toplevel window
   nsIDOMWindowInternal *GetParentInternal();