Welcome page changes for Windows 10 and over.

chrome/browser/ui/startup/startup_browser_creator_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <string>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/prefs/pref_service.h"
@@ skipping 24 matching lines @@
 #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/extensions/extension_constants.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/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/test/test_utils.h"
 #include "extensions/browser/extension_system.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 #if defined(ENABLE_CONFIGURATION_POLICY) && !defined(OS_CHROMEOS)
 #include "base/callback.h"
 #include "base/run_loop.h"
 #include "base/values.h"
 #include "components/policy/core/browser/browser_policy_connector.h"
 #include "components/policy/core/common/external_data_fetcher.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_types.h"
 #include "policy/policy_constants.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
 using testing::_;
 using testing::Return;
 #endif  // defined(ENABLE_CONFIGURATION_POLICY) && !defined(OS_CHROMEOS)
 
 #if defined(ENABLE_SUPERVISED_USERS)
 #include "chrome/browser/supervised_user/supervised_user_navigation_observer.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
 #endif
 
+#if defined(OS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 using extensions::Extension;
 
 namespace {
 
 // Check that there are two browsers. Find the one that is not |browser|.
 Browser* FindOneOtherBrowser(Browser* browser) {
   // There should only be one other browser.
   EXPECT_EQ(2u, chrome::GetBrowserCount(browser->profile(),
                                         browser->host_desktop_type()));
 
   // Find the new browser.
   Browser* other_browser = NULL;
   for (chrome::BrowserIterator it; !it.done() && !other_browser; it.Next()) {
     if (*it != browser)
       other_browser = *it;
   }
   return other_browser;
 }
 
+// Returns true if the platform supports showing the sync promo on first run.
+bool PlatformSupportsSyncPromo() {
+  // Presently, this is exactly the same set of platforms that support

[msw, 2015/07/09 17:51:31]

But the condition is simple, and changes to ShowWelcomePageAfterFirstRun might
unwittingly alter this behavior too... Why not just keep the logic separate, or
nix both for something like IsWindowsVersion10OrGreater() instead (not that I
think such a helper is needed)?

[gab, 2015/07/09 18:05:11]

s/that support/that don't support/ ?

[grt (UTC plus 2), 2015/07/10 15:43:24]

Done.

[grt (UTC plus 2), 2015/07/10 15:43:24]

Good point. I agree that it's not good to couple them together like this.
I've made the platform and version check explicit here.

+  // re-showing the welcome page outside of first run.
+  return !internals::ShowWelcomePageAfterFirstRun();
+}
+
+// Returns the index of the welcome page for normal first-run startup.
+// Ordinarily it is the second tab. It is the first tab for platforms that do
+// not show the sync promo on first run.
+int GetWelcomePageIndex() {
+  return PlatformSupportsSyncPromo() ? 1 : 0;
+}
+
+// Returns the index of the NTP/sync promo for normal first-run startup.
+// Ordinarily it is the first tab. It is the second tab for platforms that do
+// not show the sync promo on first run.
+int GetNTPIndex() {
+  return PlatformSupportsSyncPromo() ? 0 : 1;
+}
+
 }  // namespace
 
 class StartupBrowserCreatorTest : public ExtensionBrowserTest {
  protected:
   StartupBrowserCreatorTest() {}
 
   bool SetUpUserDataDirectory() override {
     return ExtensionBrowserTest::SetUpUserDataDirectory();
   }
 
@@ skipping 104 matching lines @@
 // no tabbed browser windows (eg. as if the process is running only due to a
 // background application.
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest,
                        StartupURLsOnNewWindowWithNoTabbedBrowsers) {
   // Use a couple same-site HTTP URLs.
   ASSERT_TRUE(test_server()->Start());
   std::vector<GURL> urls;
   urls.push_back(test_server()->GetURL("files/title1.html"));
   urls.push_back(test_server()->GetURL("files/title2.html"));
 
+  Profile* profile = browser()->profile();
+  chrome::HostDesktopType host_desktop_type = browser()->host_desktop_type();
+
   // Set the startup preference to open these URLs.
   SessionStartupPref pref(SessionStartupPref::URLS);
   pref.urls = urls;
-  SessionStartupPref::SetStartupPref(browser()->profile(), pref);
+  SessionStartupPref::SetStartupPref(profile, pref);
+
+  // Keep the browser process running while browsers are closed.
+  g_browser_process->AddRefModule();

[msw, 2015/07/09 17:51:31]

Why isn't this needed for StartupURLsOnNewWindow which also closes the window?

[grt (UTC plus 2), 2015/07/09 18:59:37]

StartupURLsOnNewWindow closes the second browser (new_browser), but not the
initial one created by the test fixture (browser()), so that initial window is
open throughout the test and keeping the browser process alive.

 
   // Close the browser.
   browser()->window()->Close();
 
   // Do a simple non-process-startup browser launch.
   base::CommandLine dummy(base::CommandLine::NO_PROGRAM);
   chrome::startup::IsFirstRun first_run = first_run::IsChromeFirstRun() ?
       chrome::startup::IS_FIRST_RUN : chrome::startup::IS_NOT_FIRST_RUN;
-  StartupBrowserCreatorImpl launch(base::FilePath(), dummy, first_run);
-  ASSERT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(), false,
-                            browser()->host_desktop_type()));
+  {
+    StartupBrowserCreatorImpl launch(base::FilePath(), dummy, first_run);
+    ASSERT_TRUE(
+        launch.Launch(profile, std::vector<GURL>(), false, host_desktop_type));
+  }
 
   // This should have created a new browser window.  |browser()| is still
   // around at this point, even though we've closed its window.
   Browser* new_browser = FindOneOtherBrowser(browser());
   ASSERT_TRUE(new_browser);
 
