Fixes incorrect active tab when launching links

Tab with external URL should be active when launches an URL externally
via App or CLI. The current Chromium behavior is that it activates first
pinned tab in this case.

Bug: 812545
Change-Id: I1531fc65ad8b2d6dda3271d13d3fae4f84fd1cd9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1742155
Auto-Submit: Dmitry Kalchenko <xafster@yandex-team.ru>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#686800}

chrome/browser/sessions/session_restore_browsertest.cc

@@ -288,6 +288,26 @@ const char* const SmartSessionRestoreTest::kUrls[] = {
     "http://google.com/5",
     "http://google.com/6"};
 
+// Restore session with url passed in command line.
+class SessionRestoreWithURLInCommandLineTest : public SessionRestoreTest {
+ public:
+  SessionRestoreWithURLInCommandLineTest() = default;
+
+ protected:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    SessionRestoreTest::SetUpCommandLine(command_line);
+    command_line_url_ = ui_test_utils::GetTestUrl(
+        base::FilePath(base::FilePath::kCurrentDirectory),
+        base::FilePath(FILE_PATH_LITERAL("title1.html")));
+    command_line->AppendArg(command_line_url_.spec());
+  }
+
+  GURL command_line_url_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SessionRestoreWithURLInCommandLineTest);
+};
+
 // Verifies that restored tabs have a root window. This is important
 // otherwise the wrong information is communicated to the renderer.
 // (http://crbug.com/342672).
@@ -1827,3 +1847,34 @@ IN_PROC_BROWSER_TEST_F(SmartSessionRestoreTest, MAYBE_CorrectLoadingOrder) {
     }
   }
 }
