Bug 766282 - Implement allow-popups directive for iframe sandbox. r=smaug
authorBob Owen <bobowencode@gmail.com>
Thu, 21 Mar 2013 18:44:58 +0000
changeset 165576 fce96290655d5370baeaba311f9b0318bdbcbad5
parent 165575 699f5c64f951a88bb83bdde7bb9ba8fa34e57155
child 165577 12f30ee31b2eb78ccfbe88f09a8af4501ea097a9
push id428
push userbbajaj@mozilla.com
push dateTue, 28 Jan 2014 00:16:25 +0000
treeherdermozilla-release@cd72a7ff3a75 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs766282
milestone27.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 766282 - Implement allow-popups directive for iframe sandbox. r=smaug
content/base/src/nsContentUtils.cpp
content/base/src/nsGkAtomList.h
content/base/src/nsSandboxFlags.h
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
docshell/base/nsIDocShell.idl
embedding/components/windowwatcher/src/nsWindowWatcher.cpp
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -919,16 +919,17 @@ nsContentUtils::GetParserService()
  * @return              the set of flags
  */
 uint32_t
 nsContentUtils::ParseSandboxAttributeToFlags(const nsAString& aSandboxAttrValue)
 {
   // If there's a sandbox attribute at all (and there is if this is being
   // called), start off by setting all the restriction flags.
   uint32_t out = SANDBOXED_NAVIGATION |
+                 SANDBOXED_AUXILIARY_NAVIGATION |
                  SANDBOXED_TOPLEVEL_NAVIGATION |
                  SANDBOXED_PLUGINS |
                  SANDBOXED_ORIGIN |
                  SANDBOXED_FORMS |
                  SANDBOXED_SCRIPTS |
                  SANDBOXED_AUTOMATIC_FEATURES |
                  SANDBOXED_POINTER_LOCK |
                  SANDBOXED_DOMAIN;
@@ -949,16 +950,18 @@ nsContentUtils::ParseSandboxAttributeToF
         // allow-scripts removes both SANDBOXED_SCRIPTS and
         // SANDBOXED_AUTOMATIC_FEATURES.
         out &= ~SANDBOXED_SCRIPTS;
         out &= ~SANDBOXED_AUTOMATIC_FEATURES;
       } else if (token.LowerCaseEqualsLiteral("allow-top-navigation")) {
         out &= ~SANDBOXED_TOPLEVEL_NAVIGATION;
       } else if (token.LowerCaseEqualsLiteral("allow-pointer-lock")) {
         out &= ~SANDBOXED_POINTER_LOCK;
+      } else if (token.LowerCaseEqualsLiteral("allow-popups")) {
+        out &= ~SANDBOXED_AUXILIARY_NAVIGATION;
       }
     }
   }
 
   return out;
 }
 
 #ifdef IBMBIDI
--- a/content/base/src/nsGkAtomList.h
+++ b/content/base/src/nsGkAtomList.h
@@ -67,22 +67,18 @@ GK_ATOM(actuate, "actuate")
 GK_ATOM(address, "address")
 GK_ATOM(after, "after")
 GK_ATOM(after_end, "after_end")
 GK_ATOM(after_start, "after_start")
 GK_ATOM(align, "align")
 GK_ATOM(alink, "alink")
 GK_ATOM(all, "all")
 GK_ATOM(allowevents, "allowevents")
-GK_ATOM(allowforms, "allow-forms")
 GK_ATOM(allownegativeassertions, "allownegativeassertions")
 GK_ATOM(allowfullscreen, "allowfullscreen")
-GK_ATOM(allowsameorigin, "allow-same-origin")
-GK_ATOM(allowscripts, "allow-scripts")
-GK_ATOM(allowtopnavigation, "allow-top-navigation")
 GK_ATOM(allowuntrusted, "allowuntrusted")
 GK_ATOM(alt, "alt")
 GK_ATOM(alternate, "alternate")
 GK_ATOM(always, "always")
 GK_ATOM(ancestor, "ancestor")
 GK_ATOM(ancestorOrSelf, "ancestor-or-self")
 GK_ATOM(_and, "and")
 GK_ATOM(any, "any")
