[omnibox, browser stack] Select tab before potentially closing other

If we close a tab, the tab index order can change, so, in
Navigate(), use the index before closing the other tab.

Bug: 780835
Change-Id: Ie88ab8e50528ec41e1048b3b5d12137f235202e3
Reviewed-on: https://chromium-review.googlesource.com/948822
Commit-Queue: Kevin Bailey <krb@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540987}

chrome/browser/ui/browser_navigator.cc

@@ -41,6 +41,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "extensions/features/features.h"
+#include "url/url_constants.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/ui/ash/multi_user/multi_user_window_manager.h"
@@ -667,20 +668,24 @@ void Navigate(NavigateParams* params) {
 
     // If the singleton tab isn't already selected, select it.
     if (params->source_contents != params->target_contents) {
+      // Use the index before the potential close below, because it could
+      // make the index refer to a different tab.
+      params->browser->tab_strip_model()->ActivateTabAt(singleton_index,
+                                                        user_initiated);
       if (params->disposition == WindowOpenDisposition::SWITCH_TO_TAB) {
-        // Close orphaned NTPs with no history when the user switches away from
-        // them.
+        // Close orphaned NTP (and the like) with no history when the user
+        // switches away from them.
         if (params->source_contents->GetController().CanGoBack() ||
             (params->source_contents->GetLastCommittedURL().spec() !=
                  chrome::kChromeUINewTabURL &&
              params->source_contents->GetLastCommittedURL().spec() !=
-                 chrome::kChromeSearchLocalNtpUrl))
+                 chrome::kChromeSearchLocalNtpUrl &&
+             params->source_contents->GetLastCommittedURL().spec() !=
+                 url::kAboutBlankURL))
           params->source_contents->Focus();
         else
           params->source_contents->Close();
       }
-      params->browser->tab_strip_model()->ActivateTabAt(singleton_index,
-                                                        user_initiated);
     }
   }
 

chrome/browser/ui/browser_navigator_browsertest.cc

@@ -677,6 +677,21 @@ IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, Disposition_NewWindow) {
   EXPECT_EQ(1, params.browser->tab_strip_model()->count());
 }
 
+// This test verifies that a source tab to the left of the target tab can
+// be switched away from and closed. It verifies that if we close the
+// earlier tab, that we don't use a stale index, and select the wrong tab.
+IN_PROC_BROWSER_TEST_F(BrowserNavigatorTest, OutOfOrderTabSwitchTest) {
+  GURL singleton_url("http://maps.google.com/");
+
+  NavigateHelper(singleton_url, browser(),
+                 WindowOpenDisposition::NEW_FOREGROUND_TAB);
+
+  browser()->tab_strip_model()->ActivateTabAt(0, true);
+
+  NavigateHelper(singleton_url, browser(),
+                 WindowOpenDisposition::SWITCH_TO_TAB);
+}
+
 // This test verifies that we're picking the correct browser and tab to
 // switch to. It verifies that we don't recommend the active tab, and that,
 // when switching, we don't mistakenly pick the current browser.