-  // The new browser should have one tab for each URL.
-  TabStripModel* tab_strip = new_browser->tab_strip_model();
-  ASSERT_EQ(static_cast<int>(urls.size()), tab_strip->count());
-  for (size_t i=0; i < urls.size(); i++) {
-    EXPECT_EQ(urls[i], tab_strip->GetWebContentsAt(i)->GetURL());
+  if (internals::ShowWelcomePageAfterFirstRun()) {
+    // The new browser should have the welcome tab and one tab for each URL.
+    TabStripModel* tab_strip = new_browser->tab_strip_model();
+    ASSERT_EQ(static_cast<int>(urls.size()) + 1, tab_strip->count());
+    EXPECT_EQ(internals::GetWelcomePageURL(),
+              tab_strip->GetWebContentsAt(0)->GetURL());
+    for (size_t i = 0; i < urls.size(); i++) {
+      EXPECT_EQ(urls[i], tab_strip->GetWebContentsAt(i + 1)->GetURL());
+    }
+  } else {
+    // The new browser should have one tab for each URL.
+    TabStripModel* tab_strip = new_browser->tab_strip_model();
+    ASSERT_EQ(static_cast<int>(urls.size()), tab_strip->count());
+    for (size_t i = 0; i < urls.size(); i++) {
+      EXPECT_EQ(urls[i], tab_strip->GetWebContentsAt(i)->GetURL());
+    }
+
+    // The two tabs, despite having the same site, should be in different
+    // SiteInstances.
+    EXPECT_NE(tab_strip->GetWebContentsAt(0)->GetSiteInstance(),
+              tab_strip->GetWebContentsAt(1)->GetSiteInstance());
   }
 
-  // The two tabs, despite having the same site, should be in different
-  // SiteInstances.
-  EXPECT_NE(tab_strip->GetWebContentsAt(0)->GetSiteInstance(),
-            tab_strip->GetWebContentsAt(1)->GetSiteInstance());
+  // The test is complete if the welcome page wasn't included above.
+  if (!internals::ShowWelcomePageAfterFirstRun()) {
+    g_browser_process->ReleaseModule();

[gab, 2015/07/09 18:05:11]

(orthogonal: Wouldn't it be nice to have scoped objects for these refcounts,
eh...)

[grt (UTC plus 2), 2015/07/10 15:43:24]

Acknowledged.

+    return;
+  }
+
+  // Close the browser opened above.
+  {
+    content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_BROWSER_CLOSED,
+        content::Source<Browser>(new_browser));
+    new_browser->window()->Close();
+    observer.Wait();
+  }
+
+  // Try again to ensure that the welcome page isn't shown the second time.
+  {
+    StartupBrowserCreatorImpl launch(base::FilePath(), dummy, first_run);
+    ASSERT_TRUE(
+        launch.Launch(profile, std::vector<GURL>(), false, host_desktop_type));
+  }
+
+  // Find the new browser and ensure that it has only the expected URLs this
+  // time.
+  new_browser = FindTabbedBrowser(profile, true, host_desktop_type);

