GTTF: After timeout, all further automation calls should fail immediately.

If IPC send fails, further automation calls are extremely likely to fail.
Avoid wasting a lot of time on further timeouts by closing the channel
immediately on the first error.

TEST=ui_tests
BUG=51346

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

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

6 files changed, 40 insertions, 11 deletions

diff --git a/chrome/test/automation/automation_proxy.cc b/chrome/test/automation/automation_proxy.cc
index 6cc574b..31e339b 100644
--- a/chrome/test/automation/automation_proxy.cc
+++ b/chrome/test/automation/automation_proxy.cc
@@ -91,13 +91,15 @@ class AutomationMessageFilter : public IPC::ChannelProxy::MessageFilter {
 }  // anonymous namespace
 
 
-AutomationProxy::AutomationProxy(int command_execution_timeout_ms)
+AutomationProxy::AutomationProxy(int command_execution_timeout_ms,
+                                 bool disconnect_on_failure)
     : app_launched_(true, false),
       initial_loads_complete_(true, false),
       new_tab_ui_load_complete_(true, false),
       shutdown_event_(new base::WaitableEvent(true, false)),
       app_launch_signaled_(0),
       perform_version_check_(false),
+      disconnect_on_failure_(disconnect_on_failure),
       command_execution_timeout_(
           TimeDelta::FromMilliseconds(command_execution_timeout_ms)),
       listener_thread_id_(0) {
@@ -113,12 +115,10 @@ AutomationProxy::AutomationProxy(int command_execution_timeout_ms)
 }
 
 AutomationProxy::~AutomationProxy() {
-  DCHECK(shutdown_event_.get() != NULL);
-  shutdown_event_->Signal();
   // Destruction order is important. Thread has to outlive the channel and
   // tracker has to outlive the thread since we access the tracker inside
   // AutomationMessageFilter::OnMessageReceived.
-  channel_.reset();
+  Disconnect();
   thread_.reset();
   tracker_.reset();
 }
@@ -379,6 +379,8 @@ bool AutomationProxy::SendProxyConfig(const std::string& new_proxy_config) {
 }
 
 void AutomationProxy::Disconnect() {
+  DCHECK(shutdown_event_.get() != NULL);
+  shutdown_event_->Signal();
   channel_.reset();
 }
 
@@ -389,7 +391,9 @@ void AutomationProxy::OnMessageReceived(const IPC::Message& msg) {
 }
 
 void AutomationProxy::OnChannelError() {
-  DLOG(ERROR) << "Channel error in AutomationProxy.";
+  LOG(ERROR) << "Channel error in AutomationProxy.";
+  if (disconnect_on_failure_)
+    Disconnect();
 }
 
 scoped_refptr<WindowProxy> AutomationProxy::GetActiveWindow() {
@@ -450,7 +454,19 @@ bool AutomationProxy::Send(IPC::Message* message) {
     return false;
   }
 
-  return channel_->SendWithTimeout(message, command_execution_timeout_ms());
+  bool success = channel_->SendWithTimeout(message,
+                                           command_execution_timeout_ms());
+
+  if (!success && disconnect_on_failure_) {
+    // Send failed (possibly due to a timeout). Browser is likely in a weird
+    // state, and further IPC requests are extremely likely to fail (possibly
+    // timeout, which would make tests slower). Disconnect the channel now
+    // to avoid the slowness.
+    LOG(ERROR) << "Disconnecting channel after error!";
+    Disconnect();
+  }
+
+  return success;
 }
 
 void AutomationProxy::InvalidateHandle(const IPC::Message& message) {
diff --git a/chrome/test/automation/automation_proxy.h b/chrome/test/automation/automation_proxy.h
index 8b864aa..344c248 100644
--- a/chrome/test/automation/automation_proxy.h
+++ b/chrome/test/automation/automation_proxy.h
@@ -58,7 +58,7 @@ class AutomationMessageSender : public IPC::Message::Sender {
 class AutomationProxy : public IPC::Channel::Listener,
                         public AutomationMessageSender {
  public:
-  explicit AutomationProxy(int command_execution_timeout_ms);
+  AutomationProxy(int command_execution_timeout_ms, bool disconnect_on_failure);
   virtual ~AutomationProxy();
 
   // IPC callback
@@ -293,6 +293,10 @@ class AutomationProxy : public IPC::Channel::Listener,
   // a version resource.
   bool perform_version_check_;
 
+  // If true, the proxy will disconnect the IPC channel on first failure
+  // to send an IPC message. This helps avoid long timeouts in tests.
+  bool disconnect_on_failure_;
+
   // Delay to let the browser execute the command.
   base::TimeDelta command_execution_timeout_;
 
diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc
index 4493979..96e43dd 100644
--- a/chrome/test/automation/automation_proxy_uitest.cc
+++ b/chrome/test/automation/automation_proxy_uitest.cc
@@ -624,7 +624,7 @@ const char simple_data_url[] =
     "</body></html>";
 
 ExternalTabUITestMockClient::ExternalTabUITestMockClient(int execution_timeout)
-    : AutomationProxy(execution_timeout),
+    : AutomationProxy(execution_timeout, false),
       host_window_style_(WS_OVERLAPPEDWINDOW | WS_CLIPCHILDREN | WS_VISIBLE),
       host_window_(NULL) {
 }
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index 0fc08ad..b321ebf 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -265,8 +265,7 @@ void UITestBase::InitializeTimeouts() {
 }
 
 AutomationProxy* UITestBase::CreateAutomationProxy(int execution_timeout) {
-  // By default we create a plain vanilla AutomationProxy.
-  return new AutomationProxy(execution_timeout);
+  return new AutomationProxy(execution_timeout, false);
 }
 
 void UITestBase::LaunchBrowserAndServer() {
@@ -1540,3 +1539,11 @@ void UITest::TearDown() {
   UITestBase::TearDown();
   PlatformTest::TearDown();
 }
+
+AutomationProxy* UITest::CreateAutomationProxy(int execution_timeout) {
+  // Make the AutomationProxy disconnect the channel on the first error,
+  // so that we avoid spending a lot of time in timeouts. The browser is likely
+  // hosed if we hit those errors.
+  return new AutomationProxy(execution_timeout, true);
+}
+
diff --git a/chrome/test/ui/ui_test.h b/chrome/test/ui/ui_test.h
index db826f6..f10df9b 100644
--- a/chrome/test/ui/ui_test.h
+++ b/chrome/test/ui/ui_test.h
@@ -634,6 +634,8 @@ class UITest : public UITestBase, public PlatformTest {
   virtual void SetUp();
   virtual void TearDown();
 
+  virtual AutomationProxy* CreateAutomationProxy(int execution_timeout);
+
  private:
   MessageLoop message_loop_;  // Enables PostTask to main thread.
 };
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index 32fcfb0..6ede7e3 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -128,7 +128,7 @@ class ChromeFrameAutomationProxyImpl::CFMsgDispatcher
 
 ChromeFrameAutomationProxyImpl::ChromeFrameAutomationProxyImpl(
     AutomationProxyCacheEntry* entry, int launch_timeout)
-    : AutomationProxy(launch_timeout), proxy_entry_(entry) {
+    : AutomationProxy(launch_timeout, false), proxy_entry_(entry) {
   TRACE_EVENT_BEGIN("chromeframe.automationproxy", this, "");
 
   sync_ = new CFMsgDispatcher();