Bug 1562292: Part 1d - Move OnePermittedSandboxedNavigator to BrowsingContext. r=nika
authorKris Maglione <maglione.k@gmail.com>
Thu, 01 Aug 2019 16:22:52 -0700
changeset 488064 ba987e21b5329d07c3606a65ee0cb76e7bc99a74
parent 488063 a50f085eb3e6c506fd8e17acb669ef91b6f31390
child 488065 3bffe760e2db5ff009f0d798cbb63b83a68de51b
push id36435
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:46:49 +0000
treeherdermozilla-central@0db07ff50ab5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1562292
milestone70.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 1562292: Part 1d - Move OnePermittedSandboxedNavigator to BrowsingContext. r=nika We need to be able to check the one-permitted-sandboxed-navigator from potentially-cross-process access checks in DocShell, which means it needs to live on BrowsingContext rather than DocShell. Differential Revision: https://phabricator.services.mozilla.com/D40495
docshell/base/BrowsingContext.h
docshell/base/BrowsingContextFieldList.h
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
docshell/base/nsIDocShell.idl
toolkit/components/windowwatcher/nsWindowWatcher.cpp
--- a/docshell/base/BrowsingContext.h
+++ b/docshell/base/BrowsingContext.h
@@ -204,16 +204,33 @@ class BrowsingContext : public nsWrapper
 
   already_AddRefed<BrowsingContext> GetOpener() const { return Get(mOpenerId); }
   void SetOpener(BrowsingContext* aOpener) {
     SetOpenerId(aOpener ? aOpener->Id() : 0);
   }
 
   bool HasOpener() const;
 
+  /**
+   * 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".
+   */
+  already_AddRefed<BrowsingContext> GetOnePermittedSandboxedNavigator() const {
+    return Get(mOnePermittedSandboxedNavigatorId);
+  }
+  void SetOnePermittedSandboxedNavigator(BrowsingContext* aNavigator) {
+    if (mOnePermittedSandboxedNavigatorId) {
+      MOZ_ASSERT(false,
+                 "One Permitted Sandboxed Navigator should only be set once.");
+    } else {
+      SetOnePermittedSandboxedNavigatorId(aNavigator ? aNavigator->Id() : 0);
+    }
+  }
+
   void GetChildren(Children& aChildren);
 
   BrowsingContextGroup* Group() { return mGroup; }
 
   // Using the rules for choosing a browsing context we try to find
   // the browsing context with the given name in the set of
   // transitively reachable browsing contexts. Performs access control
   // with regards to this.
--- a/docshell/base/BrowsingContextFieldList.h
+++ b/docshell/base/BrowsingContextFieldList.h
@@ -14,13 +14,15 @@ MOZ_BC_FIELD(Name, nsString)
 MOZ_BC_FIELD(Closed, bool)
 MOZ_BC_FIELD(EmbedderPolicy, nsILoadInfo::CrossOriginEmbedderPolicy)
 MOZ_BC_FIELD(OpenerPolicy, nsILoadInfo::CrossOriginOpenerPolicy)
 
 // The current opener for this BrowsingContext. This is a weak reference, and
 // stored as the opener ID.
 MOZ_BC_FIELD(OpenerId, uint64_t)
 
+MOZ_BC_FIELD(OnePermittedSandboxedNavigatorId, uint64_t)
+
 // Toplevel browsing contexts only. This field controls whether the browsing
 // context is currently considered to be activated by a gesture.
 MOZ_BC_FIELD(IsActivatedByUserGesture, bool)
 
 #undef MOZ_BC_FIELD
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -3049,20 +3049,20 @@ bool nsDocShell::IsSandboxedFrom(nsIDocS
     } while (ancestorOfTarget);
 
     // Otherwise, we are sandboxed from aTargetDocShell.
     return true;
   }
 
   // aTargetDocShell is top level, are we the "one permitted sandboxed
   // navigator", i.e. did we open aTargetDocShell?
