GTTF: waste less time if there are problems with launching the app in ProxyLauncher

BUG=90489

Review URL: http://codereview.chromium.org/7521010

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

3 files changed, 65 insertions, 39 deletions

diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc
index 10303ff..2b14cd7 100644
--- a/chrome/test/automation/automation_proxy_uitest.cc
+++ b/chrome/test/automation/automation_proxy_uitest.cc
@@ -63,9 +63,9 @@ class ExternalTabUITestMockLauncher : public ProxyLauncher {
     return *mock_;
   }
 
-  void InitializeConnection(const LaunchState& state,
-                            bool wait_for_initial_loads) {
-    ASSERT_TRUE(LaunchBrowserAndServer(state, wait_for_initial_loads));
+  bool InitializeConnection(const LaunchState& state,
+                            bool wait_for_initial_loads) WARN_UNUSED_RESULT {
+    return LaunchBrowserAndServer(state, wait_for_initial_loads);
   }
 
   void TerminateConnection() {
diff --git a/chrome/test/automation/proxy_launcher.cc b/chrome/test/automation/proxy_launcher.cc
index eea9b418..0a803d4 100644
--- a/chrome/test/automation/proxy_launcher.cc
+++ b/chrome/test/automation/proxy_launcher.cc
@@ -25,6 +25,7 @@
 #include "chrome/test/ui/ui_test.h"
 #include "content/common/child_process_info.h"
 #include "content/common/debug_flags.h"
+#include "content/common/result_codes.h"
 #include "sql/connection.h"
 
 namespace {
@@ -85,11 +86,14 @@ ProxyLauncher::ProxyLauncher()
 ProxyLauncher::~ProxyLauncher() {}
 
 bool ProxyLauncher::WaitForBrowserLaunch(bool wait_for_initial_loads) {
-  AutomationLaunchResult app_launched = automation_proxy_->WaitForAppLaunch();
-  EXPECT_EQ(AUTOMATION_SUCCESS, app_launched)
-      << "Error while awaiting automation ping from browser process";
-  if (app_launched != AUTOMATION_SUCCESS)
+  if (automation_proxy_->WaitForAppLaunch() != AUTOMATION_SUCCESS) {
+    LOG(ERROR) << "Error while awaiting automation ping from browser process. "
+               << "Killing the browser.";
+    DisconnectFromRunningBrowser();
+    TerminateAllChromeProcesses(process_id_);
+    CloseBrowserProcessHandles();
     return false;
+  }
 
   if (wait_for_initial_loads) {
     if (!automation_proxy_->WaitForInitialLoads()) {
@@ -150,7 +154,7 @@ void ProxyLauncher::CloseBrowserAndServer() {
 }
 
 void ProxyLauncher::DisconnectFromRunningBrowser() {
-  automation_proxy_.reset();  // Shut down IPC testing interface.
+  automation_proxy_.reset();
 }
 
 bool ProxyLauncher::LaunchBrowser(const LaunchState& state) {
@@ -259,8 +263,7 @@ void ProxyLauncher::QuitBrowser() {
 
   // Now, drop the automation IPC channel so that the automation provider in
   // the browser notices and drops its reference to the browser process.
-  if (automation_proxy_.get())
-    automation_proxy_->Disconnect();
+  DisconnectFromRunningBrowser();
 
   // Wait for the browser process to quit. It should quit once all tabs have
   // been closed.
@@ -289,8 +292,7 @@ void ProxyLauncher::TerminateBrowser() {
 
   // Now, drop the automation IPC channel so that the automation provider in
   // the browser notices and drops its reference to the browser process.
-  if (automation_proxy_.get())
-    automation_proxy_->Disconnect();
+  DisconnectFromRunningBrowser();
 
 #if defined(OS_POSIX)
   EXPECT_EQ(kill(process_, SIGTERM), 0);
@@ -328,10 +330,7 @@ bool ProxyLauncher::WaitForBrowserProcessToQuit(int timeout, int* exit_code) {
   if (!success)
     TerminateAllChromeProcesses(process_id_);
 
-  base::CloseProcessHandle(process_);
-  process_ = base::kNullProcessHandle;
-  process_id_ = -1;
-
+  CloseBrowserProcessHandles();
   return success;
 }
 
@@ -457,7 +456,6 @@ bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state, bool wait,
 
   base::LaunchOptions options;
   options.wait = wait;
-
 #if defined(OS_WIN)
   options.start_hidden = !state.show_window;
 #elif defined(OS_POSIX)
@@ -470,6 +468,12 @@ bool ProxyLauncher::LaunchBrowserHelper(const LaunchState& state, bool wait,
   return base::LaunchProcess(command_line, options, process);
 }
 
+void ProxyLauncher::CloseBrowserProcessHandles() {
+  base::CloseProcessHandle(process_);
+  process_ = base::kNullProcessHandle;
+  process_id_ = -1;
+}
+
 AutomationProxy* ProxyLauncher::automation() const {
   EXPECT_TRUE(automation_proxy_.get());
   return automation_proxy_.get();
@@ -514,7 +518,7 @@ AutomationProxy* NamedProxyLauncher::CreateAutomationProxy(
   return proxy;
 }
 
-void NamedProxyLauncher::InitializeConnection(const LaunchState& state,
+bool NamedProxyLauncher::InitializeConnection(const LaunchState& state,
                                               bool wait_for_initial_loads) {
   FilePath testing_channel_path;
 #if defined(OS_WIN)
@@ -526,10 +530,15 @@ void NamedProxyLauncher::InitializeConnection(const LaunchState& state,
   if (launch_browser_) {
     // Because we are waiting on the existence of the testing file below,
     // make sure there isn't one already there before browser launch.
-    EXPECT_TRUE(file_util::Delete(testing_channel_path, false));
+    if (!file_util::Delete(testing_channel_path, false)) {
+      LOG(ERROR) << "Failed to delete " << testing_channel_path.value();
+      return false;
+    }
 
-    // Set up IPC testing interface as a client.
-    ASSERT_TRUE(LaunchBrowser(state));
+    if (!LaunchBrowser(state)) {
+      LOG(ERROR) << "Failed to LaunchBrowser";
+      return false;
+    }
   }
 
   // Wait for browser to be ready for connections.
@@ -542,9 +551,17 @@ void NamedProxyLauncher::InitializeConnection(const LaunchState& state,
       break;
     base::PlatformThread::Sleep(automation::kSleepTime);
   }
-  EXPECT_TRUE(testing_channel_exists);
+  if (!testing_channel_exists) {
+    LOG(ERROR) << "Failed to wait for testing channel presence.";
+    return false;
+  }
+
+  if (!ConnectToRunningBrowser(wait_for_initial_loads)) {
+    LOG(ERROR) << "Failed to ConnectToRunningBrowser";
+    return false;
+  }
 
-  ASSERT_TRUE(ConnectToRunningBrowser(wait_for_initial_loads));
+  return true;
 }
 
 void NamedProxyLauncher::TerminateConnection() {
@@ -575,9 +592,9 @@ AutomationProxy* AnonymousProxyLauncher::CreateAutomationProxy(
   return proxy;
 }
 
-void AnonymousProxyLauncher::InitializeConnection(const LaunchState& state,
+bool AnonymousProxyLauncher::InitializeConnection(const LaunchState& state,
                                                   bool wait_for_initial_loads) {
-  ASSERT_TRUE(LaunchBrowserAndServer(state, wait_for_initial_loads));
+  return LaunchBrowserAndServer(state, wait_for_initial_loads);
 }
 
 void AnonymousProxyLauncher::TerminateConnection() {
diff --git a/chrome/test/automation/proxy_launcher.h b/chrome/test/automation/proxy_launcher.h
index d306a33..96620f4 100644
--- a/chrome/test/automation/proxy_launcher.h
+++ b/chrome/test/automation/proxy_launcher.h
@@ -65,8 +65,10 @@ class ProxyLauncher {
   virtual ~ProxyLauncher();
 
   // Launches the browser if needed and establishes a connection with it.
-  virtual void InitializeConnection(const LaunchState& state,
-                                    bool wait_for_initial_loads) = 0;
+  // Returns true on success.
+  virtual bool InitializeConnection(
+      const LaunchState& state,
+      bool wait_for_initial_loads) WARN_UNUSED_RESULT = 0;
 
   // Shuts down the browser if needed and destroys any
   // connections established by InitalizeConnection.
@@ -222,6 +224,8 @@ class ProxyLauncher {
                            bool wait,
                            base::ProcessHandle* process) WARN_UNUSED_RESULT;
 
+  void CloseBrowserProcessHandles();
+
   scoped_ptr<AutomationProxy> automation_proxy_;
 
   // We use a temporary directory for profile to avoid issues with being
@@ -285,13 +289,16 @@ class NamedProxyLauncher : public ProxyLauncher {
   // If launch_browser is true, launches Chrome with named interface enabled.
   // Otherwise, there should be an existing instance the proxy can connect to.
   NamedProxyLauncher(const std::string& channel_id,
-                     bool launch_browser, bool disconnect_on_failure);
+                     bool launch_browser,
+                     bool disconnect_on_failure) OVERRIDE;
 
-  virtual AutomationProxy* CreateAutomationProxy(int execution_timeout);
-  virtual void InitializeConnection(const LaunchState& state,
-                                    bool wait_for_initial_loads);
-  virtual void TerminateConnection();
-  virtual std::string PrefixedChannelID() const;
+  virtual AutomationProxy* CreateAutomationProxy(
+      int execution_timeout) OVERRIDE;
+  virtual bool InitializeConnection(
+      const LaunchState& state,
+      bool wait_for_initial_loads) OVERRIDE WARN_UNUSED_RESULT;
+  virtual void TerminateConnection() OVERRIDE;
+  virtual std::string PrefixedChannelID() const OVERRIDE;
 
  protected:
   std::string channel_id_;      // Channel id of automation proxy.
@@ -305,12 +312,14 @@ class NamedProxyLauncher : public ProxyLauncher {
 // Uses an automation proxy that communicates over an anonymous socket.
 class AnonymousProxyLauncher : public ProxyLauncher {
  public:
-  explicit AnonymousProxyLauncher(bool disconnect_on_failure);
-  virtual AutomationProxy* CreateAutomationProxy(int execution_timeout);
-  virtual void InitializeConnection(const LaunchState& state,
-                                    bool wait_for_initial_loads);
-  virtual void TerminateConnection();
-  virtual std::string PrefixedChannelID() const;
+  explicit AnonymousProxyLauncher(bool disconnect_on_failure) OVERRIDE;
+  virtual AutomationProxy* CreateAutomationProxy(
+      int execution_timeout) OVERRIDE;
+  virtual bool InitializeConnection(
+      const LaunchState& state,
+      bool wait_for_initial_loads) OVERRIDE WARN_UNUSED_RESULT;
+  virtual void TerminateConnection() OVERRIDE;
+  virtual std::string PrefixedChannelID() const OVERRIDE;
 
  protected:
   std::string channel_id_;      // Channel id of automation proxy.