--- a/content/base/src/nsSandboxFlags.h
+++ b/content/base/src/nsSandboxFlags.h
@@ -8,18 +8,21 @@
  * HTML5 spec.
  */
 
 #ifndef nsSandboxFlags_h___
 #define nsSandboxFlags_h___
 
 /**
  * This flag prevents content from navigating browsing contexts other than
- * the sandboxed browsing context itself (or browsing contexts further
- * nested inside it), and the top-level browsing context.
+ * itself, browsing contexts nested inside it, the top-level browsing context
+ * and browsing contexts that it has opened.
+ * As it is always on for sandboxed browsing contexts, it is used implicitly
+ * within the code by checking that the overall flags are non-zero.
+ * It is only uesd directly when the sandbox flags are initially set up.
  */
 const unsigned long SANDBOXED_NAVIGATION  = 0x1;
 
 /**
  * This flag prevents content from navigating their top-level browsing
  * context.
  */
 const unsigned long SANDBOXED_TOPLEVEL_NAVIGATION = 0x2;
@@ -60,9 +63,16 @@ const unsigned long SANDBOXED_AUTOMATIC_
  * This flag blocks the document from acquiring pointerlock.
  */
 const unsigned long SANDBOXED_POINTER_LOCK = 0x80;
 
 /**
  * This flag blocks the document from changing document.domain.
  */
 const unsigned long SANDBOXED_DOMAIN = 0x100;
