Reland 97557; Original review at: http://codereview.chromium.org/7648053

[chromedriver] Add chrome.detach option for configuring Chrome not to quit
when its automation client disconnects. chrome.detach is currently implemented
by using a NamedProxyLauncher. We also add a new switch that accomplishes
the same purpose.
Also, modify pre-post command execution steps in two ways:
1) wait for loads post execution, in case pyauto issues a non-webdriver
command which expects the load to be finished
2) only return a frame checking error if no alert is present
BUG=87676, 93438
TEST=none

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

18 files changed, 145 insertions, 12 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index aac9147..eeaf263 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -7,6 +7,7 @@
 #include <set>
 
 #include "base/callback.h"
+#include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/file_path.h"
 #include "base/json/json_reader.h"
@@ -105,7 +106,9 @@ using base::Time;
 AutomationProvider::AutomationProvider(Profile* profile)
     : profile_(profile),
       reply_message_(NULL),
-      reinitialize_on_channel_error_(false),
+      reinitialize_on_channel_error_(
+          CommandLine::ForCurrentProcess()->HasSwitch(
+              switches::kAutomationReinitializeOnChannelError)),
       is_connected_(false),
       initial_tab_loads_complete_(false),
       network_library_initialized_(true) {
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 800efdc..a84c282 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -91,6 +91,11 @@ const char kAuthServerWhitelist[]           = "auth-server-whitelist";
 // automation-related messages on IPC channel with the given ID.
 const char kAutomationClientChannelID[]     = "automation-channel";
 
+// Causes the automation provider to reinitialize its IPC channel instead of
+// shutting down when a client disconnects.
+const char kAutomationReinitializeOnChannelError[] =
+    "automation-reinitialize-on-channel-error";
+
 // When the option to block third-party cookies from being set is enabled,
 // also block third-party cookies from being read.
 const char kBlockReadingThirdPartyCookies[] =
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 74ac8da..6bca51c 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -43,6 +43,7 @@ extern const char kAuthNegotiateDelegateWhitelist[];
 extern const char kAuthSchemes[];
 extern const char kAuthServerWhitelist[];
 extern const char kAutomationClientChannelID[];
+extern const char kAutomationReinitializeOnChannelError[];
 extern const char kBlockReadingThirdPartyCookies[];
 extern const char kBrowserAssertTest[];
 extern const char kBrowserCrashTest[];
diff --git a/chrome/test/webdriver/commands/command.cc b/chrome/test/webdriver/commands/command.cc
index 95ad878..3b1ea07 100644
--- a/chrome/test/webdriver/commands/command.cc
+++ b/chrome/test/webdriver/commands/command.cc
@@ -29,6 +29,8 @@ bool Command::Init(Response* const response) {
   return true;
 }
 
+void Command::Finish() {}
+
 std::string Command::GetPathVariable(const size_t i) const {
   return i < path_segments_.size() ? path_segments_.at(i) : "";
 }
diff --git a/chrome/test/webdriver/commands/command.h b/chrome/test/webdriver/commands/command.h
index 672b2c0..8f735f8 100644
--- a/chrome/test/webdriver/commands/command.h
+++ b/chrome/test/webdriver/commands/command.h
@@ -15,6 +15,7 @@
 
 namespace webdriver {
 
+class Error;
 class Response;
 
 // Base class for a command mapped to a URL in the WebDriver REST API. Each
@@ -37,6 +38,11 @@ class Command {
   // to return to the client.
   virtual bool Init(Response* const response);
 
+  // Called after this command is executed. Returns NULL if no error occurs.
+  // This is only called if |Init| is successful and regardless of whether
+  // the execution results in a |Error|.
+  virtual void Finish();
+
   // Executes the corresponding variant of this command URL.
   // Always called after |Init()| and called from the Execute function.
   // Any failure is handled as a return code found in Response.
diff --git a/chrome/test/webdriver/commands/create_session.cc b/chrome/test/webdriver/commands/create_session.cc
index d673ead..b76760b8 100644
--- a/chrome/test/webdriver/commands/create_session.cc
+++ b/chrome/test/webdriver/commands/create_session.cc
@@ -197,6 +197,10 @@ void CreateSession::ExecutePost(Response* const response) {
     error = GetBooleanCapability(capabilities, "chrome.loadAsync",
                                  &session_options.load_async);
   }
+  if (!error) {
+    error = GetBooleanCapability(capabilities, "chrome.detach",
+                                 &browser_options.detach_process);
+  }
   if (error) {
     response->SetError(error);
     return;
diff --git a/chrome/test/webdriver/commands/response.cc b/chrome/test/webdriver/commands/response.cc
index 6b96bae..1c09d39 100644
--- a/chrome/test/webdriver/commands/response.cc
+++ b/chrome/test/webdriver/commands/response.cc
@@ -59,7 +59,7 @@ void Response::SetValue(Value* value) {
 
 void Response::SetError(Error* error) {
   DictionaryValue* error_dict = new DictionaryValue();
-  error_dict->SetString(kMessageKey, error->GetMessage());
+  error_dict->SetString(kMessageKey, error->GetErrorMessage());
 
   SetStatus(error->code());
   SetValue(error_dict);
diff --git a/chrome/test/webdriver/commands/webdriver_command.cc b/chrome/test/webdriver/commands/webdriver_command.cc
index 3220316..eed1d11c 100644
--- a/chrome/test/webdriver/commands/webdriver_command.cc
+++ b/chrome/test/webdriver/commands/webdriver_command.cc
@@ -50,4 +50,12 @@ bool WebDriverCommand::Init(Response* const response) {
   return true;
 }
 
+void WebDriverCommand::Finish() {
+  scoped_ptr<Error> error(session_->AfterExecuteCommand());
+  if (error.get()) {
+    LOG(WARNING) << "Command did not finish successfully: "
+                 << error->GetErrorMessage();
+  }
+}
+
 }  // namespace webdriver
diff --git a/chrome/test/webdriver/commands/webdriver_command.h b/chrome/test/webdriver/commands/webdriver_command.h
index 248aa58..765b59b 100644
--- a/chrome/test/webdriver/commands/webdriver_command.h
+++ b/chrome/test/webdriver/commands/webdriver_command.h
@@ -16,6 +16,7 @@ class DictionaryValue;
 
 namespace webdriver {
 
+class Error;
 class Response;
 class Session;
 
@@ -34,6 +35,8 @@ class WebDriverCommand : public Command {
   // Initializes this webdriver command by fetching the command session.
   virtual bool Init(Response* const response);
 
+  virtual void Finish();
+
  protected:
   Session* session_;
 
diff --git a/chrome/test/webdriver/test/alerts.html b/chrome/test/webdriver/test/alerts.html
index 93fc102..76d5bd0 100644
--- a/chrome/test/webdriver/test/alerts.html
+++ b/chrome/test/webdriver/test/alerts.html
@@ -3,5 +3,6 @@
     <input name='onkeypress' type='text' onkeypress='alert("ok")'></input>
     <input name='onkeyup' type='text' onkeyup='alert("ok")'></input>
     <input name='normal' type='text'></input>
+    <iframe id='subframe' src='iframe_src.html'>
   </body>
 </html>
diff --git a/chrome/test/webdriver/test/chromedriver_tests.py b/chrome/test/webdriver/test/chromedriver_tests.py
index f5a5d28..e87ff3ad 100644
--- a/chrome/test/webdriver/test/chromedriver_tests.py
+++ b/chrome/test/webdriver/test/chromedriver_tests.py
@@ -14,6 +14,7 @@ from distutils import archive_util
 import hashlib
 import os
 import platform
+import signal
 import subprocess
 import sys
 import tempfile
@@ -71,6 +72,14 @@ def IsMac():
   return sys.platform.startswith('darwin')
 
 
+def Kill(pid):
+  """Terminate the given pid."""
+  if IsWindows():
+    subprocess.call(['taskkill.exe', '/T', '/F', '/PID', str(pid)])
+  else:
+    os.kill(pid, signal.SIGTERM)
+
+
 class Request(urllib2.Request):
   """Extends urllib2.Request to support all HTTP request types."""
 
@@ -324,6 +333,32 @@ class DesiredCapabilitiesTest(ChromeDriverTest):
     self.assertNotEqual(-1, driver.page_source.find('ExtTest2'))
     driver.quit()
 
+
+class DetachProcessTest(unittest.TestCase):
+
+  def setUp(self):
+    self._server = ChromeDriverLauncher(test_paths.CHROMEDRIVER_EXE).Launch()
+    self._factory = ChromeDriverFactory(self._server)
+
+  def tearDown(self):
+    self._server.Kill()
+
+  # TODO(kkania): Remove this when Chrome 15 is stable.
+  def testDetachProcess(self):
+    # This is a weak test. Its purpose is to just make sure we can start
+    # Chrome successfully in detached mode. There's not an easy way to know
+    # if Chrome is shutting down due to the channel error when the client
+    # disconnects.
+    driver = self._factory.GetNewDriver({'chrome.detach': True})
+    driver.get('about:memory')
+    pid = int(driver.find_elements_by_xpath('//*[@jscontent="pid"]')[0].text)
+    self._server.Kill()
+    try:
+      Kill(pid)
+    except OSError:
+      self.fail('Chrome quit after detached chromedriver server was killed')
+
+
 class CookieTest(ChromeDriverTest):
   """Cookie test for the json webdriver protocol"""
 
@@ -794,6 +829,13 @@ class AlertTest(ChromeDriverTest):
     self.assertRaises(WebDriverException, driver.forward)
     self.assertRaises(WebDriverException, driver.get_screenshot_as_base64)
 
+  def testCanHandleAlertInSubframe(self):
+    driver = self.GetNewDriver()
+    driver.get(GetTestDataUrl() + '/alerts.html')
+    driver.switch_to_frame('subframe')
+    driver.execute_async_script('arguments[0](); window.alert("ok")')
+    driver.switch_to_alert().accept()
+
 
 """Chrome functional test section. All implementation tests of ChromeDriver
 should go above.
diff --git a/chrome/test/webdriver/webdriver_automation.cc b/chrome/test/webdriver/webdriver_automation.cc
index f410842..9d21837 100644
--- a/chrome/test/webdriver/webdriver_automation.cc
+++ b/chrome/test/webdriver/webdriver_automation.cc
@@ -184,7 +184,8 @@ bool GetDefaultChromeExe(FilePath* browser_exe) {
 namespace webdriver {
 
 Automation::BrowserOptions::BrowserOptions()
-    : command(CommandLine::NO_PROGRAM) {}
+    : command(CommandLine::NO_PROGRAM),
+      detach_process(false) {}
 
 Automation::BrowserOptions::~BrowserOptions() {}
 
@@ -201,6 +202,8 @@ void Automation::Init(const BrowserOptions& options, Error** error) {
   command.AppendSwitch(switches::kFullMemoryCrashReport);
   command.AppendSwitch(switches::kNoDefaultBrowserCheck);
   command.AppendSwitch(switches::kNoFirstRun);
+  if (options.detach_process)
+    command.AppendSwitch(switches::kAutomationReinitializeOnChannelError);
   if (options.user_data_dir.empty())
     command.AppendSwitchASCII(switches::kHomePage, chrome::kAboutBlankURL);
 
@@ -228,10 +231,35 @@ void Automation::Init(const BrowserOptions& options, Error** error) {
   LOG(INFO) << chrome_details;
 
   // Create the ProxyLauncher and launch Chrome.
-  if (options.channel_id.empty()) {
+  // In Chrome 13/14, the only way to detach the browser process is to use a
+  // named proxy launcher.
+  // TODO(kkania): Remove this when Chrome 15 is stable.
+  std::string channel_id = options.channel_id;
+  bool launch_browser = false;
+  if (options.detach_process) {
+    launch_browser = true;
+    if (!channel_id.empty()) {
+      *error = new Error(
+          kUnknownError, "Cannot detach an already running browser process");
+      return;
+    }
+#if defined(OS_WIN)
+    channel_id = "chromedriver" + GenerateRandomID();
+#elif defined(OS_POSIX)
+    FilePath temp_file;
+    if (!file_util::CreateTemporaryFile(&temp_file) ||
+        !file_util::Delete(temp_file, false /* recursive */)) {
+      *error = new Error(kUnknownError, "Could not create temporary filename");
+      return;
+    }
+    channel_id = temp_file.value();
+#endif
+  }
+  if (channel_id.empty()) {
     launcher_.reset(new AnonymousProxyLauncher(false));
   } else {
-    launcher_.reset(new NamedProxyLauncher(options.channel_id, false, false));
+    LOG(INFO) << "Using named testing interface";
+    launcher_.reset(new NamedProxyLauncher(channel_id, launch_browser, false));
   }
   ProxyLauncher::LaunchState launch_props = {
       false,  // clear_profile
diff --git a/chrome/test/webdriver/webdriver_automation.h b/chrome/test/webdriver/webdriver_automation.h
index aa4c79a..d437a05e 100644
--- a/chrome/test/webdriver/webdriver_automation.h
+++ b/chrome/test/webdriver/webdriver_automation.h
@@ -42,9 +42,22 @@ class Automation {
     BrowserOptions();
     ~BrowserOptions();
 
+    // The command line to use for launching the browser. If no program is
+    // specified, the default browser executable will be used.
     CommandLine command;
+
+    // The user data directory to be copied and used. If empty, a temporary
+    // directory will be used.
     FilePath user_data_dir;
+
+    // The channel ID of an already running browser to connect to. If empty,
+    // the browser will be launched with an anonymous channel.
     std::string channel_id;
+
+    // True if the Chrome process should only be terminated if quit is called.
+    // If false, Chrome will also be terminated if this process is killed or
+    // shutdown.
+    bool detach_process;
   };
 
   Automation();
diff --git a/chrome/test/webdriver/webdriver_dispatch.cc b/chrome/test/webdriver/webdriver_dispatch.cc
index 60d50f7..a9dcd4a 100644
--- a/chrome/test/webdriver/webdriver_dispatch.cc
+++ b/chrome/test/webdriver/webdriver_dispatch.cc
@@ -55,6 +55,7 @@ void DispatchCommand(Command* const command,
   } else {
     NOTREACHED();
   }
+  command->Finish();
 }
 
 void Shutdown(struct mg_connection* connection,
diff --git a/chrome/test/webdriver/webdriver_error.cc b/chrome/test/webdriver/webdriver_error.cc
index 07098c0..f1831bd 100644
--- a/chrome/test/webdriver/webdriver_error.cc
+++ b/chrome/test/webdriver/webdriver_error.cc
@@ -63,7 +63,7 @@ void Error::AddDetails(const std::string& details) {
     details_ = details + ";\n " + details_;
 }
 
-std::string Error::GetMessage() const {
+std::string Error::GetErrorMessage() const {
   std::string msg;
   if (details_.length())
     msg = details_;
diff --git a/chrome/test/webdriver/webdriver_error.h b/chrome/test/webdriver/webdriver_error.h
index c0c094f..23caf08 100644
--- a/chrome/test/webdriver/webdriver_error.h
+++ b/chrome/test/webdriver/webdriver_error.h
@@ -49,7 +49,7 @@ class Error {
 
   void AddDetails(const std::string& details);
 
-  std::string GetMessage() const;
+  std::string GetErrorMessage() const;
 
   ErrorCode code() const;
   const std::string& details() const;
diff --git a/chrome/test/webdriver/webdriver_session.cc b/chrome/test/webdriver/webdriver_session.cc
index efd7b1e..f93ed46 100644
--- a/chrome/test/webdriver/webdriver_session.cc
+++ b/chrome/test/webdriver/webdriver_session.cc
@@ -93,15 +93,29 @@ Error* Session::Init(const Automation::BrowserOptions& options) {
 }
 
 Error* Session::BeforeExecuteCommand() {
+  Error* error = AfterExecuteCommand();
+  if (!error) {
+    scoped_ptr<Error> switch_error(SwitchToTopFrameIfCurrentFrameInvalid());
+    if (switch_error.get()) {
+      std::string text;
+      scoped_ptr<Error> alert_error(GetAlertMessage(&text));
+      if (alert_error.get()) {
+        // Only return a frame checking error if a modal dialog is not present.
+        // TODO(kkania): This is ugly. Fix.
+        return switch_error.release();
+      }
+    }
+  }
+  return error;
+}
+
+Error* Session::AfterExecuteCommand() {
   Error* error = NULL;
   if (!options_.load_async) {
     LOG(INFO) << "Waiting for the page to stop loading";
     error = WaitForAllTabsToStopLoading();
     LOG(INFO) << "Done waiting for the page to stop loading";
   }
-  if (!error) {
-    error = SwitchToTopFrameIfCurrentFrameInvalid();
-  }
   return error;
 }
 
diff --git a/chrome/test/webdriver/webdriver_session.h b/chrome/test/webdriver/webdriver_session.h
index 10e158c..7c2f6b6 100644
--- a/chrome/test/webdriver/webdriver_session.h
+++ b/chrome/test/webdriver/webdriver_session.h
@@ -77,10 +77,12 @@ class Session {
   // itself and return an error code.
   Error* Init(const Automation::BrowserOptions& options);
 
-  // Should be called before executing a command. Performs necessary waits
-  // and frame switching.
+  // Should be called before executing a command.
   Error* BeforeExecuteCommand();
 
+  // Should be called after executing a command.
+  Error* AfterExecuteCommand();
+
   // Terminates this session and deletes itself.
   void Terminate();