Bug 1562292: Part 2c - Use BrowserContext rather than DocShell to resolve named targets. r=nika
authorKris Maglione <maglione.k@gmail.com>
Fri, 28 Jun 2019 14:34:58 -0700
changeset 488068 b47a29dc541a8cefda7c14274581e9c868944400
parent 488067 ce60426d7e3152c92c9df31e1657bee4dc928d1b
child 488069 8303383dd95b60e0b23d961bc397f670b2fd0383
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 2c - Use BrowserContext rather than DocShell to resolve named targets. r=nika This lets us lookup cross-process targets, but does not yet allow returning or loading anything into them. Differential Revision: https://phabricator.services.mozilla.com/D40500
toolkit/components/windowwatcher/nsWindowWatcher.cpp
toolkit/components/windowwatcher/nsWindowWatcher.h
toolkit/content/tests/chrome/test_window_intrinsic_size.xul
--- a/toolkit/components/windowwatcher/nsWindowWatcher.cpp
+++ b/toolkit/components/windowwatcher/nsWindowWatcher.cpp
@@ -598,28 +598,28 @@ nsresult nsWindowWatcher::OpenWindowInte
   uint32_t chromeFlags;
   nsAutoString name;           // string version of aName
   nsAutoCString features;      // string version of aFeatures
   nsCOMPtr<nsIURI> uriToLoad;  // from aUrl, if any
   nsCOMPtr<nsIDocShellTreeOwner>
       parentTreeOwner;  // from the parent window, if any
   nsCOMPtr<nsIDocShellTreeItem> newDocShellItem;  // from the new window
 
-  nsCOMPtr<nsPIDOMWindowOuter> parent =
+  nsCOMPtr<nsPIDOMWindowOuter> parentWindow =
       aParent ? nsPIDOMWindowOuter::From(aParent) : nullptr;
 
   NS_ENSURE_ARG_POINTER(aResult);
   *aResult = 0;
 
   if (!nsContentUtils::IsSafeToRunScript()) {
     nsContentUtils::WarnScriptWasIgnored(nullptr);
     return NS_ERROR_FAILURE;
   }
 
