[browser navigation] Search for singleton tab from tab-less windows

When we try to open a singleton tab from a tab-less window (can't open
new tabs), it won't search for that singleton in the window that it
eventually opens the tab in, and thus could open a second copy in that
window. This CL causes it to first search more broadly for singletons
from tab-less windows. This should cause it to find the singleton in
that situation.

Bug: 939388
Change-Id: I96ff82925382e1db545b078b17cade11998711c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1837972
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702650}

chrome/browser/ui/browser_navigator.cc

@@ -166,17 +166,12 @@ std::pair<Browser*, int> GetBrowserAndTabForDisposition(
   switch (params.disposition) {
     case WindowOpenDisposition::SWITCH_TO_TAB:
 #if !defined(OS_ANDROID)
-      for (auto browser_it = BrowserList::GetInstance()->begin_last_active();
-           browser_it != BrowserList::GetInstance()->end_last_active();
-           ++browser_it) {
-        Browser* browser = *browser_it;
-        // When tab switching, only look at same profile and anonymity level.
-        if (browser->profile()->IsSameProfileAndType(profile)) {
-          int index = GetIndexOfExistingTab(browser, params);
-          if (index >= 0)
-            return {browser, index};
-        }
-      }
+    {
+      std::pair<Browser*, int> index =
+          GetIndexAndBrowserOfExistingTab(profile, params);
+      if (index.first)
+        return index;
+    }
 #endif
       FALLTHROUGH;
     case WindowOpenDisposition::CURRENT_TAB:
@@ -189,6 +184,15 @@ std::pair<Browser*, int> GetBrowserAndTabForDisposition(
       int index = GetIndexOfExistingTab(params.browser, params);
       if (index >= 0)
         return {params.browser, index};
+      // If this window can't open tabs, then it would load in a random
+      // window, potentially opening a second copy. Instead, make an extra
+      // effort to see if there's an already open copy.
+      if (params.browser && !WindowCanOpenTabs(params.browser)) {
+        std::pair<Browser*, int> index =
+            GetIndexAndBrowserOfExistingTab(profile, params);
+        if (index.first)
+          return index;
+      }
     }
       FALLTHROUGH;
     case WindowOpenDisposition::NEW_FOREGROUND_TAB:

chrome/browser/ui/browser_navigator_browsertest.cc

@@ -1571,6 +1571,33 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
             browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
 }
 
+IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest,
+                       NavigateFromNoTabStripWindowToOptions) {
+  {
+    content::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        content::NotificationService::AllSources());
+    ShowSettings(browser());
+    observer.Wait();
+  }
+  {
+    content::WindowedNotificationObserver observer(
+        content::NOTIFICATION_LOAD_STOP,
+        content::NotificationService::AllSources());
+    chrome::AddSelectedTabWithURL(browser(), GetGoogleURL(),
+                                  ui::PAGE_TRANSITION_LINK);
+    observer.Wait();
+  }
+  Browser* app_browser = CreateBrowserForApp("TestApp", browser()->profile());
+
+  // This load should cause a window and tab switch.
+  ShowSingletonTab(app_browser, GetSettingsURL());
+
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
+  EXPECT_EQ(GetSettingsURL(),
+            browser()->tab_strip_model()->GetActiveWebContents()->GetURL());
+}
+
 IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, CloseSingletonTab) {
   for (int i = 0; i < 2; ++i) {
     content::WindowedNotificationObserver observer(

chrome/browser/ui/singleton_tabs.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -123,3 +124,20 @@ int GetIndexOfExistingTab(Browser* browser, const NavigateParams& params) {
 
   return -1;
 }
+
+std::pair<Browser*, int> GetIndexAndBrowserOfExistingTab(
+    Profile* profile,
+    const NavigateParams& params) {
+  for (auto browser_it = BrowserList::GetInstance()->begin_last_active();
+       browser_it != BrowserList::GetInstance()->end_last_active();
+       ++browser_it) {
+    Browser* browser = *browser_it;
+    // When tab switching, only look at same profile and anonymity level.
+    if (browser->profile()->IsSameProfileAndType(profile)) {
+      int index = GetIndexOfExistingTab(browser, params);
+      if (index >= 0)
+        return {browser, index};
+    }
+  }
+  return {nullptr, -1};
+}

chrome/browser/ui/singleton_tabs.h

@@ -34,4 +34,10 @@ NavigateParams GetSingletonTabNavigateParams(Browser* browser, const GURL& url);
 // the tab and tab index for it. Otherwise, returns -1.
 int GetIndexOfExistingTab(Browser* browser, const NavigateParams& params);
 
+// This simply calls GetIndexOfExistingTab() for each browser that
+// matches the passed |profile|, and returns the first found tab.
+std::pair<Browser*, int> GetIndexAndBrowserOfExistingTab(
+    Profile* profile,
+    const NavigateParams& params);
+
 #endif  // CHROME_BROWSER_UI_SINGLETON_TABS_H_