[gab, 2015/07/09 18:05:11]

The initial assignment above uses FindOneOtherBrowser(), I don't care which is
used, but we should be consistent (or comment on why it differs if it truly
does).

[grt (UTC plus 2), 2015/07/10 15:43:24]

Comment added.

+  ASSERT_TRUE(new_browser);
+  ASSERT_EQ(static_cast<int>(urls.size()),
+            new_browser->tab_strip_model()->count());
+
+  g_browser_process->ReleaseModule();
 }
 
 // Verify that startup URLs aren't used when the process already exists
 // and has other tabbed browser windows.  This is the common case of starting a
 // new browser.
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest,
                        StartupURLsOnNewWindow) {
   // Use a couple arbitrary URLs.
   std::vector<GURL> urls;
   urls.push_back(ui_test_utils::GetTestUrl(
       base::FilePath(base::FilePath::kCurrentDirectory),
       base::FilePath(FILE_PATH_LITERAL("title1.html"))));
   urls.push_back(ui_test_utils::GetTestUrl(
       base::FilePath(base::FilePath::kCurrentDirectory),
       base::FilePath(FILE_PATH_LITERAL("title2.html"))));
 
   // Set the startup preference to open these URLs.
   SessionStartupPref pref(SessionStartupPref::URLS);
   pref.urls = urls;
   SessionStartupPref::SetStartupPref(browser()->profile(), pref);
 
   // Do a simple non-process-startup browser launch.
   base::CommandLine dummy(base::CommandLine::NO_PROGRAM);
   chrome::startup::IsFirstRun first_run = first_run::IsChromeFirstRun() ?
       chrome::startup::IS_FIRST_RUN : chrome::startup::IS_NOT_FIRST_RUN;
-  StartupBrowserCreatorImpl launch(base::FilePath(), dummy, first_run);
-  ASSERT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(), false,
-                            browser()->host_desktop_type()));
+  {
+    StartupBrowserCreatorImpl launch(base::FilePath(), dummy, first_run);
+    ASSERT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(), false,
+                              browser()->host_desktop_type()));
+  }
 
   // This should have created a new browser window.
   Browser* new_browser = FindOneOtherBrowser(browser());
   ASSERT_TRUE(new_browser);
 
-  // The new browser should have exactly one tab (not the startup URLs).
+  if (internals::ShowWelcomePageAfterFirstRun()) {
+    // The new browser should have two tabs (not the startup URLs).
+    ASSERT_EQ(2, new_browser->tab_strip_model()->count());
+  } else {
+    // The new browser should have exactly one tab (not the startup URLs).
+    ASSERT_EQ(1, new_browser->tab_strip_model()->count());
+  }
+
+  // The test is complete if the welcome page wasn't included above.
+  if (!internals::ShowWelcomePageAfterFirstRun())
+    return;
+
+  // Close the browser opened above.
+  {
+    content::WindowedNotificationObserver observer(
+        chrome::NOTIFICATION_BROWSER_CLOSED,
+        content::Source<Browser>(new_browser));
+    new_browser->window()->Close();
+    observer.Wait();
+  }
+
+  // Try again to ensure that the welcome page isn't shown the second time.
+  {
+    StartupBrowserCreatorImpl launch(base::FilePath(), dummy, first_run);
+    ASSERT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(), false,
+                              browser()->host_desktop_type()));
+  }
+
+  // Find the new browser and ensure that it has only the one tab this time.
+  new_browser = FindOneOtherBrowser(browser());
+  ASSERT_TRUE(new_browser);
   ASSERT_EQ(1, new_browser->tab_strip_model()->count());
 }
 
 // App shortcuts are not implemented on mac os.
 #if !defined(OS_MACOSX)
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, OpenAppShortcutNoPref) {
   // Load an app with launch.container = 'tab'.
   const Extension* extension_app = NULL;
   ASSERT_NO_FATAL_FAILURE(LoadApp("app_with_tab_container", &extension_app));
 
@@ skipping 187 matching lines @@
   StartupBrowserCreatorImpl launch(base::FilePath(), dummy,
                                    chrome::startup::IS_FIRST_RUN);
   ASSERT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(), false,
                             browser()->host_desktop_type()));
 
   // This should have created a new browser window.
   Browser* new_browser = FindOneOtherBrowser(browser());
   ASSERT_TRUE(new_browser);
 
   TabStripModel* tab_strip = new_browser->tab_strip_model();
