Enabling kUseConsolidatedStartupFlow by default on trunk.

Code changes:
- Disabling onboarding content for ChromeOS. Promotions for CrOS are out-of-scope for this launch.
- Ensuring that preference-specified tabs do not appear in new windows when a tabbed browser already exists for the profile.

Test changes:
- Removed 12 outdated tests
- Fixed or updated 9 tests

Details of affected tests, and rationale, can be found in this doc: https://docs.google.com/a/google.com/document/d/1PjAemEEFrl0gIDVQfnr1oAnHkb6DcV47HlOyyo7ICZI/edit?usp=sharing

BUG=608875,314819,313856

Review-Url: https://codereview.chromium.org/2475913003
Cr-Original-Commit-Position: refs/heads/master@{#442487}
Committed: https://chromium.googlesource.com/chromium/src/+/a52322134679a349366417e04e502bc8ce1f68e4
Review-Url: https://codereview.chromium.org/2475913003
Cr-Commit-Position: refs/heads/master@{#442712}

chrome/browser/app_controller_mac_browsertest.mm

@@ -44,6 +44,7 @@
 #include "components/bookmarks/test/bookmark_test_helpers.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/common/profile_management_switches.h"
+#include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
@@ -393,7 +394,8 @@ class AppControllerOpenShortcutBrowserTest : public InProcessBrowserTest {
 
 IN_PROC_BROWSER_TEST_F(AppControllerOpenShortcutBrowserTest,
                        OpenShortcutOnStartup) {
-  EXPECT_EQ(1, browser()->tab_strip_model()->count());
+  // The two tabs expected are the Welcome page and the desired URL.
+  EXPECT_EQ(2, browser()->tab_strip_model()->count());
   EXPECT_EQ(g_open_shortcut_url,
       browser()->tab_strip_model()->GetActiveWebContents()
           ->GetLastCommittedURL());
@@ -420,6 +422,15 @@ IN_PROC_BROWSER_TEST_F(AppControllerReplaceNTPBrowserTest,
   // Ensure that there is exactly 1 tab showing, and the tab is the NTP.
   GURL ntp(chrome::kChromeUINewTabURL);
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
+  browser()->tab_strip_model()->GetActiveWebContents()->GetController().LoadURL(
+      GURL(chrome::kChromeUINewTabURL), content::Referrer(),
+      ui::PageTransition::PAGE_TRANSITION_LINK, std::string());
+
+  // Wait for one navigation on the active web contents.
+  content::TestNavigationObserver ntp_navigation_observer(
+      browser()->tab_strip_model()->GetActiveWebContents());
+  ntp_navigation_observer.Wait();
+
   EXPECT_EQ(ntp,
             browser()
                 ->tab_strip_model()
@@ -429,11 +440,10 @@ IN_PROC_BROWSER_TEST_F(AppControllerReplaceNTPBrowserTest,
   GURL simple(embedded_test_server()->GetURL("/simple.html"));
   SendAppleEventToOpenUrlToAppController(simple);
 
-  // Wait for one navigation on the active web contents.
   EXPECT_EQ(1, browser()->tab_strip_model()->count());
-  content::TestNavigationObserver obs(
-      browser()->tab_strip_model()->GetActiveWebContents(), 1);
-  obs.Wait();
+  content::TestNavigationObserver event_navigation_observer(
+      browser()->tab_strip_model()->GetActiveWebContents());
+  event_navigation_observer.Wait();
 
   EXPECT_EQ(simple,
             browser()

chrome/browser/ui/startup/startup_browser_creator_triggered_reset_browsertest_win.cc

@@ -24,10 +24,13 @@
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 #include "chrome/browser/ui/startup/startup_browser_creator_impl.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
+#include "components/prefs/pref_service.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -108,6 +111,9 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTriggeredResetTest,
 
   Profile* profile = browser()->profile();
 
+  // Avoid showing the Welcome page.
+  profile->GetPrefs()->SetBoolean(prefs::kHasSeenWelcomePage, true);
+
   // Set the startup preference to open these URLs.
   SessionStartupPref pref(SessionStartupPref::URLS);
   pref.urls = urls;
@@ -146,20 +152,33 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTriggeredResetTest,
     EXPECT_EQ(expected_urls[i], tab_strip->GetWebContentsAt(i)->GetURL());
 }
 
-IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTriggeredResetTest,
+class StartupBrowserCreatorTriggeredResetFirstRunTest
+    : public StartupBrowserCreatorTriggeredResetTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    command_line->AppendSwitch(switches::kForceFirstRun);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTriggeredResetFirstRunTest,
                        TestTriggeredResetDoesNotShowWithFirstRunURLs) {
   // The presence of First Run tabs (in production code, these commonly come
   // from master_preferences) should suppress the reset UI. Check that this is
   // the case.
   ASSERT_TRUE(embedded_test_server()->Start());
   StartupBrowserCreator browser_creator;
-  browser_creator.AddFirstRunTab(GURL("http://new_tab_page"));
   browser_creator.AddFirstRunTab(
       embedded_test_server()->GetURL("/title1.html"));
+  browser_creator.AddFirstRunTab(
+      embedded_test_server()->GetURL("/title2.html"));
 
   // Prep the next launch to be offered a reset prompt.
   MockTriggeredProfileResetter::SetHasResetTrigger(true);
 
+  // Avoid showing the Welcome page.
+  browser()->profile()->GetPrefs()->SetBoolean(prefs::kHasSeenWelcomePage,
+                                               true);
+
   // Do a process-startup browser launch.
   base::CommandLine dummy(base::CommandLine::NO_PROGRAM);
   StartupBrowserCreatorImpl launch(base::FilePath(), dummy, &browser_creator,
@@ -174,15 +193,9 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTriggeredResetTest,
   TabStripModel* tab_strip = new_browser->tab_strip_model();
   ASSERT_EQ(2, tab_strip->count());
 
-  GURL expected_first_tab_url =
-      signin::ShouldShowPromoAtStartup(browser()->profile(), true)
-          ? signin::GetPromoURL(
-                signin_metrics::AccessPoint::ACCESS_POINT_START_PAGE,
-                signin_metrics::Reason::REASON_SIGNIN_PRIMARY_ACCOUNT, false)
-          : GURL(chrome::kChromeUINewTabURL);
-  EXPECT_EQ(expected_first_tab_url, tab_strip->GetWebContentsAt(0)->GetURL());
-
   EXPECT_EQ("title1.html",
+            tab_strip->GetWebContentsAt(0)->GetURL().ExtractFileName());
+  EXPECT_EQ("title2.html",
             tab_strip->GetWebContentsAt(1)->GetURL().ExtractFileName());
 }
 

chrome/browser/ui/startup/startup_features.cc

@@ -7,7 +7,7 @@
 namespace features {
 
 const base::Feature kUseConsolidatedStartupFlow{
-    "UseConsolidatedStartupFlow", base::FEATURE_DISABLED_BY_DEFAULT};
+    "UseConsolidatedStartupFlow", base::FEATURE_ENABLED_BY_DEFAULT};
 
 #if defined(OS_WIN)
 const base::Feature kWelcomeWin10InlineStyle{"WelcomeWin10InlineStyle",

chrome/browser/ui/startup/startup_tab_provider.cc

@@ -9,8 +9,11 @@
 #include "chrome/browser/profile_resetter/triggered_profile_resetter.h"
 #include "chrome/browser/profile_resetter/triggered_profile_resetter_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/tabs/pinned_tab_codec.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/locale_settings.h"
@@ -26,6 +29,10 @@
 #endif
 
 StartupTabs StartupTabProviderImpl::GetOnboardingTabs(Profile* profile) const {
+// Onboarding content has not been launched on Chrome OS.
+#if defined(OS_CHROMEOS)
+  return StartupTabs();
+#else
   if (!profile)
     return StartupTabs();
 
@@ -50,10 +57,11 @@ StartupTabs StartupTabProviderImpl::GetOnboardingTabs(Profile* profile) const {
                                          has_seen_win10_promo, is_signed_in,
                                          is_default_browser);
   }
-#endif
+#endif  // defined(OS_WIN)
 
   return CheckStandardOnboardingTabPolicy(is_first_run, has_seen_welcome_page,
                                           is_signed_in);
+#endif  // defined(OS_CHROMEOS)
 }
 
 StartupTabs StartupTabProviderImpl::GetDistributionFirstRunTabs(
@@ -86,8 +94,20 @@ StartupTabs StartupTabProviderImpl::GetPinnedTabs(
 StartupTabs StartupTabProviderImpl::GetPreferencesTabs(
     const base::CommandLine& command_line,
     Profile* profile) const {
+  // Attempt to find an existing, non-empty tabbed browser for this profile. If
+  // one exists, preferences tabs are not used.
+  BrowserList* browser_list = BrowserList::GetInstance();
+  auto other_tabbed_browser = std::find_if(
+      browser_list->begin(), browser_list->end(), [profile](Browser* browser) {
+        return browser->profile() == profile && browser->is_type_tabbed() &&
+               !browser->tab_strip_model()->empty();
+      });
+  bool profile_has_other_tabbed_browser =
+      other_tabbed_browser != browser_list->end();
+
   return CheckPreferencesTabPolicy(
-      StartupBrowserCreator::GetSessionStartupPref(command_line, profile));
+      StartupBrowserCreator::GetSessionStartupPref(command_line, profile),
+      profile_has_other_tabbed_browser);
 }
 
 StartupTabs StartupTabProviderImpl::GetNewTabPageTabs(
@@ -167,9 +187,11 @@ StartupTabs StartupTabProviderImpl::CheckPinnedTabPolicy(
 
 // static
 StartupTabs StartupTabProviderImpl::CheckPreferencesTabPolicy(
-    const SessionStartupPref& pref) {
+    const SessionStartupPref& pref,
+    bool profile_has_other_tabbed_browser) {
   StartupTabs tabs;
-  if (pref.type == SessionStartupPref::Type::URLS && !pref.urls.empty()) {
+  if (pref.type == SessionStartupPref::Type::URLS && !pref.urls.empty() &&
+      !profile_has_other_tabbed_browser) {
     for (const auto& url : pref.urls)
       tabs.push_back(StartupTab(url, false));
   }

chrome/browser/ui/startup/startup_tab_provider.h

@@ -91,10 +91,12 @@ class StartupTabProviderImpl : public StartupTabProvider {
   static StartupTabs CheckPinnedTabPolicy(const SessionStartupPref& pref,
                                           const StartupTabs& pinned_tabs);
 
-  // Determines whether preferences indicate that user-specified tabs should be
-  // shown as the default new window content, and returns the specified tabs if
-  // so.
-  static StartupTabs CheckPreferencesTabPolicy(const SessionStartupPref& pref);
+  // Determines whether preferences and window state indicate that
+  // user-specified tabs should be shown as the default new window content, and
+  // returns the specified tabs if so.
+  static StartupTabs CheckPreferencesTabPolicy(
+      const SessionStartupPref& pref,
+      bool profile_has_other_tabbed_browser);
 
   // Determines whether startup preferences require the New Tab Page to be
   // explicitly specified. Session Restore does not expect the NTP to be passed.

chrome/browser/ui/startup/startup_tab_provider_unittest.cc

@@ -192,25 +192,36 @@ TEST(StartupTabProviderTest, CheckPreferencesTabPolicy) {
   SessionStartupPref pref(SessionStartupPref::Type::URLS);
   pref.urls = {GURL(base::ASCIIToUTF16("https://www.google.com"))};
 
-  StartupTabs output = StartupTabProviderImpl::CheckPreferencesTabPolicy(pref);
+  StartupTabs output =
+      StartupTabProviderImpl::CheckPreferencesTabPolicy(pref, false);
 
   ASSERT_EQ(1U, output.size());
   EXPECT_EQ("www.google.com", output[0].url.host());
 }
 
-TEST(StartupTabProviderTest, CheckPreferencesTabPolicy_Negative) {
+TEST(StartupTabProviderTest, CheckPreferencesTabPolicy_WrongType) {
   SessionStartupPref pref_default(SessionStartupPref::Type::DEFAULT);
   pref_default.urls = {GURL(base::ASCIIToUTF16("https://www.google.com"))};
 
   StartupTabs output =
-      StartupTabProviderImpl::CheckPreferencesTabPolicy(pref_default);
+      StartupTabProviderImpl::CheckPreferencesTabPolicy(pref_default, false);
 
   EXPECT_TRUE(output.empty());
 
   SessionStartupPref pref_last(SessionStartupPref::Type::LAST);
   pref_last.urls = {GURL(base::ASCIIToUTF16("https://www.google.com"))};
 
-  output = StartupTabProviderImpl::CheckPreferencesTabPolicy(pref_last);
+  output = StartupTabProviderImpl::CheckPreferencesTabPolicy(pref_last, false);
+
+  EXPECT_TRUE(output.empty());
+}
+
+TEST(StartupTabProviderTest, CheckPreferencesTabPolicy_NotFirstBrowser) {
+  SessionStartupPref pref(SessionStartupPref::Type::URLS);
+  pref.urls = {GURL(base::ASCIIToUTF16("https://www.google.com"))};
+
+  StartupTabs output =
+      StartupTabProviderImpl::CheckPreferencesTabPolicy(pref, true);
 
   EXPECT_TRUE(output.empty());
 }