Tech Debt Repayment for StartupBrowserCreatorImpl Refactor

chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc

 // Copyright 2016 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 "chrome/browser/ui/startup/startup_browser_creator_impl.h"
 
 #include "base/command_line.h"
 #include "chrome/browser/ui/startup/startup_tab_provider.h"
 #include "chrome/common/url_constants.cc"
 #include "testing/gtest/include/gtest/gtest.h"
@@ skipping 168 matching lines @@
       base::FilePath(), base::CommandLine(base::CommandLine::NO_PROGRAM),
       chrome::startup::IS_FIRST_RUN);
 
   StartupTabs output = impl.DetermineStartupTabs(provider_allows_ntp,
                                                  StartupTabs(), false, false);
   ASSERT_EQ(3U, output.size());
   EXPECT_EQ("reset-trigger", output[0].url.host());
   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);
+}