+
+/**
+ * This flag prevents content from creating new auxiliary browsing contexts,
+ * e.g. using the target attribute, the window.open() method, or the
+ * showModalDialog() method.
+ */
+const unsigned long SANDBOXED_AUXILIARY_NAVIGATION = 0x200;
 #endif
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3185,72 +3185,21 @@ nsDocShell::FindItemWithName(const PRUni
 
         if (foundItem && !CanAccessItem(foundItem, aOriginalRequestor)) {
             foundItem = nullptr;
         }
 
         // DoFindItemWithName only returns active items and we don't check if
         // the item is active for the special cases.
         if (foundItem) {
-
-            // If our document is sandboxed, we need to do some extra checks.
-            uint32_t sandboxFlags = 0;
-
-            nsCOMPtr<nsIDocument> doc = do_GetInterface(aOriginalRequestor);
-
-            if (doc) {
-                sandboxFlags = doc->GetSandboxFlags();
+            if (IsSandboxedFrom(foundItem, aOriginalRequestor)) {
+                return NS_ERROR_DOM_INVALID_ACCESS_ERR;
+            } else {
+                foundItem.swap(*_retval);
             }
-
-            if (sandboxFlags) {
-                nsCOMPtr<nsIDocShellTreeItem> root;
-                GetSameTypeRootTreeItem(getter_AddRefs(root));
-
-                // Is the found item not a top level browsing context and not ourself ?
-                nsCOMPtr<nsIDocShellTreeItem> selfAsItem = static_cast<nsIDocShellTreeItem *>(this);
-                if (foundItem != root && foundItem != selfAsItem) {
-                    // Are we an ancestor of the foundItem ?
-                    bool isAncestor = false;
-
-                    nsCOMPtr<nsIDocShellTreeItem> parentAsItem;
-                    foundItem->GetSameTypeParent(getter_AddRefs(parentAsItem));
-                    while (parentAsItem) {
-                        if (parentAsItem == selfAsItem) {
-                            isAncestor = true;
-                            break;
-                        }
-                        nsCOMPtr<nsIDocShellTreeItem> tmp = parentAsItem;
-                        tmp->GetSameTypeParent(getter_AddRefs(parentAsItem));
-                    }
-
-                    if (!isAncestor) {
-                        // No, we are not an ancestor and our document is
-                        // sandboxed, we can't allow this.
-                        foundItem = nullptr;
-                    }
-                } else {
-                    // Top level browsing context - is it an ancestor of ours ?
-                    nsCOMPtr<nsIDocShellTreeItem> tmp;
-                    GetSameTypeParent(getter_AddRefs(tmp));
-
-                    while (tmp) {
-                        if (tmp && tmp == foundItem) {
-                            // This is an ancestor, and we are sandboxed.
-                            // Unless allow-top-navigation is set, we can't allow this.
-                            if (sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION) {
-                                foundItem = nullptr;
-                            }
-                            break;
-                        }
-                        tmp->GetParent(getter_AddRefs(tmp));
-                    }
-                }
-            }
-
-            foundItem.swap(*_retval);
         }
         return NS_OK;
     }
 }
 
 nsresult
 nsDocShell::DoFindItemWithName(const PRUnichar* aName,
                                nsISupports* aRequestor,
@@ -3313,16 +3262,80 @@ nsDocShell::DoFindItemWithName(const PRU
     if (mTreeOwner && mTreeOwner != reqAsTreeOwner) {
         return mTreeOwner->
             FindItemWithName(aName, this, aOriginalRequestor, _retval);
     }
 
     return NS_OK;
 }
 
+/* static */
+bool
+nsDocShell::IsSandboxedFrom(nsIDocShellTreeItem* aTargetItem,
+                            nsIDocShellTreeItem* aAccessingItem)
+{
+    // aAccessingItem cannot be sandboxed from itself.
+    if (SameCOMIdentity(aTargetItem, aAccessingItem)) {
+        return false;
+    }
+
+    uint32_t sandboxFlags = 0;
+
+    nsCOMPtr<nsIDocument> doc = do_GetInterface(aAccessingItem);
+    if (doc) {
+        sandboxFlags = doc->GetSandboxFlags();
+    }
+
+    // If no flags, aAccessingItem is not sandboxed at all.
+    if (!sandboxFlags) {
+        return false;
+    }
+
+    // If aTargetItem has an ancestor, it is not top level.
+    nsCOMPtr<nsIDocShellTreeItem> ancestorOfTarget;
+    aTargetItem->GetSameTypeParent(getter_AddRefs(ancestorOfTarget));
+    if (ancestorOfTarget) {
+        do {
+            // aAccessingItem is not sandboxed if it is an ancestor of target.
+            if (SameCOMIdentity(aAccessingItem, ancestorOfTarget)) {
+                return false;
+            }
+            nsCOMPtr<nsIDocShellTreeItem> tempTreeItem;
+            ancestorOfTarget->GetSameTypeParent(getter_AddRefs(tempTreeItem));
+            tempTreeItem.swap(ancestorOfTarget);
+        } while (ancestorOfTarget);
+
+        // Otherwise, aAccessingItem is sandboxed from aTargetItem.
+        return true;
+    }
+
+    // aTargetItem is top level, is aAccessingItem the "one permitted sandboxed
+    // navigator", i.e. did aAccessingItem open aTargetItem?
+    nsCOMPtr<nsIDocShell> targetDocShell = do_QueryInterface(aTargetItem);
+    nsCOMPtr<nsIDocShell> permittedNavigator;
+    targetDocShell->
+        GetOnePermittedSandboxedNavigator(getter_AddRefs(permittedNavigator));
+    if (SameCOMIdentity(aAccessingItem, permittedNavigator)) {
+        return false;
+    }
+
+    // If SANDBOXED_TOPLEVEL_NAVIGATION flag is not on, aAccessingItem is
+    // not sandboxed from its top.
+    if (!(sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION)) {
+        nsCOMPtr<nsIDocShellTreeItem> rootTreeItem;
+        aAccessingItem->GetSameTypeRootTreeItem(getter_AddRefs(rootTreeItem));
+        if (SameCOMIdentity(aTargetItem, rootTreeItem)) {
+            return false;
+        }
+    }
+
+    // Otherwise, aAccessingItem is sandboxed from aTargetItem.
+    return true;
+}
+
 NS_IMETHODIMP
 nsDocShell::GetTreeOwner(nsIDocShellTreeOwner ** aTreeOwner)
 {
     NS_ENSURE_ARG_POINTER(aTreeOwner);
 
     *aTreeOwner = mTreeOwner;
     NS_IF_ADDREF(*aTreeOwner);
     return NS_OK;
@@ -5050,16 +5063,18 @@ nsDocShell::Destroy()
         if (shPrivate) {
             shPrivate->EvictAllContentViewers();
         }
         mSessionHistory = nullptr;
     }
 
     SetTreeOwner(nullptr);
 
+    mOnePermittedSandboxedNavigator = nullptr;
+
     // required to break ref cycle
     mSecurityUI = nullptr;
 
     // Cancel any timers that were set for this docshell; this is needed
     // to break the cycle between us and the timers.
     CancelRefreshURITimers();
 
     if (mInPrivateBrowsing) {
@@ -5399,16 +5414,41 @@ nsDocShell::SetSandboxFlags(uint32_t aSa
 NS_IMETHODIMP
 nsDocShell::GetSandboxFlags(uint32_t  *aSandboxFlags)
 {
     *aSandboxFlags = mSandboxFlags;
     return NS_OK;
 }
 
 NS_IMETHODIMP
+nsDocShell::SetOnePermittedSandboxedNavigator(nsIDocShell* aSandboxedNavigator)
+{
+    if (mOnePermittedSandboxedNavigator) {
+        NS_ERROR("One Permitted Sandboxed Navigator should only be set once.");
+        return NS_OK;
+    }
+
+    mOnePermittedSandboxedNavigator = do_GetWeakReference(aSandboxedNavigator);
+    NS_ASSERTION(mOnePermittedSandboxedNavigator,
+             "One Permitted Sandboxed Navigator must support weak references.");
+
+    return NS_OK;
+}
+
+NS_IMETHODIMP
+nsDocShell::GetOnePermittedSandboxedNavigator(nsIDocShell** aSandboxedNavigator)
+{
+    NS_ENSURE_ARG_POINTER(aSandboxedNavigator);
+    nsCOMPtr<nsIDocShell> permittedNavigator =
+        do_QueryReferent(mOnePermittedSandboxedNavigator);
+    NS_IF_ADDREF(*aSandboxedNavigator = permittedNavigator);
+    return NS_OK;
+}
+
+NS_IMETHODIMP
 nsDocShell::SetDefaultLoadFlags(uint32_t aDefaultLoadFlags)
 {
     mDefaultLoadFlags = aDefaultLoadFlags;
 
     // Tell the load group to set these flags all requests in the group
     if (mLoadGroup) {
         mLoadGroup->SetDefaultLoadFlags(aDefaultLoadFlags);
     } else {
@@ -8714,18 +8754,20 @@ nsDocShell::InternalLoad(nsIURI * aURI,
     int16_t shouldLoad = nsIContentPolicy::ACCEPT;
     uint32_t contentType;
     bool isNewDocShell = false;
     bool isTargetTopLevelDocShell = false;
     nsCOMPtr<nsIDocShell> targetDocShell;
     if (aWindowTarget && *aWindowTarget) {
         // Locate the target DocShell.
         nsCOMPtr<nsIDocShellTreeItem> targetItem;
-        FindItemWithName(aWindowTarget, nullptr, this,
-                         getter_AddRefs(targetItem));
+        if (FindItemWithName(aWindowTarget, nullptr, this,
+               getter_AddRefs(targetItem)) == NS_ERROR_DOM_INVALID_ACCESS_ERR) {
+            return NS_ERROR_DOM_INVALID_ACCESS_ERR;
+        }
 
         targetDocShell = do_QueryInterface(targetItem);
         // If the targetDocShell doesn't exist, then this is a new docShell
         // and we should consider this a TYPE_DOCUMENT load
         isNewDocShell = !targetDocShell;
 
         // If the targetDocShell and the rootDocShell are the same, then the
         // targetDocShell is the top level document and hence we should
@@ -8842,29 +8884,27 @@ nsDocShell::InternalLoad(nsIURI * aURI,
     if (aWindowTarget && *aWindowTarget) {
         // We've already done our owner-inheriting.  Mask out that bit, so we
         // don't try inheriting an owner from the target window if we came up
         // with a null owner above.
         aFlags = aFlags & ~INTERNAL_LOAD_FLAGS_INHERIT_OWNER;
         
         bool isNewWindow = false;
         if (!targetDocShell) {
-            // If the docshell's document is sandboxed and was trying to
-            // navigate/load a frame it wasn't allowed to access, the
-            // FindItemWithName above will have returned null for the target
-            // item - we don't want to actually open a new window in this case
-            // though. Check if we are sandboxed and bail out here if so.
+            // If the docshell's document is sandboxed, only open a new window
+            // if the document's SANDBOXED_AUXILLARY_NAVIGATION flag is not set.
+            // (i.e. if allow-popups is specified)
             NS_ENSURE_TRUE(mContentViewer, NS_ERROR_FAILURE);
             nsIDocument* doc = mContentViewer->GetDocument();
             uint32_t sandboxFlags = 0;
 
             if (doc) {
                 sandboxFlags = doc->GetSandboxFlags();
-                if (sandboxFlags & SANDBOXED_NAVIGATION) {
-                    return NS_ERROR_FAILURE;
+                if (sandboxFlags & SANDBOXED_AUXILIARY_NAVIGATION) {
+                    return NS_ERROR_DOM_INVALID_ACCESS_ERR;
                 }
             }
 
             nsCOMPtr<nsPIDOMWindow> win =
                 do_GetInterface(GetAsSupports(this));
             NS_ENSURE_TRUE(win, NS_ERROR_NOT_AVAILABLE);
 
             nsDependentString name(aWindowTarget);
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -766,16 +766,17 @@ protected:
     int32_t                    mItemType;
 
     // Index into the SHTransaction list, indicating the previous and current
     // transaction at the time that this DocShell begins to load
     int32_t                    mPreviousTransIndex;
     int32_t                    mLoadedTransIndex;
 
     uint32_t                   mSandboxFlags;
+    nsWeakPtr                  mOnePermittedSandboxedNavigator;
 
     // mFullscreenAllowed stores how we determine whether fullscreen is allowed
     // when GetFullscreenAllowed() is called. Fullscreen is allowed in a
     // docshell when all containing iframes have the allowfullscreen
     // attribute set to true. When mFullscreenAllowed is CHECK_ATTRIBUTES
     // we check this docshell's containing frame for the allowfullscreen
     // attribute, and recurse onto the parent docshell to ensure all containing
     // frames also have the allowfullscreen attribute. If we find an ancestor
@@ -875,16 +876,20 @@ private:
 
     // Separate function to do the actual name (i.e. not _top, _self etc.)
     // searching for FindItemWithName.
     nsresult DoFindItemWithName(const PRUnichar* aName,
                                 nsISupports* aRequestor,
                                 nsIDocShellTreeItem* aOriginalRequestor,
                                 nsIDocShellTreeItem** _retval);
 
+    // Check whether accessing item is sandboxed from the target item.
+    static bool IsSandboxedFrom(nsIDocShellTreeItem* aTargetItem,
+                                nsIDocShellTreeItem* aAccessingItem);
+
 #ifdef DEBUG
     // We're counting the number of |nsDocShells| to help find leaks
     static unsigned long gNumberOfDocShells;
 #endif /* DEBUG */
 
 public:
     class InterfaceRequestorProxy : public nsIInterfaceRequestor {
     public:
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -38,17 +38,17 @@ interface nsIDOMStorage;
 interface nsIPrincipal;
 interface nsIWebBrowserPrint;
 interface nsIVariant;
 interface nsIPrivacyTransitionObserver;
 interface nsIReflowObserver;
 
 typedef unsigned long nsLoadFlags;
 
-[scriptable, builtinclass, uuid(5f4d82fc-3220-4f7e-9b00-626f1033318a)]
+[scriptable, builtinclass, uuid(4ca172c3-67bf-4e6d-89a3-cbfb929c370d)]
 interface nsIDocShell : nsIDocShellTreeItem
 {
   /**
    * Loads a given URI.  This will give priority to loading the requested URI
    * in the object implementing	this interface.  If it can't be loaded here
    * however, the URL dispatcher will go through its normal process of content
    * loading.
    *
@@ -794,16 +794,23 @@ interface nsIDocShell : nsIDocShellTreeI
    * loaded.
    * The sandbox flags of a document depend on the sandbox flags on its
    * docshell and of its parent document, if any.
    * See nsSandboxFlags.h for the possible flags.
    */
   attribute unsigned long sandboxFlags;
 
   /**
+   * When a new browsing context is opened by a sandboxed document, it needs to
+   * keep track of the browsing context that opened it, so that it can be
+   * navigated by it.  This is the "one permitted sandboxed navigator".
+   */
+  attribute nsIDocShell onePermittedSandboxedNavigator;
+
+  /**
    * This member variable determines whether a document has Mixed Active Content that
    * was initially blocked from loading, but the user has choosen to override the
    * block and allow the content to load. mMixedContentChannel is set to the document's
    * channel when the user allows mixed content. The nsMixedContentBlocker content policy
    * checks if the document's root channel matches the mMixedContentChannel.  If it matches,
    * then Mixed Content is loaded.  If it does match, mixed content is blocked.
    *
    * A match implies that there is definitely mixed active content on a page that was
--- a/embedding/components/windowwatcher/src/nsWindowWatcher.cpp
+++ b/embedding/components/windowwatcher/src/nsWindowWatcher.cpp
@@ -470,17 +470,20 @@ nsWindowWatcher::OpenWindowInternal(nsID
   if (aFeatures) {
     features.Assign(aFeatures);
     featuresSpecified = true;
     features.StripWhitespace();
   }
 
   // try to find an extant window with the given name
   nsCOMPtr<nsIDOMWindow> foundWindow;
-  SafeGetWindowByName(name, aParent, getter_AddRefs(foundWindow));
+  if (SafeGetWindowByName(name, aParent, getter_AddRefs(foundWindow)) ==
+      NS_ERROR_DOM_INVALID_ACCESS_ERR) {
+    return NS_ERROR_DOM_INVALID_ACCESS_ERR;
+  }
   GetWindowTreeItem(foundWindow, getter_AddRefs(newDocShellItem));
 
   // no extant window? make a new one.
 
   // If no parent, consider it chrome.
   bool hasChromeParent = true;
   if (aParent) {
     // Check if the parent document has chrome privileges.
@@ -538,31 +541,36 @@ nsWindowWatcher::OpenWindowInternal(nsID
     // 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);
   }
 
+  uint32_t activeDocsSandboxFlags = 0;
   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.
     windowNeedsName = true;
 
-    // If the parent trying to open a new window is sandboxed,
-    // this is not allowed and we fail here.
+    // If the parent trying to open a new window is sandboxed
+    // without 'allow-popups', this is not allowed and we fail here.
     if (aParent) {
       nsCOMPtr<nsIDOMDocument> domdoc;
       aParent->GetDocument(getter_AddRefs(domdoc));
       nsCOMPtr<nsIDocument> doc = do_QueryInterface(domdoc);
 
-      if (doc && (doc->GetSandboxFlags() & SANDBOXED_NAVIGATION)) {
-        return NS_ERROR_FAILURE;
+      if (doc) {
+        // Save sandbox flags for copying to new browsing context (docShell).
+        activeDocsSandboxFlags = doc->GetSandboxFlags();
+        if (activeDocsSandboxFlags & SANDBOXED_AUXILIARY_NAVIGATION) {
+          return NS_ERROR_DOM_INVALID_ACCESS_ERR;
+        }
       }
     }
 
     // Now check whether it's ok to ask a window provider for a window.  Don't
     // do it if we're opening a dialog or if our parent is a chrome window or
     // if we're opening something that has modal, dialog, or chrome flags set.
     nsCOMPtr<nsIDOMChromeWindow> chromeWin = do_QueryInterface(aParent);
     if (!aDialog && !chromeWin &&
@@ -704,16 +712,26 @@ nsWindowWatcher::OpenWindowInternal(nsID
   }
 
   // better have a window to use by this point
   if (!newDocShellItem)
     return rv;
 
   nsCOMPtr<nsIDocShell> newDocShell(do_QueryInterface(newDocShellItem));
   NS_ENSURE_TRUE(newDocShell, NS_ERROR_UNEXPECTED);
+
+  // Set up sandboxing attributes if the window is new.
+  // The flags can only be non-zero for new windows.
+  if (activeDocsSandboxFlags != 0) {
+    newDocShell->SetSandboxFlags(activeDocsSandboxFlags);
+    nsCOMPtr<nsPIDOMWindow> window = do_QueryInterface(aParent);
+    if (window) {
+      newDocShell->SetOnePermittedSandboxedNavigator(window->GetDocShell());
+    }
+  }
   
   rv = ReadyOpenedDocShellItem(newDocShellItem, aParent, windowIsNew, _retval);
   if (NS_FAILED(rv))
     return rv;
 
   /* disable persistence of size/position in popups (determined by
      determining whether the features parameter specifies width or height
      in any way). We consider any overriding of the window's size or position
@@ -1294,37 +1312,38 @@ nsWindowWatcher::GetChromeForWindow(nsID
 NS_IMETHODIMP
 nsWindowWatcher::GetWindowByName(const PRUnichar *aTargetName, 
                                  nsIDOMWindow *aCurrentWindow,
                                  nsIDOMWindow **aResult)
 {
   if (!aResult) {
     return NS_ERROR_INVALID_ARG;
   }
+  nsresult rv;
 
   *aResult = nullptr;
 
   nsCOMPtr<nsIDocShellTreeItem> treeItem;
 
   nsCOMPtr<nsIDocShellTreeItem> startItem;
   GetWindowTreeItem(aCurrentWindow, getter_AddRefs(startItem));
   if (startItem) {
     // Note: original requestor is null here, per idl comments
-    startItem->FindItemWithName(aTargetName, nullptr, nullptr,
+    rv = startItem->FindItemWithName(aTargetName, nullptr, nullptr,
                                 getter_AddRefs(treeItem));
   }
   else {
     // Note: original requestor is null here, per idl comments
-    FindItemWithName(aTargetName, nullptr, nullptr, getter_AddRefs(treeItem));
+    rv = FindItemWithName(aTargetName, nullptr, nullptr, getter_AddRefs(treeItem));
   }
 
   nsCOMPtr<nsIDOMWindow> domWindow = do_GetInterface(treeItem);
   domWindow.swap(*aResult);
 
-  return NS_OK;
+  return rv;
 }
 
 bool
 nsWindowWatcher::AddEnumerator(nsWatcherWindowEnumerator* inEnumerator)
 {
   // (requires a lock; assumes it's called by someone holding the lock)
   return mEnumeratorList.AppendElement(inEnumerator) != nullptr;
 }
@@ -1726,37 +1745,38 @@ nsWindowWatcher::GetCallerTreeItem(nsIDo
 }
 
 nsresult
 nsWindowWatcher::SafeGetWindowByName(const nsAString& aName,
                                      nsIDOMWindow* aCurrentWindow,
                                      nsIDOMWindow** aResult)
 {
   *aResult = nullptr;
+  nsresult rv;
   
   nsCOMPtr<nsIDocShellTreeItem> startItem;
   GetWindowTreeItem(aCurrentWindow, getter_AddRefs(startItem));
 
   nsCOMPtr<nsIDocShellTreeItem> callerItem = GetCallerTreeItem(startItem);
 
   const nsAFlatString& flatName = PromiseFlatString(aName);
 
   nsCOMPtr<nsIDocShellTreeItem> foundItem;
   if (startItem) {
-    startItem->FindItemWithName(flatName.get(), nullptr, callerItem,
+    rv = startItem->FindItemWithName(flatName.get(), nullptr, callerItem,
                                 getter_AddRefs(foundItem));
   }
   else {
-    FindItemWithName(flatName.get(), nullptr, callerItem,
+    rv = FindItemWithName(flatName.get(), nullptr, callerItem,
                      getter_AddRefs(foundItem));
   }
 
   nsCOMPtr<nsIDOMWindow> foundWin = do_GetInterface(foundItem);
   foundWin.swap(*aResult);
-  return NS_OK;
+  return rv;
 }
 
 /* Fetch the nsIDOMWindow corresponding to the given nsIDocShellTreeItem.
    This forces the creation of a script context, if one has not already
    been created. Note it also sets the window's opener to the parent,
    if applicable -- because it's just convenient, that's all. null aParent
    is acceptable. */
 nsresult