-  EXPECT_EQ(1, tab_strip->count());
 
   if (signin::ShouldShowPromoAtStartup(browser()->profile(), true)) {
+    ASSERT_EQ(1, tab_strip->count());
     EXPECT_EQ(signin::GetPromoURL(signin_metrics::SOURCE_START_PAGE, false),
               tab_strip->GetWebContentsAt(0)->GetURL());
+  } else if (internals::ShowWelcomePageAfterFirstRun()) {
+    ASSERT_EQ(2, tab_strip->count());
+    EXPECT_EQ(GURL(internals::GetWelcomePageURL()),
+              tab_strip->GetWebContentsAt(0)->GetURL());
+    EXPECT_EQ(GURL(chrome::kChromeUINewTabURL),
+              tab_strip->GetWebContentsAt(1)->GetURL());
   } else {
+    ASSERT_EQ(1, tab_strip->count());
     EXPECT_EQ(GURL(chrome::kChromeUINewTabURL),
               tab_strip->GetWebContentsAt(0)->GetURL());
   }
 }
 
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, SyncPromoWithWelcomePage) {
   first_run::SetShouldShowWelcomePage();
 
   // Do a simple non-process-startup browser launch.
   base::CommandLine dummy(base::CommandLine::NO_PROGRAM);
   StartupBrowserCreatorImpl launch(base::FilePath(), dummy,
                                    chrome::startup::IS_FIRST_RUN);
   ASSERT_TRUE(launch.Launch(browser()->profile(), std::vector<GURL>(), false,
                             browser()->host_desktop_type()));
 
   // This should have created a new browser window.
   Browser* new_browser = FindOneOtherBrowser(browser());
   ASSERT_TRUE(new_browser);
 
   TabStripModel* tab_strip = new_browser->tab_strip_model();
   EXPECT_EQ(2, tab_strip->count());
 
   if (signin::ShouldShowPromoAtStartup(browser()->profile(), true)) {
     EXPECT_EQ(signin::GetPromoURL(signin_metrics::SOURCE_START_PAGE, false),
-              tab_strip->GetWebContentsAt(0)->GetURL());
+              tab_strip->GetWebContentsAt(GetNTPIndex())->GetURL());
   } else {
     EXPECT_EQ(GURL(chrome::kChromeUINewTabURL),
-              tab_strip->GetWebContentsAt(0)->GetURL());
+              tab_strip->GetWebContentsAt(GetNTPIndex())->GetURL());
   }
   EXPECT_EQ(internals::GetWelcomePageURL(),
-            tab_strip->GetWebContentsAt(1)->GetURL());
+            tab_strip->GetWebContentsAt(GetWelcomePageIndex())->GetURL());
 }
 
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, SyncPromoWithFirstRunTabs) {
   StartupBrowserCreator browser_creator;
   browser_creator.AddFirstRunTab(test_server()->GetURL("files/title1.html"));
 
   // The welcome page should not be shown, even if
   // first_run::ShouldShowWelcomePage() says so, when there are already
   // more than 2 first run tabs.
   first_run::SetShouldShowWelcomePage();
@@ skipping 113 matching lines @@
   // urls1 were opened in a browser for default_profile, and urls2 were opened
   // in a browser for other_profile.
   Browser* new_browser = NULL;
   // |browser()| is still around at this point, even though we've closed its
   // window. Thus the browser count for default_profile is 2.
   ASSERT_EQ(2u, chrome::GetBrowserCount(default_profile,
                                         browser()->host_desktop_type()));
   new_browser = FindOneOtherBrowserForProfile(default_profile, browser());
   ASSERT_TRUE(new_browser);
   TabStripModel* tab_strip = new_browser->tab_strip_model();
-  ASSERT_EQ(1, tab_strip->count());
-  EXPECT_EQ(urls1[0], tab_strip->GetWebContentsAt(0)->GetURL());
+  ASSERT_LT(0, tab_strip->count());

[msw, 2015/07/09 17:51:30]

nit: not necessary with the checks in each case below.

[gab, 2015/07/09 18:05:11]

Constant on RHS for inequalities (also >= 1 makes more sense here IMO).

But actually I think you should get rid of this line as it's already covered
more strictly be the ASSERT_EQ's below.

[grt (UTC plus 2), 2015/07/09 18:59:37]

Done.
Done.

[grt (UTC plus 2), 2015/07/09 18:59:37]

tab_strip->GetWebContentsAt(0) on the next line will cause the test to crash if
the tab strip is empty. I thought it would be better for the test to fail with
the ASSERT here rather than crash outright. I've rejiggered things now so that
it's no longer necessary.

+  content::WebContents* web_contents = tab_strip->GetWebContentsAt(0);
+  if (internals::ShowWelcomePageAfterFirstRun()) {

[gab, 2015/07/09 18:05:12]

Add a comment here like:

// The first profile should get the welcome page on supported platforms, but the
second profile should not get a second instance of it.

[grt (UTC plus 2), 2015/07/09 18:59:37]

Done.

+    ASSERT_EQ(2, tab_strip->count());
+    EXPECT_EQ(GURL(internals::GetWelcomePageURL()), web_contents->GetURL());
+    web_contents = tab_strip->GetWebContentsAt(1);

[gab, 2015/07/09 18:05:11]

When reading this I first got confused thinking this was the Welcome Page's
index and wondering why it wasn't first...

I think this would be more readable if each WebContents* had its own explicitly
named variable then re-using a single one declared outside this scope

[grt (UTC plus 2), 2015/07/09 18:59:37]

Done.

[grt (UTC plus 2), 2015/07/10 15:43:24]

i went ahead and removed these web_contents variables altogether. i think it's
even more clear (and consistent) to have GetWebContentsAt() in the test
expectation.

+    EXPECT_EQ(urls1[0], web_contents->GetURL());
+  } else {
+    ASSERT_EQ(1, tab_strip->count());
+    EXPECT_EQ(urls1[0], web_contents->GetURL());
+  }
 
   ASSERT_EQ(1u, chrome::GetBrowserCount(other_profile,
                                         browser()->host_desktop_type()));
   new_browser = FindOneOtherBrowserForProfile(other_profile, NULL);
   ASSERT_TRUE(new_browser);
   tab_strip = new_browser->tab_strip_model();
   ASSERT_EQ(1, tab_strip->count());
   EXPECT_EQ(urls2[0], tab_strip->GetWebContentsAt(0)->GetURL());
 }
 