-  GetWindowTreeOwner(parent, getter_AddRefs(parentTreeOwner));
+  GetWindowTreeOwner(parentWindow, getter_AddRefs(parentTreeOwner));
 
   // We expect BrowserParent to have provided us the absolute URI of the window
   // we're to open, so there's no need to call URIfromURL (or more importantly,
   // to check for a chrome URI, which cannot be opened from a remote tab).
   if (aUrl) {
     rv = URIfromURL(aUrl, aParent, getter_AddRefs(uriToLoad));
     if (NS_FAILED(rv)) {
       return rv;
@@ -637,26 +637,27 @@ nsresult nsWindowWatcher::OpenWindowInte
 
   if (aFeatures) {
     features.Assign(aFeatures);
     features.StripWhitespace();
   } else {
     features.SetIsVoid(true);
   }
 
-  // try to find an extant window with the given name
-  nsCOMPtr<nsPIDOMWindowOuter> foundWindow =
-      SafeGetWindowByName(name, aForceNoOpener, aParent);
-  GetWindowTreeItem(foundWindow, getter_AddRefs(newDocShellItem));
+  // try to find an extant browsing context with the given name
+  RefPtr<BrowsingContext> foundContext = GetBrowsingContextByName(
+      name, aForceNoOpener,
+      parentWindow ? parentWindow->GetBrowsingContext() : nullptr);
+  if (foundContext) {
+    newDocShellItem = foundContext->GetDocShell();
+  }
 
   // Do sandbox checks here, instead of waiting until nsIDocShell::LoadURI.
   // The state of the window can change before this call and if we are blocked
   // because of sandboxing, we wouldn't want that to happen.
-  nsCOMPtr<nsPIDOMWindowOuter> parentWindow =
-      aParent ? nsPIDOMWindowOuter::From(aParent) : nullptr;
   nsCOMPtr<nsIDocShell> parentDocShell;
   if (parentWindow) {
     parentDocShell = parentWindow->GetDocShell();
     if (parentDocShell) {
       if (parentDocShell->IsSandboxedFrom(
               newDocShellItem->GetBrowsingContext())) {
         return NS_ERROR_DOM_INVALID_ACCESS_ERR;
       }
@@ -1952,57 +1953,65 @@ already_AddRefed<nsIDocShellTreeItem> ns
   nsCOMPtr<nsIDocShellTreeItem> callerItem = do_QueryInterface(callerWebNav);
   if (!callerItem) {
     callerItem = aParentItem;
   }
 
   return callerItem.forget();
 }
 
-nsPIDOMWindowOuter* nsWindowWatcher::SafeGetWindowByName(
+BrowsingContext* nsWindowWatcher::GetCallerBrowsingContext(
+    BrowsingContext* aParentItem) {
+  if (nsCOMPtr<nsIDocShell> caller = do_GetInterface(GetEntryGlobal())) {
+    return caller->GetBrowsingContext();
+  }
+  return aParentItem;
+}
+
+already_AddRefed<BrowsingContext> nsWindowWatcher::GetBrowsingContextByName(
     const nsAString& aName, bool aForceNoOpener,
-    mozIDOMWindowProxy* aCurrentWindow) {
+    BrowsingContext* aCurrentContext) {
   if (aName.IsEmpty()) {
     return nullptr;
   }
 
   if (aForceNoOpener) {
     if (!aName.LowerCaseEqualsLiteral("_self") &&
         !aName.LowerCaseEqualsLiteral("_top") &&
         !aName.LowerCaseEqualsLiteral("_parent")) {
       // Ignore all other names in the noopener case.
       return nullptr;
     }
   }
 
-  nsCOMPtr<nsIDocShellTreeItem> startItem;
-  GetWindowTreeItem(aCurrentWindow, getter_AddRefs(startItem));
-
-  nsCOMPtr<nsIDocShellTreeItem> callerItem = GetCallerTreeItem(startItem);
+  RefPtr<BrowsingContext> caller = GetCallerBrowsingContext(aCurrentContext);
 
-  nsCOMPtr<nsIDocShellTreeItem> foundItem;
-  if (startItem) {
-    startItem->FindItemWithName(aName, nullptr, callerItem,
-                                /* aSkipTabGroup = */ false,
-                                getter_AddRefs(foundItem));
+  RefPtr<BrowsingContext> foundContext;
+  if (aCurrentContext) {
+    foundContext = aCurrentContext->FindWithName(aName, *caller);
   } else {
     if (aName.LowerCaseEqualsLiteral("_blank") ||
         aName.LowerCaseEqualsLiteral("_top") ||
         aName.LowerCaseEqualsLiteral("_parent") ||
         aName.LowerCaseEqualsLiteral("_self")) {
       return nullptr;
     }
 
     // If we are looking for an item and we don't have a docshell we are
     // checking on, let's just look in the chrome tab group!
+    nsCOMPtr<nsIDocShellTreeItem> foundItem;
     Unused << TabGroup::GetChromeTabGroup()->FindItemWithName(
-        aName, nullptr, callerItem, getter_AddRefs(foundItem));
+        aName, nullptr, caller ? caller->GetDocShell() : nullptr,
+        getter_AddRefs(foundItem));
+    if (foundItem) {
+      foundContext = foundItem->GetBrowsingContext();
+    }
   }
 
-  return foundItem ? foundItem->GetWindow() : nullptr;
+  return foundContext.forget();
 }
 
 /* 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 nsWindowWatcher::ReadyOpenedDocShellItem(
--- a/toolkit/components/windowwatcher/nsWindowWatcher.h
+++ b/toolkit/components/windowwatcher/nsWindowWatcher.h
@@ -66,22 +66,25 @@ class nsWindowWatcher : public nsIWindow
   nsWatcherWindowEntry* FindWindowEntry(mozIDOMWindowProxy* aWindow);
   nsresult RemoveWindow(nsWatcherWindowEntry* aInfo);
 
   // Get the caller tree item.  Look on the JS stack, then fall back
   // to the parent if there's nothing there.
   already_AddRefed<nsIDocShellTreeItem> GetCallerTreeItem(
       nsIDocShellTreeItem* aParentItem);
 
-  // Unlike GetWindowByName this will look for a caller on the JS
-  // stack, and then fall back on aCurrentWindow if it can't find one.
+  mozilla::dom::BrowsingContext* GetCallerBrowsingContext(
+      mozilla::dom::BrowsingContext* aParent);
+
+  // Will first look for a caller on the JS stack, and then fall back on
+  // aCurrentContext if it can't find one.
   // It also knows to not look for things if aForceNoOpener is set.
-  nsPIDOMWindowOuter* SafeGetWindowByName(const nsAString& aName,
-                                          bool aForceNoOpener,
-                                          mozIDOMWindowProxy* aCurrentWindow);
+  already_AddRefed<mozilla::dom::BrowsingContext> GetBrowsingContextByName(
+      const nsAString& aName, bool aForceNoOpener,
+      mozilla::dom::BrowsingContext* aCurrentContext);
 
   // Just like OpenWindowJS, but knows whether it got called via OpenWindowJS
   // (which means called from script) or called via OpenWindow.
   nsresult OpenWindowInternal(mozIDOMWindowProxy* aParent, const char* aUrl,
                               const char* aName, const char* aFeatures,
                               bool aCalledFromJS, bool aDialog, bool aNavigate,
                               nsIArray* aArgv, bool aIsPopupSpam,
                               bool aForceNoOpener, bool aForceNoReferrer,
--- a/toolkit/content/tests/chrome/test_window_intrinsic_size.xul
+++ b/toolkit/content/tests/chrome/test_window_intrinsic_size.xul
@@ -6,17 +6,17 @@
         xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 <script src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
 <script src="chrome://mochikit/content/tests/SimpleTest/EventUtils.js"/>
 <script class="testbody" type="application/javascript"><![CDATA[
   SimpleTest.waitForExplicitFinish();
 
   function openWindow(features = "") {
     return window.openDialog("window_intrinsic_size.xul",
-                             null, "chrome,dialog=no,all," + features);
+                             "", "chrome,dialog=no,all," + features);
   }
 
   function checkWindowSize(win, width, height, msg) {
     is(win.innerWidth, width, "width should match " + msg);
     is(win.innerHeight, height, "height should match " + msg);
   }
 
   async function runTest() {