Bug 774633 - Remove "is chrome window" condition for inner window reuse. r=jst
authorBobby Holley <bobbyholley@gmail.com>
Thu, 23 Aug 2012 16:44:53 -0700
changeset 105281 d4d36e3b948f7b8527247433c2e1effb2de71016
parent 105280 35334e8206322bbb4a591c02ee680c98ab5b2d8c
child 105282 036eb8c2a08afd909c05101c0fccc92dee2d15bc
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersjst
bugs774633
milestone17.0a1
Bug 774633 - Remove "is chrome window" condition for inner window reuse. r=jst WouldReuseInnerWindow also returns true if the new window is same-origin with the old one about:blank document. This condition exists in order to handle some sloppiness with respect to the principals on initial about:blank documents. Chrome callers sometimes parent chrome windows (with XUL document) to content windows. But this parenting causes us to push the cx of the content window during window creation, meaning that the subsequent load of chrome://foo.xul blows away the old inner window and any expandos on it. We can handle this case more precisely by skipping the cx push for type="chrome" windows. Furthermore, this was also necessary to prevent the inner window from being blown away in the call to SetOpenerScriptPrincipal once nsWindowWatcher gets the window back from the window creator (and after it's already told consumers about the window via "domwindowcreated"). But we fixed this nastiness in the previous patches. So we can remove this case. By doing so, we can prevent inner windows from ever changing origins, which is very important for compartment security invariants.
dom/base/nsGlobalWindow.cpp
embedding/components/windowwatcher/src/nsWindowWatcher.cpp
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -1459,58 +1459,46 @@ nsGlobalWindow::GetGlobalJSObject()
 }
 
 bool
 nsGlobalWindow::WouldReuseInnerWindow(nsIDocument *aNewDocument)
 {
   // We reuse the inner window when:
   // a. We are currently at our original document.
   // 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 document has the same origin as what we have loaded right now.
 
   if (!mDoc || !aNewDocument) {
     return false;
   }
 
   if (!mDoc->IsInitialDocument()) {
     return false;
   }
   
   NS_ASSERTION(NS_IsAboutBlank(mDoc->GetDocumentURI()),
                "How'd this happen?");
-  
+
   // Great, we're the original document, check for one of the other
   // conditions.
+
   if (mDoc == aNewDocument) {
     return true;
   }
 
   bool equal;
   if (NS_SUCCEEDED(mDoc->NodePrincipal()->Equals(aNewDocument->NodePrincipal(),
                                                  &equal)) &&
       equal) {
     // The origin is the same.
     return true;
   }
 
-  nsCOMPtr<nsIDocShellTreeItem> treeItem(do_QueryInterface(mDocShell));
-
-  if (treeItem) {
-    int32_t 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 false;
 }
 
 void
 nsGlobalWindow::SetInitialPrincipalToSubject()
 {
   FORWARD_TO_OUTER_VOID(SetInitialPrincipalToSubject, ());
 
--- a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/src/nsWindowWatcher.cpp
@@ -581,21 +581,31 @@ nsWindowWatcher::OpenWindowInternal(nsID
   bool isCallerChrome = true;
   if (callerPrincipal) {
     rv = sm->IsSystemPrincipal(callerPrincipal, &isCallerChrome);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   JSContext *cx = GetJSContextFromWindow(aParent);
 
-  if (isCallerChrome && !hasChromeParent && cx) {
-    // open() is called from chrome on a non-chrome window, push
-    // the context of the callee onto the context stack to
-    // prevent the caller's priveleges from leaking into code
-    // that runs while opening the new window.
+  bool windowTypeIsChrome = chromeFlags & nsIWebBrowserChrome::CHROME_OPENAS_CHROME;
+  if (isCallerChrome && !hasChromeParent && !windowTypeIsChrome && cx) {
+    // open() is called from chrome on a non-chrome window, push the context of the
+    // callee onto the context stack to prevent the caller's priveleges from leaking
+    // into code that runs while opening the new window.
+    //
+    // The reasoning for this is in bug 289204. Basically, chrome sometimes does
+    // someContentWindow.open(untrustedURL), and wants to be insulated from nasty
+    // javascript: URLs and such. But there are also cases where we create a
+    // window parented to a content window (such as a download dialog), usually
+    // directly with nsIWindowWatcher. In those cases, we want the principal of
+    // the initial about:blank document to be system, so that the subsequent XUL
+    // load can reuse the inner window and avoid blowing away expandos. As such,
+    // we decide whether to load with the principal of the caller or of the parent
+    // based on whether the docshell type is chrome or content.
 
     callerContextGuard.Push(cx);
   }
 
   if (!newDocShellItem) {
     // We're going to either open up a new window ourselves or ask a
     // nsIWindowProvider for one.  In either case, we'll want to set the right
     // name on it.