@@ skipping 198 matching lines @@
 
   while (SessionRestore::IsRestoring(default_profile) ||
          SessionRestore::IsRestoring(profile_home1) ||
          SessionRestore::IsRestoring(profile_home2) ||
          SessionRestore::IsRestoring(profile_last) ||
          SessionRestore::IsRestoring(profile_urls))
     base::MessageLoop::current()->RunUntilIdle();
 
   Browser* new_browser = NULL;
   // The last open profile (the profile_home1 in this case) will always be
-  // launched, even if it will open just the home page.
+  // launched, even if it will open just the home page (and the welcome page on

[gab, 2015/07/09 18:05:11]

s/home page/new tab page/

[grt (UTC plus 2), 2015/07/10 15:43:24]

Done.

+  // relevant platforms).
   ASSERT_EQ(1u, chrome::GetBrowserCount(profile_home1, original_desktop_type));
   new_browser = FindOneOtherBrowserForProfile(profile_home1, NULL);
   ASSERT_TRUE(new_browser);
   TabStripModel* tab_strip = new_browser->tab_strip_model();
-  ASSERT_EQ(1, tab_strip->count());
-  EXPECT_EQ(GURL(chrome::kChromeUINewTabURL),
-            tab_strip->GetWebContentsAt(0)->GetURL());
+  ASSERT_LT(0, tab_strip->count());

