Revert 97557 - [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


Reverting because of chromedriver_tests failure on windows.
http://build.chromium.org/p/chromium.pyauto/builders/Win%20XP/builds/9846/steps/chromedriver_tests/logs/stdio

ERROR: chromedriver_tests.DetachProcessTest.testDetachProcess: "None"
----------------------------------------------------------------------
Traceback (most recent call last):
    File
    "E:\b\build\slave\Win_XP\build\src\chrome\test\webdriver\test\chromedriver_tests.py",
    line 357, in testDetachProcess
        os.kill(pid, 0)  # Would throw if process no longer exists
    AttributeError: 'module' object has no attribute 'kill'

Feel free to recommit after fix (no review necessary)


BUG=87676,93438
TEST=none


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

TBR=kkania@chromium.org
Review URL: http://codereview.chromium.org/7696022

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

18 files changed, 12 insertions, 146 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index eeaf263..aac9147 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -7,7 +7,6 @@
 #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"
@@ -106,9 +105,7 @@ using base::Time;
 AutomationProvider::AutomationProvider(Profile* profile)
     : profile_(profile),
       reply_message_(NULL),
-      reinitialize_on_channel_error_(
-          CommandLine::ForCurrentProcess()->HasSwitch(
-              switches::kAutomationReinitializeOnChannelError)),
+      reinitialize_on_channel_error_(false),
       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 a84c282..800efdc 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -91,11 +91,6 @@ 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 6bca51c..74ac8da 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -43,7 +43,6 @@ 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 3b1ea07..95ad878 100644
--- a/chrome/test/webdriver/commands/command.cc
+++ b/chrome/test/webdriver/commands/command.cc
@@ -29,8 +29,6 @@ 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 8f735f8..672b2c0 100644
--- a/chrome/test/webdriver/commands/command.h
+++ b/chrome/test/webdriver/commands/command.h
@@ -15,7 +15,6 @@
 
 namespace webdriver {
 
-class Error;
 class Response;
 
 // Base class for a command mapped to a URL in the WebDriver REST API. Each
@@ -38,11 +37,6 @@ 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 b76760b8..d673ead 100644
--- a/chrome/test/webdriver/commands/create_session.cc
+++ b/chrome/test/webdriver/commands/create_session.cc
@@ -197,10 +197,6 @@ 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 1c09d39..6b96bae 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->GetErrorMessage());
+  error_dict->SetString(kMessageKey, error->GetMessage());
 
   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 eed1d11c..3220316 100644
--- a/chrome/test/webdriver/commands/webdriver_command.cc
+++ b/chrome/test/webdriver/commands/webdriver_command.cc
@@ -50,12 +50,4 @@ 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 765b59b..248aa58 100644
--- a/chrome/test/webdriver/commands/webdriver_command.h
+++ b/chrome/test/webdriver/commands/webdriver_command.h
@@ -16,7 +16,6 @@ class DictionaryValue;
 
 namespace webdriver {
 
-class Error;
 class Response;
 class Session;
 
@@ -35,8 +34,6 @@ 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 76d5bd0..93fc102 100644
--- a/chrome/test/webdriver/test/alerts.html
+++ b/chrome/test/webdriver/test/alerts.html
@@ -3,6 +3,5 @@
     <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 659a872..f5a5d28 100644
--- a/chrome/test/webdriver/test/chromedriver_tests.py
+++ b/chrome/test/webdriver/test/chromedriver_tests.py
@@ -14,7 +14,6 @@ from distutils import archive_util
 import hashlib
 import os
 import platform
-import signal
 import subprocess
 import sys
 import tempfile
@@ -72,14 +71,6 @@ 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."""
 
@@ -333,33 +324,6 @@ 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:
-      os.kill(pid, 0)  # Would throw if process no longer exists
-      Kill(pid)
-    except OSError:
-      self.fail('Chrome quit after detached chromedriver server was killed')
-
-
 class CookieTest(ChromeDriverTest):
   """Cookie test for the json webdriver protocol"""
 
@@ -830,13 +794,6 @@ 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 9d21837..f410842 100644
--- a/chrome/test/webdriver/webdriver_automation.cc
+++ b/chrome/test/webdriver/webdriver_automation.cc
@@ -184,8 +184,7 @@ bool GetDefaultChromeExe(FilePath* browser_exe) {
 namespace webdriver {
 
 Automation::BrowserOptions::BrowserOptions()
-    : command(CommandLine::NO_PROGRAM),
-      detach_process(false) {}
+    : command(CommandLine::NO_PROGRAM) {}
 
 Automation::BrowserOptions::~BrowserOptions() {}
 
@@ -202,8 +201,6 @@ 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);
 
@@ -231,35 +228,10 @@ void Automation::Init(const BrowserOptions& options, Error** error) {
   LOG(INFO) << chrome_details;
 
   // Create the ProxyLauncher and launch Chrome.
-  // 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()) {
+  if (options.channel_id.empty()) {
     launcher_.reset(new AnonymousProxyLauncher(false));
   } else {
-    LOG(INFO) << "Using named testing interface";
-    launcher_.reset(new NamedProxyLauncher(channel_id, launch_browser, false));
+    launcher_.reset(new NamedProxyLauncher(options.channel_id, false, 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 d437a05e..aa4c79a 100644
--- a/chrome/test/webdriver/webdriver_automation.h
+++ b/chrome/test/webdriver/webdriver_automation.h
@@ -42,22 +42,9 @@ 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 a9dcd4a..60d50f7 100644
--- a/chrome/test/webdriver/webdriver_dispatch.cc
+++ b/chrome/test/webdriver/webdriver_dispatch.cc
@@ -55,7 +55,6 @@ 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 f1831bd..07098c0 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::GetErrorMessage() const {
+std::string Error::GetMessage() 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 23caf08..c0c094f 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 GetErrorMessage() const;
+  std::string GetMessage() 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 f93ed46..efd7b1e 100644
--- a/chrome/test/webdriver/webdriver_session.cc
+++ b/chrome/test/webdriver/webdriver_session.cc
@@ -93,29 +93,15 @@ 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 7c2f6b6..10e158c 100644
--- a/chrome/test/webdriver/webdriver_session.h
+++ b/chrome/test/webdriver/webdriver_session.h
@@ -77,12 +77,10 @@ class Session {
   // itself and return an error code.
   Error* Init(const Automation::BrowserOptions& options);
 
-  // Should be called before executing a command.
+  // Should be called before executing a command. Performs necessary waits
+  // and frame switching.
   Error* BeforeExecuteCommand();
 
-  // Should be called after executing a command.
-  Error* AfterExecuteCommand();
-
   // Terminates this session and deletes itself.
   void Terminate();