+
+IN_PROC_BROWSER_TEST_F(SessionRestoreWithURLInCommandLineTest,
+                       PRE_TabWithURLFromCommandLineIsActive) {
+  SessionStartupPref pref(SessionStartupPref::DEFAULT);
+  SessionStartupPref::SetStartupPref(browser()->profile(), pref);
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+  // Add 3 pinned tabs.
+  for (const auto& url : {url1_, url2_, url3_}) {
+    ui_test_utils::NavigateToURLWithDisposition(
+        browser(), url, WindowOpenDisposition::NEW_FOREGROUND_TAB,
+        ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+    tab_strip_model->SetTabPinned(tab_strip_model->active_index(), true);
+  }
+  EXPECT_EQ(4, tab_strip_model->count());
+  EXPECT_EQ(3, tab_strip_model->IndexOfFirstNonPinnedTab());
+}
+
+IN_PROC_BROWSER_TEST_F(SessionRestoreWithURLInCommandLineTest,
+                       TabWithURLFromCommandLineIsActive) {
+  TabStripModel* tab_strip_model = browser()->tab_strip_model();
+  EXPECT_EQ(4, tab_strip_model->count());
+  EXPECT_EQ(3, tab_strip_model->active_index());
+  EXPECT_EQ(command_line_url_,
+            tab_strip_model->GetActiveWebContents()->GetURL());
+
+  // Check that the all pinned tabs have been restored.
+  EXPECT_EQ(3, tab_strip_model->IndexOfFirstNonPinnedTab());
+  EXPECT_EQ(url1_, tab_strip_model->GetWebContentsAt(0)->GetURL());
+  EXPECT_EQ(url2_, tab_strip_model->GetWebContentsAt(1)->GetURL());
+  EXPECT_EQ(url3_, tab_strip_model->GetWebContentsAt(2)->GetURL());
+}

chrome/browser/ui/startup/startup_browser_creator_impl.cc

@@ -708,51 +708,52 @@ StartupTabs StartupBrowserCreatorImpl::DetermineStartupTabs(
   // may be shown alongside command-line tabs.
   StartupTabs tabs = provider.GetResetTriggerTabs(profile_);
 
-  // Maybe add any tabs which the user has previously pinned.
-  AppendTabs(provider.GetPinnedTabs(command_line_, profile_), &tabs);
-
-  // URLs passed on the command line supersede all others.
+  // URLs passed on the command line supersede all others, except pinned tabs.
   AppendTabs(cmd_line_tabs, &tabs);
-  if (!cmd_line_tabs.empty())
-    return tabs;
-
-  // A Master Preferences file provided with this distribution may specify
-  // tabs to be displayed on first run, overriding all non-command-line tabs,
-  // including the profile reset tab.
-  StartupTabs distribution_tabs =
-      provider.GetDistributionFirstRunTabs(browser_creator_);
-  if (!distribution_tabs.empty())
-    return distribution_tabs;
-
-  StartupTabs onboarding_tabs;
-  if (promotional_tabs_enabled) {
-    // This is a launch from a prompt presented to an inactive user who chose to
-    // open Chrome and is being brought to a specific URL for this one launch.
-    // Launch the browser with the desired welcome back URL in the foreground
-    // and the other ordinary URLs (e.g., a restored session) in the background.
-    StartupTabs welcome_back_tabs = provider.GetWelcomeBackTabs(
-        profile_, browser_creator_, process_startup);
-    AppendTabs(welcome_back_tabs, &tabs);
-
-    if (welcome_enabled) {
-      // Policies for welcome (e.g., first run) may show promotional and
-      // introductory content depending on a number of system status factors,
-      // including OS and whether or not this is First Run.
-      onboarding_tabs = provider.GetOnboardingTabs(profile_);
-      AppendTabs(onboarding_tabs, &tabs);
+  if (cmd_line_tabs.empty()) {
+    // A Master Preferences file provided with this distribution may specify
+    // tabs to be displayed on first run, overriding all non-command-line tabs,
+    // including the profile reset tab.
+    StartupTabs distribution_tabs =
+        provider.GetDistributionFirstRunTabs(browser_creator_);
+    if (!distribution_tabs.empty())
+      return distribution_tabs;
+
+    StartupTabs onboarding_tabs;
+    if (promotional_tabs_enabled) {
+      // This is a launch from a prompt presented to an inactive user who chose
+      // to open Chrome and is being brought to a specific URL for this one
+      // launch. Launch the browser with the desired welcome back URL in the
+      // foreground and the other ordinary URLs (e.g., a restored session) in
+      // the background.
+      StartupTabs welcome_back_tabs = provider.GetWelcomeBackTabs(
+          profile_, browser_creator_, process_startup);
+      AppendTabs(welcome_back_tabs, &tabs);
+
+      if (welcome_enabled) {
+        // Policies for welcome (e.g., first run) may show promotional and
+        // introductory content depending on a number of system status factors,
+        // including OS and whether or not this is First Run.
+        onboarding_tabs = provider.GetOnboardingTabs(profile_);
+        AppendTabs(onboarding_tabs, &tabs);
+      }
     }
-  }
 
-  // If the user has set the preference indicating URLs to show on opening,
-  // read and add those.
-  StartupTabs prefs_tabs = provider.GetPreferencesTabs(command_line_, profile_);
-  AppendTabs(prefs_tabs, &tabs);
+    // If the user has set the preference indicating URLs to show on opening,
+    // read and add those.
+    StartupTabs prefs_tabs =
+        provider.GetPreferencesTabs(command_line_, profile_);
+    AppendTabs(prefs_tabs, &tabs);
+
+    // Potentially add the New Tab Page. Onboarding content is designed to
+    // replace (and eventually funnel the user to) the NTP. Likewise, URLs
+    // from preferences are explicitly meant to override showing the NTP.
+    if (onboarding_tabs.empty() && prefs_tabs.empty())
+      AppendTabs(provider.GetNewTabPageTabs(command_line_, profile_), &tabs);
+  }
 
-  // Potentially add the New Tab Page. Onboarding content is designed to
-  // replace (and eventually funnel the user to) the NTP. Likewise, URLs
-  // from preferences are explicitly meant to override showing the NTP.
-  if (onboarding_tabs.empty() && prefs_tabs.empty())
-    AppendTabs(provider.GetNewTabPageTabs(command_line_, profile_), &tabs);
+  // Maybe add any tabs which the user has previously pinned.
+  AppendTabs(provider.GetPinnedTabs(command_line_, profile_), &tabs);
 
   return tabs;
 }

chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc

@@ -113,25 +113,25 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs) {
       provider, StartupTabs(), true, false, false, false, true, true);
   ASSERT_EQ(4U, output.size());
   EXPECT_EQ("reset-trigger", output[0].url.host());
-  EXPECT_EQ("pinned", output[1].url.host());
-  EXPECT_EQ("onboarding", output[2].url.host());
-  EXPECT_EQ("prefs", output[3].url.host());
+  EXPECT_EQ("onboarding", output[1].url.host());
+  EXPECT_EQ("prefs", output[2].url.host());
+  EXPECT_EQ("pinned", output[3].url.host());
 
   // No extra onboarding content for managed starts.
   output = impl.DetermineStartupTabs(provider, StartupTabs(), true, false,
                                      false, false, false, true);
   ASSERT_EQ(3U, output.size());
   EXPECT_EQ("reset-trigger", output[0].url.host());
-  EXPECT_EQ("pinned", output[1].url.host());
-  EXPECT_EQ("prefs", output[2].url.host());
+  EXPECT_EQ("prefs", output[1].url.host());
+  EXPECT_EQ("pinned", output[2].url.host());
 
   // No onboarding if not enabled even if promo is allowed.
   output = impl.DetermineStartupTabs(provider, StartupTabs(), true, false,
                                      false, false, true, false);
   ASSERT_EQ(3U, output.size());
   EXPECT_EQ("reset-trigger", output[0].url.host());
-  EXPECT_EQ("pinned", output[1].url.host());
-  EXPECT_EQ("prefs", output[2].url.host());
+  EXPECT_EQ("prefs", output[1].url.host());
+  EXPECT_EQ("pinned", output[2].url.host());
 }
 
 // Only the New Tab Page should appear in Incognito mode, skipping all the usual