[gab, 2015/07/09 18:05:12]

Redundant.

[grt (UTC plus 2), 2015/07/10 15:43:24]

removed

+  content::WebContents* web_contents = tab_strip->GetWebContentsAt(0);
+  if (internals::ShowWelcomePageAfterFirstRun()) {
+    ASSERT_EQ(2, tab_strip->count());
+    EXPECT_EQ(GURL(internals::GetWelcomePageURL()), web_contents->GetURL());
+    web_contents = tab_strip->GetWebContentsAt(1);

[gab, 2015/07/09 18:05:12]

Same comment as above, suggest using explicit variable names.

[grt (UTC plus 2), 2015/07/10 15:43:24]

Done.

+    EXPECT_EQ(GURL(chrome::kChromeUINewTabURL), web_contents->GetURL());
+  } else {
+    ASSERT_EQ(1, tab_strip->count());
+    EXPECT_EQ(GURL(chrome::kChromeUINewTabURL), web_contents->GetURL());
+  }
 
   // profile_urls opened the urls.
   ASSERT_EQ(1u, chrome::GetBrowserCount(profile_urls, original_desktop_type));
   new_browser = FindOneOtherBrowserForProfile(profile_urls, NULL);
   ASSERT_TRUE(new_browser);
   tab_strip = new_browser->tab_strip_model();
   ASSERT_EQ(1, tab_strip->count());
   EXPECT_EQ(urls[0], tab_strip->GetWebContentsAt(0)->GetURL());
 
   // profile_last opened the last open pages.
@@ skipping 85 matching lines @@
   browser_creator.Start(dummy, profile_manager->user_data_dir(), profile_home,
                         last_opened_profiles);
 
   // No profiles are getting restored, since they all display the crash info
   // bar.
   EXPECT_FALSE(SessionRestore::IsRestoring(profile_home));
   EXPECT_FALSE(SessionRestore::IsRestoring(profile_last));
   EXPECT_FALSE(SessionRestore::IsRestoring(profile_urls));
 
   // The profile which normally opens the home page displays the new tab page.
+  // The welcome page is also shown for relevant platforms.
   Browser* new_browser = NULL;
   ASSERT_EQ(1u, chrome::GetBrowserCount(profile_home,
                                         browser()->host_desktop_type()));
   new_browser = FindOneOtherBrowserForProfile(profile_home, NULL);
   ASSERT_TRUE(new_browser);
   TabStripModel* tab_strip = new_browser->tab_strip_model();
-  ASSERT_EQ(1, tab_strip->count());
+  ASSERT_LT(0, tab_strip->count());

[gab, 2015/07/09 18:05:11]

Redundant.

[grt (UTC plus 2), 2015/07/10 15:43:24]

removed

   content::WebContents* web_contents = tab_strip->GetWebContentsAt(0);
