Convert the session restore ui_tests to browser_tests. I renabled the disabled...

Convert the session restore ui_tests to browser_tests. I renabled the disabled tests; hopefully they're now more flaky.

I found a simple way of launching a browser_test binary as a regular chrome binary which is needed for the app test case. This means I can undo r133560 which I've done here.

BUG=121574,29110,52022
Review URL: https://chromiumcodereview.appspot.com/10202005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@133852 0039d316-1c4b-4281-b951-d872f2087c98

base/test/test_suite.cc

@@ -70,7 +70,6 @@ class TestClientInitializer : public testing::EmptyTestEventListener {
 }  // namespace
 
 const char TestSuite::kStrictFailureHandling[] = "strict_failure_handling";
-const char TestSuite::kSilent[] = "silent";
 
 TestSuite::TestSuite(int argc, char** argv) : initialized_command_line_(false) {
   PreInitialize(argc, argv, true);
@@ -178,13 +177,6 @@ int TestSuite::Run() {
       CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kTestChildProcess);
 
-  bool silent = CommandLine::ForCurrentProcess()->HasSwitch(kSilent);
-  if (silent) {
-    testing::TestEventListeners& listeners =
-        testing::UnitTest::GetInstance()->listeners();
-    delete listeners.Release(listeners.default_result_printer());
-  }
-
   // Check to see if we are being run as a client process.
   if (!client_func.empty())
     return multi_process_function_list::InvokeChildProcessTest(client_func);
@@ -194,20 +186,18 @@ int TestSuite::Run() {
   if (result != 0 && GetTestCount(&TestSuite::NonIgnoredFailures) == 0)
     result = 0;
 
-  if (!silent) {
-    // Display the number of flaky tests.
-    int flaky_count = GetTestCount(&TestSuite::IsMarkedFlaky);
-    if (flaky_count) {
-      printf("  YOU HAVE %d FLAKY %s\n\n", flaky_count,
-             flaky_count == 1 ? "TEST" : "TESTS");
-    }
+  // Display the number of flaky tests.
+  int flaky_count = GetTestCount(&TestSuite::IsMarkedFlaky);
+  if (flaky_count) {
+    printf("  YOU HAVE %d FLAKY %s\n\n", flaky_count,
+           flaky_count == 1 ? "TEST" : "TESTS");
+  }
 
-    // Display the number of tests with ignored failures (FAILS).
-    int failing_count = GetTestCount(&TestSuite::IsMarkedFailing);
-    if (failing_count) {
-      printf("  YOU HAVE %d %s with ignored failures (FAILS prefix)\n\n",
-             failing_count, failing_count == 1 ? "test" : "tests");
-    }
+  // Display the number of tests with ignored failures (FAILS).
+  int failing_count = GetTestCount(&TestSuite::IsMarkedFailing);
+  if (failing_count) {
+    printf("  YOU HAVE %d %s with ignored failures (FAILS prefix)\n\n",
+           failing_count, failing_count == 1 ? "test" : "tests");
   }
 
 #if defined(OS_MACOSX)

base/test/test_suite.h

@@ -59,9 +59,6 @@ class TestSuite {
   // process exit code.
   static const char kStrictFailureHandling[];
 
-  // A command-line flag that silences all gtest output.
-  static const char kSilent[];
-
  protected:
   // This constructor is only accessible to specialized test suite
   // implementations which need to control the creation of an AtExitManager

chrome/browser/chrome_main_browsertest.cc

@@ -8,7 +8,6 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
-#include "base/test/test_suite.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/common/chrome_notification_types.h"
@@ -20,52 +19,25 @@
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/web_contents.h"
-#include "content/test/test_launcher.h"
 #include "net/base/net_util.h"
 
-// These tests don't apply to the Mac version; see
-// LaunchAnotherBrowserBlockUntilClosed for details.
+// These tests don't apply to the Mac version; see GetCommandLineForRelaunch
+// for details.
 #if !defined(OS_MACOSX)
 
 class ChromeMainTest : public InProcessBrowserTest {
  public:
-  ChromeMainTest()
-      : InProcessBrowserTest(),
-        new_command_line_(CommandLine::ForCurrentProcess()->GetProgram()) {
-  }
-
-  virtual void SetUpOnMainThread() OVERRIDE {
-    CommandLine::SwitchMap switches =
-        CommandLine::ForCurrentProcess()->GetSwitches();
-    switches.erase(switches::kUserDataDir);
-    switches.erase(test_launcher::kGTestFilterFlag);
-
-    for (CommandLine::SwitchMap::const_iterator iter = switches.begin();
-          iter != switches.end(); ++iter) {
-      new_command_line_.AppendSwitchNative((*iter).first, (*iter).second);
-    }
-
-    FilePath user_data_dir;
-    PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
-    new_command_line_.AppendSwitchPath(switches::kUserDataDir, user_data_dir);
+  ChromeMainTest() {}
 
-    new_command_line_.AppendSwitchASCII(
-        test_launcher::kGTestFilterFlag, test_launcher::kEmptyTestName);
-    new_command_line_.AppendSwitch(TestSuite::kSilent);
+  void Relaunch(const CommandLine& new_command_line) {
+    base::LaunchProcess(new_command_line, base::LaunchOptions(), NULL);
   }
-
-  void Relaunch() {
-    base::LaunchProcess(new_command_line_, base::LaunchOptions(), NULL);
-  }
-
- protected:
-  CommandLine new_command_line_;
 };
 
 // Make sure that the second invocation creates a new window.
 IN_PROC_BROWSER_TEST_F(ChromeMainTest, SecondLaunch) {
   ui_test_utils::BrowserAddedObserver observer;
-  Relaunch();
+  Relaunch(GetCommandLineForRelaunch());
   observer.WaitForSingleNewBrowser();
   ASSERT_EQ(BrowserList::GetBrowserCount(browser()->profile()), 2u);
 }
@@ -73,11 +45,12 @@ IN_PROC_BROWSER_TEST_F(ChromeMainTest, SecondLaunch) {
 IN_PROC_BROWSER_TEST_F(ChromeMainTest, ReuseBrowserInstanceWhenOpeningFile) {
   FilePath test_file_path = ui_test_utils::GetTestFilePath(
       FilePath(), FilePath().AppendASCII("empty.html"));
-  new_command_line_.AppendArgPath(test_file_path);
+  CommandLine new_command_line(GetCommandLineForRelaunch());
+  new_command_line.AppendArgPath(test_file_path);
   ui_test_utils::WindowedNotificationObserver observer(
         chrome::NOTIFICATION_TAB_ADDED,
         content::NotificationService::AllSources());
-  Relaunch();
+  Relaunch(new_command_line);
   observer.Wait();
 
   GURL url = net::FilePathToFileURL(test_file_path);
@@ -94,14 +67,15 @@ IN_PROC_BROWSER_TEST_F(ChromeMainTest, SecondLaunchWithIncognitoUrl) {
   // Run with --incognito switch and an URL specified.
   FilePath test_file_path = ui_test_utils::GetTestFilePath(
       FilePath(), FilePath().AppendASCII("empty.html"));
-  new_command_line_.AppendSwitch(switches::kIncognito);
-  new_command_line_.AppendArgPath(test_file_path);
+  CommandLine new_command_line(GetCommandLineForRelaunch());
+  new_command_line.AppendSwitch(switches::kIncognito);
+  new_command_line.AppendArgPath(test_file_path);
 
-  Relaunch();
+  Relaunch(new_command_line);
 
   // There should be one normal and one incognito window now.
   ui_test_utils::BrowserAddedObserver observer;
-  Relaunch();
+  Relaunch(new_command_line);
   observer.WaitForSingleNewBrowser();
   ASSERT_EQ(2u, BrowserList::size());
 
@@ -136,11 +110,12 @@ IN_PROC_BROWSER_TEST_F(ChromeMainTest, SecondLaunchFromIncognitoWithNormalUrl) {
   // Run with just an URL specified, no --incognito switch.
   FilePath test_file_path = ui_test_utils::GetTestFilePath(
       FilePath(), FilePath().AppendASCII("empty.html"));
-  new_command_line_.AppendArgPath(test_file_path);
+  CommandLine new_command_line(GetCommandLineForRelaunch());
+  new_command_line.AppendArgPath(test_file_path);
   ui_test_utils::WindowedNotificationObserver tab_observer(
         chrome::NOTIFICATION_TAB_ADDED,
         content::NotificationService::AllSources());
-  Relaunch();
+  Relaunch(new_command_line);
   tab_observer.Wait();
 
   // There should be one normal and one incognito window now.

chrome/browser/sessions/session_service.cc

@@ -198,7 +198,8 @@ SessionService::SessionService(Profile* profile)
       move_on_new_browser_(false),
       save_delay_in_millis_(base::TimeDelta::FromMilliseconds(2500)),
       save_delay_in_mins_(base::TimeDelta::FromMinutes(10)),
-      save_delay_in_hrs_(base::TimeDelta::FromHours(8)) {
+      save_delay_in_hrs_(base::TimeDelta::FromHours(8)),
+      force_browser_not_alive_with_no_windows_(false) {
   Init();
 }
 
@@ -208,7 +209,8 @@ SessionService::SessionService(const FilePath& save_path)
       move_on_new_browser_(false),
       save_delay_in_millis_(base::TimeDelta::FromMilliseconds(2500)),
       save_delay_in_mins_(base::TimeDelta::FromMinutes(10)),
-      save_delay_in_hrs_(base::TimeDelta::FromHours(8)) {
+      save_delay_in_hrs_(base::TimeDelta::FromHours(8)),
+      force_browser_not_alive_with_no_windows_(false)  {
   Init();
 }
 
@@ -525,7 +527,10 @@ bool SessionService::ShouldNewWindowStartSession() {
   // ChromeOS and OSX have different ideas of application lifetime than
   // the other platforms.
   // On ChromeOS opening a new window should never start a new session.
-#if !defined(OS_CHROMEOS)
+#if defined(OS_CHROMEOS)
+  if (!force_browser_not_alive_with_no_windows_)
+    return false;
+#endif
   if (!has_open_trackable_browsers_ &&
       !BrowserInit::InSynchronousProfileLaunch() &&
       !SessionRestore::IsRestoring(profile())
@@ -537,7 +542,6 @@ bool SessionService::ShouldNewWindowStartSession() {
       ) {
     return true;
   }
-#endif  // !OS_CHROMEOS
   return false;
 }
 

chrome/browser/sessions/session_service.h

@@ -53,7 +53,8 @@ class NavigationEntry;
 // of the browser.
 class SessionService : public BaseSessionService,
                        public content::NotificationObserver {
-  friend class SessionServiceTestHelper;
+  friend class SessionRestoreTest;
+  friend class SessionServiceTestHelper;  
  public:
   // Used to distinguish an application window from a normal one.
   enum AppType {
@@ -387,6 +388,7 @@ class SessionService : public BaseSessionService,
     // windows or similar. In this case, we record the close as pending.
     return !has_open_trackable_browsers_ &&
         (!browser_defaults::kBrowserAliveWithNoWindows ||
+         force_browser_not_alive_with_no_windows_ ||
          BrowserList::size() > 1);
   }
 
@@ -462,6 +464,10 @@ class SessionService : public BaseSessionService,
   const base::TimeDelta save_delay_in_mins_;
   const base::TimeDelta save_delay_in_hrs_;
 
+  // For browser_tests, since we want to simulate the browser shutting down
+  // without quitting.
+  bool force_browser_not_alive_with_no_windows_;
+
   DISALLOW_COPY_AND_ASSIGN(SessionService);
 };
 

chrome/chrome_tests.gypi

@@ -715,7 +715,6 @@
         # New tests should be browser_tests. browser_tests are sharded and are
         # less flakier.
         'browser/process_singleton_linux_uitest.cc',
-        'browser/sessions/session_restore_uitest.cc',
         'browser/ui/tests/browser_uitest.cc',
         'browser/ui/webui/bookmarks_ui_uitest.cc',
         'browser/ui/webui/ntp/new_tab_ui_uitest.cc',

chrome/test/automation/proxy_launcher.cc

@@ -214,13 +214,6 @@ bool ProxyLauncher::LaunchBrowser(const LaunchState& state) {
   return true;
 }
 
-#if !defined(OS_MACOSX)
-bool ProxyLauncher::LaunchAnotherBrowserBlockUntilClosed(
-    const LaunchState& state) {
-  return LaunchBrowserHelper(state, false, true, NULL);
-}
-#endif
-
 void ProxyLauncher::QuitBrowser() {
   // If we have already finished waiting for the browser to exit
   // (or it hasn't launched at all), there's nothing to do here.

chrome/test/automation/proxy_launcher.h

@@ -89,19 +89,9 @@ class ProxyLauncher {
   void CloseBrowserAndServer();
 
   // Launches the browser with the given command line. Returns true on success.
-  // TODO(phajdan.jr): Make LaunchBrowser private. Tests should use
-  // LaunchAnotherBrowserBlockUntilClosed.
+  // TODO(phajdan.jr): Make LaunchBrowser private.
   bool LaunchBrowser(const LaunchState& state) WARN_UNUSED_RESULT;
 
-#if !defined(OS_MACOSX)
-  // This function is not defined on the Mac because the concept
-  // doesn't apply to Mac; you can't have N browser processes.
-
-  // Launches another browser process and waits for it to finish.
-  // Returns true on success.
-  bool LaunchAnotherBrowserBlockUntilClosed(const LaunchState& state);
-#endif
-
   // Exits out of browser instance.
   void QuitBrowser();
 

chrome/test/base/chrome_test_launcher.cc

@@ -38,9 +38,14 @@ class ChromeTestLauncherDelegate : public test_launcher::TestLauncherDelegate {
   }
 
   virtual bool Run(int argc, char** argv, int* return_code) OVERRIDE {
-#if defined(OS_WIN)
+#if defined(OS_WIN) || defined(OS_LINUX)
     CommandLine* command_line = CommandLine::ForCurrentProcess();
-    if (command_line->HasSwitch(switches::kProcessType)) {
+    bool launch_chrome =
+        command_line->HasSwitch(switches::kProcessType) ||
+        command_line->HasSwitch(ChromeTestSuite::kLaunchAsBrowser);
+#endif
+#if defined(OS_WIN)
+    if (launch_chrome) {
       sandbox::SandboxInterfaceInfo sandbox_info = {0};
       content::InitializeSandboxInfo(&sandbox_info);
       ChromeMainDelegate chrome_main_delegate;
@@ -50,8 +55,7 @@ class ChromeTestLauncherDelegate : public test_launcher::TestLauncherDelegate {
       return true;
     }
 #elif defined(OS_LINUX)
-    CommandLine* command_line = CommandLine::ForCurrentProcess();
-    if (command_line->HasSwitch(switches::kProcessType)) {
+    if (launch_chrome) {
       ChromeMainDelegate chrome_main_delegate;
       *return_code = content::ContentMain(argc,
                                           const_cast<const char**>(argv),

chrome/test/base/chrome_test_suite.cc

@@ -151,6 +151,8 @@ class ChromeTestSuiteInitializer : public testing::EmptyTestEventListener {
 
 }  // namespace
 
+const char ChromeTestSuite::kLaunchAsBrowser[] = "as-browser";
+
 ChromeTestSuite::ChromeTestSuite(int argc, char** argv)
     : base::TestSuite(argc, argv) {
 }

chrome/test/base/chrome_test_suite.h

@@ -22,6 +22,9 @@ class StatsTable;
 
 class ChromeTestSuite : public base::TestSuite {
  public:
+  // Make the browser_test binary launch as a regular browser.
+  static const char kLaunchAsBrowser[];
+
   ChromeTestSuite(int argc, char** argv);
   virtual ~ChromeTestSuite();
 

chrome/test/base/in_process_browser_test.cc

@@ -27,6 +27,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/test/base/chrome_test_suite.h"
 #include "chrome/test/base/test_launcher_utils.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -35,6 +36,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/renderer/mock_content_renderer_client.h"
 #include "content/test/test_browser_thread.h"
+#include "content/test/test_launcher.h"
 #include "net/base/mock_host_resolver.h"
 #include "net/test/test_server.h"
 
@@ -277,6 +279,29 @@ void InProcessBrowserTest::AddBlankTabAndShow(Browser* browser) {
   browser->window()->Show();
 }
 
+#if !defined(OS_MACOSX)
+CommandLine InProcessBrowserTest::GetCommandLineForRelaunch() {
+  CommandLine new_command_line(CommandLine::ForCurrentProcess()->GetProgram());
+  CommandLine::SwitchMap switches =
+      CommandLine::ForCurrentProcess()->GetSwitches();
+  switches.erase(switches::kUserDataDir);
+  switches.erase(test_launcher::kSingleProcessTestsFlag);
+  switches.erase(test_launcher::kSingleProcessTestsAndChromeFlag);
+  switches.erase(test_launcher::kSingleProcessTestsFlag);
+  new_command_line.AppendSwitch(ChromeTestSuite::kLaunchAsBrowser);
+
+  FilePath user_data_dir;
+  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+  new_command_line.AppendSwitchPath(switches::kUserDataDir, user_data_dir);
+
+  for (CommandLine::SwitchMap::const_iterator iter = switches.begin();
+        iter != switches.end(); ++iter) {
+    new_command_line.AppendSwitchNative((*iter).first, (*iter).second);
+  }
+  return new_command_line;
+}
+#endif
+
 #if defined(OS_POSIX)
 // On SIGTERM (sent by the runner on timeouts), dump a stack trace (to make
 // debugging easier) and also exit with a known error code (so that the test

chrome/test/base/in_process_browser_test.h

@@ -160,6 +160,16 @@ class InProcessBrowserTest : public BrowserTestBase {
   // the navigation to complete, and show the browser's window.
   void AddBlankTabAndShow(Browser* browser);
 
+#if !defined OS_MACOSX
+  // Return a CommandLine object  that is used to relaunch the browser_test binary
+  // as a browser process. This function is deliberately not defined on the Mac
+  // because re-using an existing browser process when launching from the command
+  // line isn't a concept that we support on the Mac; AppleEvents are the Mac
+  // solution for the same need. Any test based on these functions doesn't apply
+  // to the Mac.
+  CommandLine GetCommandLineForRelaunch();
+#endif
+
   // Returns the host resolver being used for the tests. Subclasses might want
   // to configure it inside tests.
   net::RuleBasedHostResolverProc* host_resolver() {

chrome/test/ui/ui_test.cc

@@ -250,42 +250,6 @@ void UITestBase::LaunchBrowser(const CommandLine& arguments,
   ASSERT_TRUE(launcher_->LaunchBrowser(state));
 }
 
-#if !defined(OS_MACOSX)
-bool UITestBase::LaunchAnotherBrowserBlockUntilClosed(
-    const CommandLine& cmdline) {
-  ProxyLauncher::LaunchState state = DefaultLaunchState();
-  state.command.AppendArguments(cmdline, false);
-  return launcher_->LaunchAnotherBrowserBlockUntilClosed(state);
-}
-
-bool UITestBase::LaunchAnotherBrowserNoUrlArg(const CommandLine& cmdline) {
-  // Clear the homepage temporarily, and reset the launch switches, so that the
-  // URL argument doesn't get added.
-
-  std::string homepage_original;
-  std::swap(homepage_original, homepage_);
-
-  CommandLine launch_arguments_original(launch_arguments_);
-  launch_arguments_ = CommandLine(launch_arguments_.GetProgram());
-
-  SetLaunchSwitches();
-
-  ProxyLauncher::LaunchState state = DefaultLaunchState();
-
-  // But do add the --homepage switch
-  state.command.AppendSwitchASCII(switches::kHomePage, homepage_original);
-
-  state.command.AppendArguments(cmdline, false);
-  bool result = launcher_->LaunchAnotherBrowserBlockUntilClosed(state);
-
-  // Reset launch_arguments_ and homepage_ to their original values.
-  std::swap(homepage_original, homepage_);
-  std::swap(launch_arguments_original, launch_arguments_);
-
-  return result;
-}
-#endif
-
 void UITestBase::QuitBrowser() {
   launcher_->QuitBrowser();
 }

chrome/test/ui/ui_test.h

@@ -78,25 +78,9 @@ class UITestBase {
   void CloseBrowserAndServer();
 
   // Launches the browser with the given command line.
-  // TODO(phajdan.jr): Make LaunchBrowser private. Tests should use
-  // LaunchAnotherBrowserBlockUntilClosed.
+  // TODO(phajdan.jr): Make LaunchBrowser private.
   void LaunchBrowser(const CommandLine& cmdline, bool clear_profile);
 
-#if !defined(OS_MACOSX)
-  // These functions are deliberately not defined on the Mac because re-using an
-  // existing browser process when launching from the command line isn't a
-  // concept that we support on the Mac; AppleEvents are the Mac solution for
-  // the same need. Any test based on these functions doesn't apply to the Mac.
-
-  // Launches an another browser process and waits for it to finish. Returns
-  // true on success.
-  bool LaunchAnotherBrowserBlockUntilClosed(const CommandLine& cmdline);
-
-  // Same as LaunchAnotherBrowserBlockUntilClosed except that the URL argument
-  // is not used.
-  bool LaunchAnotherBrowserNoUrlArg(const CommandLine& cmdline);
-#endif
-
   // Exits out browser instance.
   void QuitBrowser();
 

content/test/test_launcher.cc

@@ -52,6 +52,8 @@ const FilePath::CharType kDefaultOutputFile[] = FILE_PATH_LITERAL(
 // Quit test execution after this number of tests has timed out.
 const int kMaxTimeouts = 5;  // 45s timeout * (5 + 1) = 270s max run time.
 
+const char kEmptyTestName[] = "InProcessBrowserTest.Empty";
+
 namespace {
 
 // An empty test (it starts up and shuts down the browser as part of its
@@ -556,8 +558,6 @@ void PrintUsage() {
 
 }  // namespace
 
-const char kEmptyTestName[] = "InProcessBrowserTest.Empty";
-
 const char kGTestFilterFlag[] = "gtest_filter";
 const char kGTestHelpFlag[]   = "gtest_help";
 const char kGTestListTestsFlag[] = "gtest_list_tests";