-  nsCOMPtr<nsIDocShell> permittedNavigator;
-  aTargetDocShell->GetOnePermittedSandboxedNavigator(
-      getter_AddRefs(permittedNavigator));
-  if (permittedNavigator == this) {
+  RefPtr<BrowsingContext> permittedNavigator(
+      aTargetDocShell->GetBrowsingContext()
+          ->GetOnePermittedSandboxedNavigator());
+  if (permittedNavigator == mBrowsingContext) {
     return false;
   }
 
   // If SANDBOXED_TOPLEVEL_NAVIGATION flag is not on, we are not sandboxed
   // from our top.
   if (!(sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION)) {
     nsCOMPtr<nsIDocShellTreeItem> rootTreeItem;
     GetInProcessSameTypeRootTreeItem(getter_AddRefs(rootTreeItem));
@@ -4962,18 +4962,16 @@ nsDocShell::Destroy() {
   }
 
   SetTreeOwner(nullptr);
 
   mBrowserChild = nullptr;
 
   mChromeEventHandler = 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 (UsePrivateBrowsing()) {
@@ -5363,44 +5361,16 @@ 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;
-  }
-
-  MOZ_ASSERT(!mIsBeingDestroyed);
-
-  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);
-  permittedNavigator.forget(aSandboxedNavigator);
-  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 {
     NS_WARNING(
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -1061,17 +1061,16 @@ class nsDocShell final : public nsDocLoa
   // We're counting the number of |nsDocShells| to help find leaks
   static unsigned long gNumberOfDocShells;
 #endif /* DEBUG */
 
   nsID mHistoryID;
   nsString mTitle;
   nsString mCustomUserAgent;
   nsCString mOriginalUriString;
-  nsWeakPtr mOnePermittedSandboxedNavigator;
   nsWeakPtr mOpener;
   nsTObserverArray<nsWeakPtr> mPrivacyObservers;
   nsTObserverArray<nsWeakPtr> mReflowObservers;
   nsTObserverArray<nsWeakPtr> mScrollObservers;
   mozilla::OriginAttributes mOriginAttributes;
   mozilla::UniquePtr<mozilla::dom::ClientSource> mInitialClientSource;
   nsCOMPtr<nsINetworkInterceptController> mInterceptController;
   RefPtr<nsDOMNavigationTiming> mTiming;
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -780,23 +780,16 @@ 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;
-
-  /**
    * Returns true if we are sandboxed from aTargetDocShell.
    * aTargetDocShell - the browsing context we are attempting to navigate.
    */
   [noscript,notxpcom,nostdcall] bool isSandboxedFrom(in nsIDocShell aTargetDocShell);
 
   /**
    * 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
--- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp
+++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp
@@ -917,17 +917,18 @@ nsresult nsWindowWatcher::OpenWindowInte
   }
 
   nsCOMPtr<nsIDocShell> newDocShell(do_QueryInterface(newDocShellItem));
   NS_ENSURE_TRUE(newDocShell, NS_ERROR_UNEXPECTED);
 
   // If our parent is sandboxed, set it as the one permitted sandboxed navigator
   // on the new window we're opening.
   if (activeDocsSandboxFlags && parentWindow) {
-    newDocShell->SetOnePermittedSandboxedNavigator(parentWindow->GetDocShell());
+    newDocShell->GetBrowsingContext()->SetOnePermittedSandboxedNavigator(
+        parentWindow->GetBrowsingContext());
   }
 
   // Copy sandbox flags to the new window if activeDocsSandboxFlags says to do
   // so.  Note that it's only nonzero if the window is new, so clobbering
   // sandbox flags on the window makes sense in that case.
   if (activeDocsSandboxFlags &
       SANDBOX_PROPAGATES_TO_AUXILIARY_BROWSING_CONTEXTS) {
     newDocShell->SetSandboxFlags(activeDocsSandboxFlags);