-  EXPECT_EQ(GURL(chrome::kChromeUINewTabURL), web_contents->GetURL());
+  if (internals::ShowWelcomePageAfterFirstRun()) {
+    ASSERT_EQ(2, tab_strip->count());
+    EXPECT_EQ(GURL(internals::GetWelcomePageURL()), web_contents->GetURL());
+    web_contents = tab_strip->GetWebContentsAt(1);

[gab, 2015/07/09 18:05:12]

Explicit names.

[grt (UTC plus 2), 2015/07/10 15:43:24]

Done.

+    EXPECT_EQ(GURL(chrome::kChromeUINewTabURL), web_contents->GetURL());
+  } else {
+    ASSERT_EQ(1, tab_strip->count());
+    EXPECT_EQ(GURL(chrome::kChromeUINewTabURL), web_contents->GetURL());
+  }
   EnsureRestoreUIWasShown(web_contents);
 
   // The profile which normally opens last open pages displays the new tab page.
   ASSERT_EQ(1u, chrome::GetBrowserCount(profile_last,
                                         browser()->host_desktop_type()));
   new_browser = FindOneOtherBrowserForProfile(profile_last, NULL);
   ASSERT_TRUE(new_browser);
   tab_strip = new_browser->tab_strip_model();
   ASSERT_EQ(1, tab_strip->count());
   web_contents = tab_strip->GetWebContentsAt(0);