@@ -211,8 +211,8 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_CommandLine) {
       provider, cmd_line_tabs, true, false, false, false, true, true);
   ASSERT_EQ(3U, output.size());
   EXPECT_EQ("reset-trigger", output[0].url.host());
-  EXPECT_EQ("pinned", output[1].url.host());
-  EXPECT_EQ("cmd-line", output[2].url.host());
+  EXPECT_EQ("cmd-line", output[1].url.host());
+  EXPECT_EQ("pinned", output[2].url.host());
 
   // Also test that both incognito and crash recovery don't interfere with
   // command line tabs.
@@ -250,8 +250,8 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_NewTabPage) {
                                 false, false, true, true);
   ASSERT_EQ(3U, output.size());
   EXPECT_EQ("reset-trigger", output[0].url.host());
-  EXPECT_EQ("pinned", output[1].url.host());
-  EXPECT_EQ("new-tab", output[2].url.host());
+  EXPECT_EQ("new-tab", output[1].url.host());
+  EXPECT_EQ("pinned", output[2].url.host());
 }
 
 // The welcome back page should appear before any other session restore tabs.
@@ -266,23 +266,23 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_WelcomeBackPage) {
       impl.DetermineStartupTabs(provider_allows_ntp, StartupTabs(), true, false,
                                 false, false, true, true);
   ASSERT_EQ(3U, output.size());
-  EXPECT_EQ("pinned", output[0].url.host());
-  EXPECT_EQ("welcome-back", output[1].url.host());
-  EXPECT_EQ("prefs", output[2].url.host());
+  EXPECT_EQ("welcome-back", output[0].url.host());
+  EXPECT_EQ("prefs", output[1].url.host());
+  EXPECT_EQ("pinned", output[2].url.host());
 
   // No welcome back for non-startup opens.
   output = impl.DetermineStartupTabs(provider_allows_ntp, StartupTabs(), false,
                                      false, false, false, true, true);
   ASSERT_EQ(2U, output.size());
-  EXPECT_EQ("pinned", output[0].url.host());
-  EXPECT_EQ("prefs", output[1].url.host());
+  EXPECT_EQ("prefs", output[0].url.host());
+  EXPECT_EQ("pinned", output[1].url.host());
 
   // No welcome back for managed starts even if first run.
   output = impl.DetermineStartupTabs(provider_allows_ntp, StartupTabs(), true,
                                      false, false, false, false, true);
   ASSERT_EQ(2U, output.size());
-  EXPECT_EQ("pinned", output[0].url.host());
-  EXPECT_EQ("prefs", output[1].url.host());
+  EXPECT_EQ("prefs", output[0].url.host());
+  EXPECT_EQ("pinned", output[1].url.host());
 }
 
 TEST(StartupBrowserCreatorImplTest, DetermineBrowserOpenBehavior_Startup) {