Bug 1337537 - Remove the need for TabGroup to be an nsISupports for DocShell::FindItemWithName (r=mystor)

MozReview-Commit-ID: CvnWt9Ny1IF

docshell/base/nsDocShell.cpp

@@ -3693,8 +3693,9 @@ ItemIsActive(nsIDocShellTreeItem* aItem)
 
 NS_IMETHODIMP
 nsDocShell::FindItemWithName(const nsAString& aName,
-                             nsISupports* aRequestor,
+                             nsIDocShellTreeItem* aRequestor,
                              nsIDocShellTreeItem* aOriginalRequestor,
+                             bool aSkipTabGroup,
                              nsIDocShellTreeItem** aResult)
 {
   NS_ENSURE_ARG_POINTER(aResult);
@@ -3709,7 +3710,8 @@ nsDocShell::FindItemWithName(const nsAString& aName,
   if (aRequestor) {
     // If aRequestor is not null we don't need to check special names, so
     // just hand straight off to the search by actual name function.
-    return DoFindItemWithName(aName, aRequestor, aOriginalRequestor, aResult);
+    return DoFindItemWithName(aName, aRequestor, aOriginalRequestor,
+                              aSkipTabGroup, aResult);
   } else {
     // This is the entry point into the target-finding algorithm.  Check
     // for special names.  This should only be done once, hence the check
@@ -3733,7 +3735,7 @@ nsDocShell::FindItemWithName(const nsAString& aName,
     } else {
       // Do the search for item by an actual name.
       DoFindItemWithName(aName, aRequestor, aOriginalRequestor,
-                         getter_AddRefs(foundItem));
+                         aSkipTabGroup, getter_AddRefs(foundItem));
     }
 
     if (foundItem && !CanAccessItem(foundItem, aOriginalRequestor)) {
@@ -3763,8 +3765,9 @@ nsDocShell::AssertOriginAttributesMatchPrivateBrowsing() {
 
 nsresult
 nsDocShell::DoFindItemWithName(const nsAString& aName,
-                               nsISupports* aRequestor,
+                               nsIDocShellTreeItem* aRequestor,
                                nsIDocShellTreeItem* aOriginalRequestor,
+                               bool aSkipTabGroup,
                                nsIDocShellTreeItem** aResult)
 {
   // First we check our name.
@@ -3774,16 +3777,12 @@ nsDocShell::DoFindItemWithName(const nsAString& aName,
     return NS_OK;
   }
 
-  // This QI may fail, but the places where we want to compare, comparing
-  // against nullptr serves the same purpose.
-  nsCOMPtr<nsIDocShellTreeItem> reqAsTreeItem(do_QueryInterface(aRequestor));
-
   // Second we check our children making sure not to ask a child if
   // it is the aRequestor.
 #ifdef DEBUG
   nsresult rv =
 #endif
-  FindChildWithName(aName, true, true, reqAsTreeItem, aOriginalRequestor,
+  FindChildWithName(aName, true, true, aRequestor, aOriginalRequestor,
                     aResult);
   NS_ASSERTION(NS_SUCCEEDED(rv),
                "FindChildWithName should not be failing here.");
@@ -3799,7 +3798,7 @@ nsDocShell::DoFindItemWithName(const nsAString& aName,
   nsCOMPtr<nsIDocShellTreeItem> parentAsTreeItem =
     do_QueryInterface(GetAsSupports(mParent));
   if (parentAsTreeItem) {
-    if (parentAsTreeItem == reqAsTreeItem) {
+    if (parentAsTreeItem == aRequestor) {
       return NS_OK;
     }
 
@@ -3810,6 +3809,7 @@ nsDocShell::DoFindItemWithName(const nsAString& aName,
         aName,
         static_cast<nsIDocShellTreeItem*>(this),
         aOriginalRequestor,
+        /* aSkipTabGroup = */ false,
         aResult);
     }
   }
@@ -3817,13 +3817,9 @@ nsDocShell::DoFindItemWithName(const nsAString& aName,
   // If we have a null parent or the parent is not of the same type, we need to
   // give up on finding it in our tree, and start looking in our TabGroup.
   nsCOMPtr<nsPIDOMWindowOuter> window = GetWindow();
-  if (window) {
+  if (window && !aSkipTabGroup) {
     RefPtr<mozilla::dom::TabGroup> tabGroup = window->TabGroup();
-    // We don't want to make the request to our TabGroup if they are the ones
-    // which made a request to us.
-    if (tabGroup != aRequestor) {
-      tabGroup->FindItemWithName(aName, this, aOriginalRequestor, aResult);
-    }
+    tabGroup->FindItemWithName(aName, aRequestor, aOriginalRequestor, aResult);
   }
 
   return NS_OK;
@@ -9834,7 +9830,7 @@ nsDocShell::InternalLoad(nsIURI* aURI,
         aWindowTarget.LowerCaseEqualsLiteral("_self") ||
         aWindowTarget.LowerCaseEqualsLiteral("_parent") ||
         aWindowTarget.LowerCaseEqualsLiteral("_top")) {
-      rv = FindItemWithName(aWindowTarget, nullptr, this,
+      rv = FindItemWithName(aWindowTarget, nullptr, this, false,
                             getter_AddRefs(targetItem));
       NS_ENSURE_SUCCESS(rv, rv);
     }

docshell/base/nsDocShell.h

@@ -1057,8 +1057,9 @@ private:
   // Separate function to do the actual name (i.e. not _top, _self etc.)
   // searching for FindItemWithName.
   nsresult DoFindItemWithName(const nsAString& aName,
-                              nsISupports* aRequestor,
+                              nsIDocShellTreeItem* aRequestor,
                               nsIDocShellTreeItem* aOriginalRequestor,
+                              bool aSkipTabGroup,
                               nsIDocShellTreeItem** aResult);
 
   // Helper assertion to enforce that mInPrivateBrowsing is in sync with

docshell/base/nsIDocShellTreeItem.idl

@@ -87,8 +87,8 @@ interface nsIDocShellTreeItem : nsISupports
 			ii.) Do not ask a child if it is of a different item type.
 	3.)  If there is a parent of the same item type ask parent to perform the check
 		a.) Do not ask parent if it is the aRequestor
-	4.)  If there is a tree owner ask the tree owner to perform the check
-		a.)  Do not ask the tree owner if it is the aRequestor
+	4.)  If there is a tab group ask the tab group to perform the check
+		a.)  Do not ask the tab group if aSkipTabGroup
 		b.)  This should only be done if there is no parent of the same type.
 
 	Return the child DocShellTreeItem with the specified name.
@@ -102,10 +102,12 @@ interface nsIDocShellTreeItem : nsISupports
 		asked it to search.  Children also use this to test against the treeOwner;
 	aOriginalRequestor - The original treeitem that made the request, if any.
 		This is used to ensure that we don't run into cross-site issues.
+	aSkipTabGroup - Whether the tab group should be checked.
 	*/
 	nsIDocShellTreeItem findItemWithName(in AString name,
-	                                     in nsISupports aRequestor,
-	                                     in nsIDocShellTreeItem aOriginalRequestor);
+	                                     in nsIDocShellTreeItem aRequestor,
+	                                     in nsIDocShellTreeItem aOriginalRequestor,
+	                                     in bool aSkipTabGroup);
 
 	/*
 	The owner of the DocShell Tree.  This interface will be called upon when

dom/base/TabGroup.cpp

@@ -217,7 +217,8 @@ TabGroup::FindItemWithName(const nsAString& aName,
     docshell->GetSameTypeRootTreeItem(getter_AddRefs(root));
     MOZ_RELEASE_ASSERT(docshell == root);
     if (root && aRequestor != root) {
-      root->FindItemWithName(aName, this, aOriginalRequestor, aFoundItem);
+      root->FindItemWithName(aName, aRequestor, aOriginalRequestor,
+                             /* aSkipTabGroup = */ true, aFoundItem);
       if (*aFoundItem) {
         break;
       }

