Bug 1575051 - Part 2: Look in chrome browsing context group when docshell is missing. r=kmag

Also some minor cleanup in nsWindowWatcher, as well as a small fix,
where GetWindowByName forgot to addref its return value (as changed in
Part 1).

Differential Revision: https://phabricator.services.mozilla.com/D48976

--HG--
extra : moz-landing-system : lando

docshell/base/BrowsingContext.cpp

@@ -116,7 +116,9 @@ already_AddRefed<BrowsingContext> BrowsingContext::Create(
 
   // Determine which BrowsingContextGroup this context should be created in.
   RefPtr<BrowsingContextGroup> group =
-      BrowsingContextGroup::Select(aParent, aOpener);
+      (aType == Type::Chrome)
+          ? do_AddRef(BrowsingContextGroup::GetChromeGroup())
+          : BrowsingContextGroup::Select(aParent, aOpener);
 
   RefPtr<BrowsingContext> context;
   if (XRE_IsParentProcess()) {
@@ -490,7 +492,7 @@ BrowsingContext* BrowsingContext::FindWithName(const nsAString& aName) {
     // Just return null. Caller must handle creating a new window with
     // a blank name.
     found = nullptr;
-  } else if (IsSpecialName(aName)) {
+  } else if (nsContentUtils::IsSpecialName(aName)) {
     found = FindWithSpecialName(aName, *requestingContext);
   } else if (BrowsingContext* child =
                  FindWithNameInSubtree(aName, *requestingContext)) {
@@ -557,14 +559,6 @@ BrowsingContext* BrowsingContext::FindChildWithName(
   return nullptr;
 }
 
-/* static */
-bool BrowsingContext::IsSpecialName(const nsAString& aName) {
-  return (aName.LowerCaseEqualsLiteral("_self") ||
-          aName.LowerCaseEqualsLiteral("_parent") ||
-          aName.LowerCaseEqualsLiteral("_top") ||
-          aName.LowerCaseEqualsLiteral("_blank"));
-}
-
 BrowsingContext* BrowsingContext::FindWithSpecialName(
     const nsAString& aName, BrowsingContext& aRequestingContext) {
   // TODO(farre): Neither BrowsingContext nor nsDocShell checks if the

docshell/base/BrowsingContext.h

@@ -264,7 +264,7 @@ class BrowsingContext : public nsISupports,
   // 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.
+  // checks with regard to this.
   // See
   // https://html.spec.whatwg.org/multipage/browsers.html#the-rules-for-choosing-a-browsing-context-given-a-browsing-context-name.
   //
@@ -275,11 +275,18 @@ class BrowsingContext : public nsISupports,
 
   // Find a browsing context in this context's list of
   // children. Doesn't consider the special names, '_self', '_parent',
-  // '_top', or '_blank'. Performs access control with regard to
+  // '_top', or '_blank'. Performs access control checks with regard to
   // 'this'.
   BrowsingContext* FindChildWithName(const nsAString& aName,
                                      BrowsingContext& aRequestingContext);
 
+  // Find a browsing context in the subtree rooted at 'this' Doesn't
+  // consider the special names, '_self', '_parent', '_top', or
+  // '_blank'. Performs access control checks with regard to
+  // 'aRequestingContext'.
+  BrowsingContext* FindWithNameInSubtree(const nsAString& aName,
+                                         BrowsingContext& aRequestingContext);
+
   nsISupports* GetParentObject() const;
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;
@@ -470,22 +477,11 @@ class BrowsingContext : public nsISupports,
                   uint64_t aBrowsingContextId, Type aType);
 
  private:
-  // Returns true if the given name is one of the "special" names, currently:
-  // "_self", "_parent", "_top", or "_blank".
-  static bool IsSpecialName(const nsAString& aName);
-
   // Find the special browsing context if aName is '_self', '_parent',
   // '_top', but not '_blank'. The latter is handled in FindWithName
   BrowsingContext* FindWithSpecialName(const nsAString& aName,
                                        BrowsingContext& aRequestingContext);
 
-  // Find a browsing context in the subtree rooted at 'this' Doesn't
-  // consider the special names, '_self', '_parent', '_top', or
-  // '_blank'. Performs access control with regard to
-  // 'aRequestingContext'.
-  BrowsingContext* FindWithNameInSubtree(const nsAString& aName,
-                                         BrowsingContext& aRequestingContext);
-
   friend class ::nsOuterWindowProxy;
   friend class ::nsGlobalWindowOuter;
   // Update the window proxy object that corresponds to this browsing context.

toolkit/components/windowwatcher/nsWindowWatcher.cpp

@@ -1608,7 +1608,7 @@ nsWindowWatcher::GetWindowByName(const nsAString& aTargetName,
       GetBrowsingContextByName(aTargetName, false, currentContext);
 
   if (context) {
-    *aResult = context->GetDOMWindow();
+    *aResult = do_AddRef(context->GetDOMWindow()).take();
     MOZ_ASSERT(*aResult);
   }
 
@@ -2007,34 +2007,26 @@ already_AddRefed<BrowsingContext> nsWindowWatcher::GetBrowsingContextByName(
     return nullptr;
   }
 
-  if (aForceNoOpener) {
-    if (!aName.LowerCaseEqualsLiteral("_self") &&
-        !aName.LowerCaseEqualsLiteral("_top") &&
-        !aName.LowerCaseEqualsLiteral("_parent")) {
-      // Ignore all other names in the noopener case.
-      return nullptr;
-    }
-  }
-
-  RefPtr<BrowsingContext> currentContext(aCurrentContext);
-  if (!currentContext) {
-    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!
-    currentContext =
-        BrowsingContextGroup::GetChromeGroup()->Toplevels().SafeElementAt(0);
+  if (aForceNoOpener && !nsContentUtils::IsSpecialName(aName)) {
+    // Ignore all other names in the noopener case.
+    return nullptr;
   }
 
   RefPtr<BrowsingContext> foundContext;
-  if (currentContext) {
-    foundContext = currentContext->FindWithName(aName);
+  if (aCurrentContext) {
+    foundContext = aCurrentContext->FindWithName(aName);
+  } else if (!nsContentUtils::IsSpecialName(aName)) {
+    // 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 browsing context group!
+    for (RefPtr<BrowsingContext> toplevel :
+         BrowsingContextGroup::GetChromeGroup()->Toplevels()) {
+      foundContext = toplevel->FindWithNameInSubtree(aName, *toplevel);
+      if (foundContext) {
+        break;
+      }
+    }
   }
+
   return foundContext.forget();
 }