Wait properly for renderer crashes

chrome/test/automation/proxy_launcher.cc

 // Copyright (c) 2011 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/test/automation/proxy_launcher.h"
 
 #include "base/environment.h"
 #include "base/file_util.h"
 #include "base/string_number_conversions.h"
 #include "base/string_split.h"
 #include "base/stringprintf.h"
 #include "base/test/test_file_util.h"
 #include "base/test/test_timeouts.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/common/automation_constants.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/logging_chrome.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/automation/automation_proxy.h"
 #include "chrome/test/base/chrome_process_util.h"
 #include "chrome/test/base/test_launcher_utils.h"
 #include "chrome/test/base/test_switches.h"
 #include "chrome/test/ui/ui_test.h"
 #include "content/common/child_process_info.h"
 #include "content/common/debug_flags.h"
 #include "ipc/ipc_channel.h"
+#include "ipc/ipc_descriptors.h"
 #include "sql/connection.h"
 
 namespace {
 
 // Passed as value of kTestType.
 const char kUITestType[] = "ui";
 
 // We want to have a current history database when we start the browser so
 // things like the NTP will have thumbnails.  This method updates the dates
 // in the history to be more recent.
@@ skipping 63 matching lines @@
       << "Error while awaiting automation ping from browser process";
   if (app_launched != AUTOMATION_SUCCESS)
     return false;
 
   if (wait_for_initial_loads) {
     if (!automation_proxy_->WaitForInitialLoads()) {
       LOG(ERROR) << "WaitForInitialLoads failed.";
       return false;
     }
   } else {
-    // TODO(phajdan.jr): We should get rid of this sleep, but some tests
-    // "rely" on it, e.g. AssertionTest.Assertion and CheckFalseTest.CheckFails.
-    // Those tests do not wait in any way until the crash gets noticed,
-    // so it's possible for the browser to exit before the tested crash happens.
+#if defined(OS_WIN)
+    // TODO(phajdan.jr): Get rid of this Sleep when logging_chrome_uitest
+    // stops "relying" on it.
     base::PlatformThread::Sleep(TestTimeouts::action_timeout_ms());
+#endif
   }
 
   if (!automation()->SetFilteredInet(ShouldFilterInet())) {
     LOG(ERROR) << "SetFilteredInet failed.";
     return false;
   }
 
   return true;
 }
 
@@ skipping 64 matching lines @@
     }
 
     // Update the history file to include recent dates.
     UpdateHistoryDates(user_data_dir());
   }
 
   // Optionally do any final setup of the test environment.
   if (!state.setup_profile_callback.is_null())
     state.setup_profile_callback.Run();
 
-  if (!LaunchBrowserHelper(state, false, &process_)) {
+  if (!LaunchBrowserHelper(state, true, false, &process_)) {
     LOG(ERROR) << "LaunchBrowserHelper failed.";
     return false;
   }
   process_id_ = base::GetProcId(process_);
 
   return true;
 }
 
 #if !defined(OS_MACOSX)
 bool ProxyLauncher::LaunchAnotherBrowserBlockUntilClosed(
     const LaunchState& state) {
-  return LaunchBrowserHelper(state, true, NULL);
+  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.
   if (process_ == base::kNullProcessHandle)
     return;
 
   if (SESSION_ENDING == shutdown_type_) {
@@ skipping 203 matching lines @@
   // Disable TabCloseableStateWatcher for tests.
   command_line->AppendSwitch(switches::kDisableTabCloseableStateWatcher);
 
   // Allow file:// access on ChromeOS.
   command_line->AppendSwitch(switches::kAllowFileAccess);
 
   // Allow testing File API over http.
   command_line->AppendSwitch(switches::kUnlimitedQuotaForFiles);
 }
 
-bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state, bool wait,
+bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state,
+                                        bool main_launch,
+                                        bool wait,
                                         base::ProcessHandle* process) {
   CommandLine command_line(state.command);
 
   // Add command line arguments that should be applied to all UI tests.
   PrepareTestCommandline(&command_line, state.include_testing_id);
   DebugFlags::ProcessDebugFlags(
       &command_line, ChildProcessInfo::UNKNOWN_PROCESS, false);
 
   // Sometimes one needs to run the browser under a special environment
   // (e.g. valgrind) without also running the test harness (e.g. python)
   // under the special environment.  Provide a way to wrap the browser
   // commandline with a special prefix to invoke the special environment.
   const char* browser_wrapper = getenv("BROWSER_WRAPPER");
   if (browser_wrapper) {
 #if defined(OS_WIN)
     command_line.PrependWrapper(ASCIIToWide(browser_wrapper));
 #elif defined(OS_POSIX)
     command_line.PrependWrapper(browser_wrapper);
 #endif
     VLOG(1) << "BROWSER_WRAPPER was set, prefixing command_line with "
             << browser_wrapper;
   }
 
-  // TODO(phajdan.jr): Only run it for "main" browser launch.
-  browser_launch_time_ = base::TimeTicks::Now();
+  if (main_launch)
+    browser_launch_time_ = base::TimeTicks::Now();
 
   base::LaunchOptions options;
   options.wait = wait;
 
 #if defined(OS_WIN)
   options.start_hidden = !state.show_window;
 #elif defined(OS_POSIX)
+  int ipcfd = -1;
+  file_util::ScopedFD ipcfd_closer(&ipcfd);
   base::file_handle_mapping_vector fds;
-  if (automation_proxy_.get())
-    fds = automation_proxy_->fds_to_map();
-  options.fds_to_remap = &fds;
+  if (main_launch && automation_proxy_.get()) {
+    ipcfd = automation_proxy_->channel()->TakeClientFileDescriptor();
+    fds.push_back(std::make_pair(ipcfd, kPrimaryIPCChannel + 3));

[darin (slow to review), 2011/09/19 15:18:54]

looks like this worked out well... 

+    options.fds_to_remap = &fds;
+  }
 #endif
 
   return base::LaunchProcess(command_line, options, process);
 }
 
 AutomationProxy* ProxyLauncher::automation() const {
   EXPECT_TRUE(automation_proxy_.get());
   return automation_proxy_.get();
 }
 
@@ skipping 109 matching lines @@
   return LaunchBrowserAndServer(state, wait_for_initial_loads);
 }
 
 void AnonymousProxyLauncher::TerminateConnection() {
   CloseBrowserAndServer();
 }
 
 std::string AnonymousProxyLauncher::PrefixedChannelID() const {
   return channel_id_;
 }