dom/base/nsGlobalWindow.cpp

@@ -6475,6 +6475,7 @@ nsGlobalWindow::WindowExists(const nsAString& aName,
 
   nsCOMPtr<nsIDocShellTreeItem> namedItem;
   mDocShell->FindItemWithName(aName, nullptr, caller,
+                              /* aSkipTabGroup = */ false,
                               getter_AddRefs(namedItem));
   return namedItem != nullptr;
 }

toolkit/components/browser/nsWebBrowser.cpp

@@ -519,8 +519,9 @@ nsWebBrowser::GetSameTypeRootTreeItem(nsIDocShellTreeItem** aRootTreeItem)
 
 NS_IMETHODIMP
 nsWebBrowser::FindItemWithName(const nsAString& aName,
-                               nsISupports* aRequestor,
+                               nsIDocShellTreeItem* aRequestor,
                                nsIDocShellTreeItem* aOriginalRequestor,
+                               bool aSkipTabGroup,
                                nsIDocShellTreeItem** aResult)
 {
   NS_ENSURE_STATE(mDocShell);
@@ -528,8 +529,7 @@ nsWebBrowser::FindItemWithName(const nsAString& aName,
                "This should always be set when in this situation");
 
   return mDocShell->FindItemWithName(
-    aName, static_cast<nsIDocShellTreeOwner*>(mDocShellTreeOwner),
-    aOriginalRequestor, aResult);
+    aName, aRequestor, aOriginalRequestor, aSkipTabGroup, aResult);
 }
 
 nsIDocument*

toolkit/components/windowwatcher/nsWindowWatcher.cpp

@@ -1652,6 +1652,7 @@ nsWindowWatcher::GetWindowByName(const nsAString& aTargetName,
   if (startItem) {
     // Note: original requestor is null here, per idl comments
     startItem->FindItemWithName(aTargetName, nullptr, nullptr,
+                                /* aSkipTabGroup = */ false,
                                 getter_AddRefs(treeItem));
   } else {
     // Note: original requestor is null here, per idl comments
@@ -2108,6 +2109,7 @@ nsWindowWatcher::SafeGetWindowByName(const nsAString& aName,
   nsCOMPtr<nsIDocShellTreeItem> foundItem;
   if (startItem) {
     startItem->FindItemWithName(aName, nullptr, callerItem,
+                                /* aSkipTabGroup = */ false,
                                 getter_AddRefs(foundItem));
   } else {
     FindItemWithName(aName, nullptr, callerItem,