Tech Debt Repayment for StartupBrowserCreatorImpl Refactor

chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc

Index: chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc
diff --git a/chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc b/chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc
index d36c3a515a75c651685ff12e780211360e80ccc8..f1c47d597ec92b9d1a9793f26768891b4275dbfb 100644
--- a/chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc
+++ b/chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc
@@ -186,3 +186,126 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_NewTabPage) {
   EXPECT_EQ("new-tab", output[1].url.host());
   EXPECT_EQ("pinned", output[2].url.host());
 }
+
+TEST(StartupBrowserCreatorImplTest, DetermineBrowserOpenBehavior_Startup) {
+  SessionStartupPref pref_default(SessionStartupPref::Type::DEFAULT);
+  SessionStartupPref pref_last(SessionStartupPref::Type::LAST);
+  SessionStartupPref pref_urls(SessionStartupPref::Type::URLS);
+
+  // The most typical case: startup, not recovering from a crash, no switches.
+  // Test each pref with and without command-line tabs.
+  StartupBrowserCreatorImpl::BrowserOpenBehavior output =

[Peter Kasting, 2017/02/06 23:26:40]

Nit: This file is really verbose because of how long the name
StartupBrowserCreatorImpl is... I wonder if it makes sense to do something like
"using Creator = StartupBrowserCreatorImpl" atop the file.

+      StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+          pref_default, StartupBrowserCreatorImpl::PROCESS_STARTUP);
+

[Peter Kasting, 2017/02/06 23:26:40]

Nit: Why a blank line between setting and checking |output|?  (several places)

+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);

[Peter Kasting, 2017/02/06 23:26:40]

Nit: Should probably be EXPECT (several places)

+
+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_default, StartupBrowserCreatorImpl::PROCESS_STARTUP |
+                        StartupBrowserCreatorImpl::HAS_CMD_LINE_TABS);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+
+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_urls, StartupBrowserCreatorImpl::PROCESS_STARTUP);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+
+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_urls, StartupBrowserCreatorImpl::PROCESS_STARTUP |
+                     StartupBrowserCreatorImpl::HAS_CMD_LINE_TABS);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+
+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_last, StartupBrowserCreatorImpl::PROCESS_STARTUP);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::SYNCHRONOUS_RESTORE,
+            output);
+
+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_last, StartupBrowserCreatorImpl::PROCESS_STARTUP |
+                     StartupBrowserCreatorImpl::HAS_CMD_LINE_TABS);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::SYNCHRONOUS_RESTORE,
+            output);
+}
+
+TEST(StartupBrowserCreatorImplTest, DetermineBrowserOpenBehavior_CmdLineTabs) {
+  SessionStartupPref pref_default(SessionStartupPref::Type::DEFAULT);
+  SessionStartupPref pref_last(SessionStartupPref::Type::LAST);
+  SessionStartupPref pref_urls(SessionStartupPref::Type::URLS);
+
+  // Command line tabs after startup should prompt use of existing window,
+  // regardless of pref.
+  StartupBrowserCreatorImpl::BrowserOpenBehavior output =
+      StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+          pref_default, StartupBrowserCreatorImpl::HAS_CMD_LINE_TABS);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::USE_EXISTING,
+            output);
+
+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_urls, StartupBrowserCreatorImpl::HAS_CMD_LINE_TABS);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::USE_EXISTING,
+            output);
+
+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_last, StartupBrowserCreatorImpl::HAS_CMD_LINE_TABS);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::USE_EXISTING,
+            output);
+
+  // Exception: this can be overridden by switch.

[Peter Kasting, 2017/02/06 23:26:40]

Nit: switch -> passing a switch (2 places)

+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_urls, StartupBrowserCreatorImpl::HAS_NEW_WINDOW_SWITCH |
+                     StartupBrowserCreatorImpl::HAS_CMD_LINE_TABS);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+}
+
+TEST(StartupBrowserCreatorImplTest, DetermineBrowserOpenBehavior_PostCrash) {
+  SessionStartupPref pref_last(SessionStartupPref::Type::LAST);
+
+  // Launching after crash should block session restore.
+  StartupBrowserCreatorImpl::BrowserOpenBehavior output =
+      StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+          pref_last, StartupBrowserCreatorImpl::PROCESS_STARTUP |
+                         StartupBrowserCreatorImpl::IS_POST_CRASH_LAUNCH);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+
+  // Exception: this can be overridden by switch.
+

[Peter Kasting, 2017/02/06 23:26:40]

Nit: No blank line

+  output = StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(
+      pref_last, StartupBrowserCreatorImpl::PROCESS_STARTUP |
+                     StartupBrowserCreatorImpl::IS_POST_CRASH_LAUNCH |
+                     StartupBrowserCreatorImpl::HAS_RESTORE_SWITCH);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::SYNCHRONOUS_RESTORE,
+            output);
+}
+
+TEST(StartupBrowserCreatorImplTest, DetermineBrowserOpenBehavior_NotStartup) {
+  SessionStartupPref pref_default(SessionStartupPref::Type::DEFAULT);
+  SessionStartupPref pref_last(SessionStartupPref::Type::LAST);
+  SessionStartupPref pref_urls(SessionStartupPref::Type::URLS);
+
+  // Launch after startup without command-line tabs should always create a new
+  // window.
+  StartupBrowserCreatorImpl::BrowserOpenBehavior output =
+      StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(pref_default, 0);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+
+  output =
+      StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(pref_last, 0);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+
+  output =
+      StartupBrowserCreatorImpl::DetermineBrowserOpenBehavior(pref_urls, 0);
+
+  ASSERT_EQ(StartupBrowserCreatorImpl::BrowserOpenBehavior::NEW, output);
+}