@@ skipping 123 matching lines @@
                             browser()->host_desktop_type()));
 
   // This should have created a new browser window.
   Browser* new_browser = FindOneOtherBrowser(browser());
   ASSERT_TRUE(new_browser);
 
   // Verify that the NTP and the welcome page are shown.
   TabStripModel* tab_strip = new_browser->tab_strip_model();
   ASSERT_EQ(2, tab_strip->count());
   EXPECT_EQ(GURL(chrome::kChromeUINewTabURL),
-            tab_strip->GetWebContentsAt(0)->GetURL());
+            tab_strip->GetWebContentsAt(GetNTPIndex())->GetURL());
   EXPECT_EQ(internals::GetWelcomePageURL(),
-            tab_strip->GetWebContentsAt(1)->GetURL());
+            tab_strip->GetWebContentsAt(GetWelcomePageIndex())->GetURL());
 }
 
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_MACOSX)
 // http://crbug.com/314819
 #define MAYBE_SyncPromoAllowed DISABLED_SyncPromoAllowed
 #else
 #define MAYBE_SyncPromoAllowed SyncPromoAllowed
 #endif
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorFirstRunTest,
                        MAYBE_SyncPromoAllowed) {
+  if (!PlatformSupportsSyncPromo())
+    return;
   // Consistently enable the welcome page on all platforms.
   first_run::SetShouldShowWelcomePage();
 
   // Simulate the following master_preferences:
   // {
   //  "sync_promo": {
   //    "show_on_first_run_allowed": true
   //  }
   // }
   StartupBrowserCreator browser_creator;
@@ skipping 21 matching lines @@
 }
 
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_MACOSX)
 // http://crbug.com/314819
 #define MAYBE_FirstRunTabsPromoAllowed DISABLED_FirstRunTabsPromoAllowed
 #else
 #define MAYBE_FirstRunTabsPromoAllowed FirstRunTabsPromoAllowed
 #endif
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorFirstRunTest,
                        MAYBE_FirstRunTabsPromoAllowed) {
+  if (!PlatformSupportsSyncPromo())
+    return;
   // Simulate the following master_preferences:
   // {
   //  "first_run_tabs" : [
   //    "files/title1.html"
   //  ],
   //  "sync_promo": {
   //    "show_on_first_run_allowed": true
   //  }
   // }
   StartupBrowserCreator browser_creator;
@@ skipping 23 matching lines @@
 
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_MACOSX)
 // http://crbug.com/314819
 #define MAYBE_FirstRunTabsContainSyncPromo \
     DISABLED_FirstRunTabsContainSyncPromo
 #else
 #define MAYBE_FirstRunTabsContainSyncPromo FirstRunTabsContainSyncPromo
 #endif
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorFirstRunTest,
                        MAYBE_FirstRunTabsContainSyncPromo) {
+  if (!PlatformSupportsSyncPromo())
+    return;
   // Simulate the following master_preferences:
   // {
   //  "first_run_tabs" : [
   //    "files/title1.html",
   //    "chrome://signin/?source=0&next_page=chrome%3A%2F%2Fnewtab%2F"
   //  ],
   //  "sync_promo": {
   //    "show_on_first_run_allowed": true
   //  }
   // }
@@ skipping 29 matching lines @@
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_MACOSX)
 // http://crbug.com/314819
 #define MAYBE_FirstRunTabsContainNTPSyncPromoAllowed \
     DISABLED_FirstRunTabsContainNTPSyncPromoAllowed
 #else
 #define MAYBE_FirstRunTabsContainNTPSyncPromoAllowed \
     FirstRunTabsContainNTPSyncPromoAllowed
 #endif
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorFirstRunTest,
                        MAYBE_FirstRunTabsContainNTPSyncPromoAllowed) {
+  if (!PlatformSupportsSyncPromo())
+    return;
   // Simulate the following master_preferences:
   // {
   //  "first_run_tabs" : [
   //    "new_tab_page",
   //    "files/title1.html"
   //  ],
   //  "sync_promo": {
   //    "show_on_first_run_allowed": true
   //  }
   // }
@@ skipping 27 matching lines @@
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_MACOSX)
 // http://crbug.com/314819
 #define MAYBE_FirstRunTabsContainNTPSyncPromoForbidden \
     DISABLED_FirstRunTabsContainNTPSyncPromoForbidden
 #else
 #define MAYBE_FirstRunTabsContainNTPSyncPromoForbidden \
     FirstRunTabsContainNTPSyncPromoForbidden
 #endif
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorFirstRunTest,
                        MAYBE_FirstRunTabsContainNTPSyncPromoForbidden) {
+  if (!PlatformSupportsSyncPromo())
+    return;
   // Simulate the following master_preferences:
   // {
   //  "first_run_tabs" : [
   //    "new_tab_page",
   //    "files/title1.html"
   //  ],
   //  "sync_promo": {
   //    "show_on_first_run_allowed": false
   //  }
   // }
@@ skipping 26 matching lines @@
 
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_MACOSX)
 // http://crbug.com/314819
 #define MAYBE_FirstRunTabsSyncPromoForbidden \
     DISABLED_FirstRunTabsSyncPromoForbidden
 #else
 #define MAYBE_FirstRunTabsSyncPromoForbidden FirstRunTabsSyncPromoForbidden
 #endif
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorFirstRunTest,
                        MAYBE_FirstRunTabsSyncPromoForbidden) {
+  if (!PlatformSupportsSyncPromo())
+    return;
   // Simulate the following master_preferences:
   // {
   //  "first_run_tabs" : [
   //    "files/title1.html"
   //  ],
   //  "sync_promo": {
   //    "show_on_first_run_allowed": false
   //  }
   // }
   StartupBrowserCreator browser_creator;
@@ skipping 23 matching lines @@
 #if defined(GOOGLE_CHROME_BUILD) && defined(OS_MACOSX)
 // http://crbug.com/314819
 #define MAYBE_RestoreOnStartupURLsPolicySpecified \
     DISABLED_RestoreOnStartupURLsPolicySpecified
 #else
 #define MAYBE_RestoreOnStartupURLsPolicySpecified \
     RestoreOnStartupURLsPolicySpecified
 #endif
 IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorFirstRunTest,
                        MAYBE_RestoreOnStartupURLsPolicySpecified) {
+  if (!PlatformSupportsSyncPromo())
+    return;
   // Simulate the following master_preferences:
   // {
   //  "sync_promo": {
   //    "show_on_first_run_allowed": true
   //  }
   // }
   StartupBrowserCreator browser_creator;
   browser()->profile()->GetPrefs()->SetBoolean(
       prefs::kSignInPromoShowOnFirstRunAllowed, true);
 
@@ skipping 32 matching lines @@
   ASSERT_EQ(1, tab_strip->count());
   EXPECT_EQ("title1.html",
             tab_strip->GetWebContentsAt(0)->GetURL().ExtractFileName());
 }
 #endif  // defined(ENABLE_CONFIGURATION_POLICY)
 
 #endif  // !defined(OS_LINUX) || !defined(GOOGLE_CHROME_BUILD) ||
         // defined(ENABLE_CONFIGURATION_POLICY)
 
 #endif  // !defined(OS_CHROMEOS)