[chromedriver] Use single path for handling android/desktop args.

-also allow extensions to be loaded via --load-extension
-change useExistingBrowser to debuggerAddress
BUG=chromedriver:488

Review URL: https://chromiumcodereview.appspot.com/23643005

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

chrome/test/chromedriver/capabilities.h

@@ -5,6 +5,7 @@
 #ifndef CHROME_TEST_CHROMEDRIVER_CAPABILITIES_H_
 #define CHROME_TEST_CHROMEDRIVER_CAPABILITIES_H_
 
+#include <map>
 #include <set>
 #include <string>
 #include <vector>
@@ -12,15 +13,50 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/strings/string16.h"
 #include "chrome/test/chromedriver/net/net_util.h"
 
 namespace base {
 class DictionaryValue;
 }
 
+class CommandLine;
 class Log;
 class Status;
 
+class Switches {
+ public:
+  typedef base::FilePath::StringType NativeString;
+  Switches();
+  ~Switches();
+
+  void SetSwitch(const std::string& name);
+  void SetSwitch(const std::string& name, const std::string& value);
+  void SetSwitch(const std::string& name, const string16& value);
+  void SetSwitch(const std::string& name, const base::FilePath& value);
+
+  // In case of same key, |switches| will override.
+  void SetFromSwitches(const Switches& switches);
+
+  // Sets a switch from the capabilities, of the form [--]name[=value].
+  void SetUnparsedSwitch(const std::string& unparsed_switch);
+
+  void RemoveSwitch(const std::string& name);
+
+  bool HasSwitch(const std::string& name) const;
+  std::string GetSwitchValue(const std::string& name) const;
+  NativeString GetSwitchValueNative(const std::string& name) const;
+
+  size_t GetSize() const;
+
+  void AppendToCommandLine(CommandLine* command) const;
+  std::string ToString() const;
+
+ private:
+  typedef std::map<std::string, NativeString> SwitchMap;
+  SwitchMap switch_map_;
+};
+
 struct Capabilities {
   Capabilities();
   ~Capabilities();
@@ -33,34 +69,43 @@ struct Capabilities {
 
   Status Parse(const base::DictionaryValue& desired_caps, Log* log);
 
-  // True if should always use DevTools for taking screenshots.
-  // This is experimental and may be removed at a later point.
-  bool force_devtools_screenshot;
+  std::string android_activity;
+
+  std::string android_device_serial;
+
+  std::string android_package;
+
+  std::string android_process;
+
+  base::FilePath binary;
+
+  // If provided, the remote debugging address to connect to.
+  NetAddress debugger_address;
 
   // Whether the lifetime of the started Chrome browser process should be
   // bound to ChromeDriver's process. If true, Chrome will not quit if
   // ChromeDriver dies.
   bool detach;
 
-  std::string android_package;
-  std::string android_activity;
-  std::string android_process;
-  std::string android_device_serial;
-  std::string android_args;
+  // Set of switches which should be removed from default list when launching
+  // Chrome.
+  std::set<std::string> exclude_switches;
 
-  std::string log_path;
-  CommandLine command;
-  scoped_ptr<base::DictionaryValue> prefs;
-  scoped_ptr<base::DictionaryValue> local_state;
   std::vector<std::string> extensions;
+
+  // True if should always use DevTools for taking screenshots.
+  // This is experimental and may be removed at a later point.
+  bool force_devtools_screenshot;
+
+  scoped_ptr<base::DictionaryValue> local_state;
+
+  std::string log_path;
+
   scoped_ptr<base::DictionaryValue> logging_prefs;
 
-  // Set of switches which should be removed from default list when launching
-  // Chrome.
-  std::set<std::string> exclude_switches;
+  scoped_ptr<base::DictionaryValue> prefs;
 
-  // If provided, the remote debugging address to connect to.
-  NetAddress use_existing_browser;
+  Switches switches;
 };
 
 #endif  // CHROME_TEST_CHROMEDRIVER_CAPABILITIES_H_

chrome/test/chromedriver/capabilities_unittest.cc

@@ -9,6 +9,94 @@
 #include "chrome/test/chromedriver/chrome/status.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+TEST(Switches, Empty) {
+  Switches switches;
+  CommandLine cmd(CommandLine::NO_PROGRAM);
+  switches.AppendToCommandLine(&cmd);
+  ASSERT_EQ(0u, cmd.GetSwitches().size());
+  ASSERT_EQ("", switches.ToString());
+}
+
+TEST(Switches, NoValue) {
+  Switches switches;
+  switches.SetSwitch("hello");
+
+  ASSERT_TRUE(switches.HasSwitch("hello"));
+  ASSERT_EQ("", switches.GetSwitchValue("hello"));
+
+  CommandLine cmd(CommandLine::NO_PROGRAM);
+  switches.AppendToCommandLine(&cmd);
+  ASSERT_TRUE(cmd.HasSwitch("hello"));
+  ASSERT_EQ(FILE_PATH_LITERAL(""), cmd.GetSwitchValueNative("hello"));
+  ASSERT_EQ("--hello", switches.ToString());
+}
+
+TEST(Switches, Value) {
+  Switches switches;
+  switches.SetSwitch("hello", "there");
+
+  ASSERT_TRUE(switches.HasSwitch("hello"));
+  ASSERT_EQ("there", switches.GetSwitchValue("hello"));
+
+  CommandLine cmd(CommandLine::NO_PROGRAM);
+  switches.AppendToCommandLine(&cmd);
+  ASSERT_TRUE(cmd.HasSwitch("hello"));
+  ASSERT_EQ(FILE_PATH_LITERAL("there"), cmd.GetSwitchValueNative("hello"));
+  ASSERT_EQ("--hello=there", switches.ToString());
+}
+
+TEST(Switches, FromOther) {
+  Switches switches;
+  switches.SetSwitch("a", "1");
+  switches.SetSwitch("b", "1");
+
+  Switches switches2;
+  switches2.SetSwitch("b", "2");
+  switches2.SetSwitch("c", "2");
+
+  switches.SetFromSwitches(switches2);
+  ASSERT_EQ("--a=1 --b=2 --c=2", switches.ToString());
+}
+
+TEST(Switches, Remove) {
+  Switches switches;
+  switches.SetSwitch("a", "1");
+  switches.RemoveSwitch("a");
+  ASSERT_FALSE(switches.HasSwitch("a"));
+}
+
+TEST(Switches, Quoting) {
+  Switches switches;
+  switches.SetSwitch("hello", "a  b");
+  switches.SetSwitch("hello2", "  '\"  ");
+
+  ASSERT_EQ("--hello=\"a  b\" --hello2=\"  '\\\"  \"", switches.ToString());
+}
+
+TEST(Switches, Multiple) {
+  Switches switches;
+  switches.SetSwitch("switch");
+  switches.SetSwitch("hello", "there");
+
+  CommandLine cmd(CommandLine::NO_PROGRAM);
+  switches.AppendToCommandLine(&cmd);
+  ASSERT_TRUE(cmd.HasSwitch("switch"));
+  ASSERT_TRUE(cmd.HasSwitch("hello"));
+  ASSERT_EQ(FILE_PATH_LITERAL("there"), cmd.GetSwitchValueNative("hello"));
+  ASSERT_EQ("--hello=there --switch", switches.ToString());
+}
+
+TEST(Switches, Unparsed) {
+  Switches switches;
+  switches.SetUnparsedSwitch("a");
+  switches.SetUnparsedSwitch("--b");
+  switches.SetUnparsedSwitch("--c=1");
+  switches.SetUnparsedSwitch("d=1");
+  switches.SetUnparsedSwitch("-e=--1=1");
+
+  ASSERT_EQ("---e=--1=1 --a --b --c=1 --d=1", switches.ToString());
+}
+
 TEST(ParseCapabilities, WithAndroidPackage) {
   Capabilities capabilities;
   base::DictionaryValue caps;
@@ -48,68 +136,23 @@ TEST(ParseCapabilities, LogPath) {
   ASSERT_STREQ("path/to/logfile", capabilities.log_path.c_str());
 }
 
-TEST(ParseCapabilities, NoArgs) {
-  Capabilities capabilities;
-  base::ListValue args;
-  ASSERT_TRUE(args.empty());
-  base::DictionaryValue caps;
-  caps.Set("chromeOptions.args", args.DeepCopy());
-  Logger log(Log::kError);
-  Status status = capabilities.Parse(caps, &log);
-  ASSERT_TRUE(status.IsOk());
-  ASSERT_TRUE(capabilities.command.GetSwitches().empty());
-}
-
-TEST(ParseCapabilities, SingleArgWithoutValue) {
-  Capabilities capabilities;
-  base::ListValue args;
-  args.AppendString("enable-nacl");
-  ASSERT_EQ(1u, args.GetSize());
-  base::DictionaryValue caps;
-  caps.Set("chromeOptions.args", args.DeepCopy());
-  Logger log(Log::kError);
-  Status status = capabilities.Parse(caps, &log);
-  ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(1u, capabilities.command.GetSwitches().size());
-  ASSERT_TRUE(capabilities.command.HasSwitch("enable-nacl"));
-}
-
-TEST(ParseCapabilities, SingleArgWithValue) {
-  Capabilities capabilities;
-  base::ListValue args;
-  args.AppendString("load-extension=/test/extension");
-  ASSERT_EQ(1u, args.GetSize());
-  base::DictionaryValue caps;
-  caps.Set("chromeOptions.args", args.DeepCopy());
-  Logger log(Log::kError);
-  Status status = capabilities.Parse(caps, &log);
-  ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(1u, capabilities.command.GetSwitches().size());
-  ASSERT_TRUE(capabilities.command.HasSwitch("load-extension"));
-  ASSERT_STREQ(
-      "/test/extension",
-      capabilities.command.GetSwitchValueASCII("load-extension").c_str());
-}
-
-TEST(ParseCapabilities, MultipleArgs) {
+TEST(ParseCapabilities, Args) {
   Capabilities capabilities;
   base::ListValue args;
   args.AppendString("arg1");
   args.AppendString("arg2=val");
-  args.AppendString("arg3='a space'");
-  ASSERT_EQ(3u, args.GetSize());
   base::DictionaryValue caps;
   caps.Set("chromeOptions.args", args.DeepCopy());
+
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(3u, capabilities.command.GetSwitches().size());
-  ASSERT_TRUE(capabilities.command.HasSwitch("arg1"));
-  ASSERT_TRUE(capabilities.command.HasSwitch("arg2"));
-  ASSERT_STREQ("val", capabilities.command.GetSwitchValueASCII("arg2").c_str());
-  ASSERT_TRUE(capabilities.command.HasSwitch("arg3"));
-  ASSERT_STREQ("'a space'",
-               capabilities.command.GetSwitchValueASCII("arg3").c_str());
+
+  ASSERT_EQ(2u, capabilities.switches.GetSize());
+  ASSERT_TRUE(capabilities.switches.HasSwitch("arg1"));
+  ASSERT_TRUE(capabilities.switches.HasSwitch("arg2"));
+  ASSERT_EQ("", capabilities.switches.GetSwitchValue("arg1"));
+  ASSERT_EQ("val", capabilities.switches.GetSwitchValue("arg2"));
 }
 
 TEST(ParseCapabilities, Prefs) {
@@ -184,8 +227,8 @@ TEST(ParseCapabilities, DirectProxy) {
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(1u, capabilities.command.GetSwitches().size());
-  ASSERT_TRUE(capabilities.command.HasSwitch("no-proxy-server"));
+  ASSERT_EQ(1u, capabilities.switches.GetSize());
+  ASSERT_TRUE(capabilities.switches.HasSwitch("no-proxy-server"));
 }
 
 TEST(ParseCapabilities, SystemProxy) {
@@ -197,7 +240,7 @@ TEST(ParseCapabilities, SystemProxy) {
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_TRUE(capabilities.command.GetSwitches().empty());
+  ASSERT_EQ(0u, capabilities.switches.GetSize());
 }
 
 TEST(ParseCapabilities, PacProxy) {
@@ -210,10 +253,8 @@ TEST(ParseCapabilities, PacProxy) {
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(1u, capabilities.command.GetSwitches().size());
-  ASSERT_STREQ(
-      "test.wpad",
-      capabilities.command.GetSwitchValueASCII("proxy-pac-url").c_str());
+  ASSERT_EQ(1u, capabilities.switches.GetSize());
+  ASSERT_EQ("test.wpad", capabilities.switches.GetSwitchValue("proxy-pac-url"));
 }
 
 TEST(ParseCapabilities, MissingProxyAutoconfigUrl) {
@@ -237,8 +278,8 @@ TEST(ParseCapabilities, AutodetectProxy) {
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(1u, capabilities.command.GetSwitches().size());
-  ASSERT_TRUE(capabilities.command.HasSwitch("proxy-auto-detect"));
+  ASSERT_EQ(1u, capabilities.switches.GetSize());
+  ASSERT_TRUE(capabilities.switches.HasSwitch("proxy-auto-detect"));
 }
 
 TEST(ParseCapabilities, ManualProxy) {
@@ -254,13 +295,13 @@ TEST(ParseCapabilities, ManualProxy) {
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(2u, capabilities.command.GetSwitches().size());
-  ASSERT_STREQ(
+  ASSERT_EQ(2u, capabilities.switches.GetSize());
+  ASSERT_EQ(
       "ftp=localhost:9001;http=localhost:8001;https=localhost:10001",
-      capabilities.command.GetSwitchValueASCII("proxy-server").c_str());
-  ASSERT_STREQ(
+      capabilities.switches.GetSwitchValue("proxy-server"));
+  ASSERT_EQ(
       "google.com, youtube.com",
-      capabilities.command.GetSwitchValueASCII("proxy-bypass-list").c_str());
+      capabilities.switches.GetSwitchValue("proxy-bypass-list"));
 }
 
 TEST(ParseCapabilities, MissingSettingForManualProxy) {
@@ -286,11 +327,11 @@ TEST(ParseCapabilities, IgnoreNullValueForManualProxy) {
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_EQ(1u, capabilities.command.GetSwitches().size());
-  ASSERT_TRUE(capabilities.command.HasSwitch("proxy-server"));
-  ASSERT_STREQ(
+  ASSERT_EQ(1u, capabilities.switches.GetSize());
+  ASSERT_TRUE(capabilities.switches.HasSwitch("proxy-server"));
+  ASSERT_EQ(
       "ftp=localhost:9001",
-      capabilities.command.GetSwitchValueASCII("proxy-server").c_str());
+      capabilities.switches.GetSwitchValue("proxy-server"));
 }
 
 TEST(ParseCapabilities, LoggingPrefsOk) {
@@ -337,11 +378,11 @@ TEST(ParseCapabilities, ExcludeSwitches) {
 TEST(ParseCapabilities, UseExistingBrowser) {
   Capabilities capabilities;
   base::DictionaryValue caps;
-  caps.SetString("chromeOptions.useExistingBrowser", "abc:123");
+  caps.SetString("chromeOptions.debuggerAddress", "abc:123");
   Logger log(Log::kError);
   Status status = capabilities.Parse(caps, &log);
   ASSERT_TRUE(status.IsOk());
   ASSERT_TRUE(capabilities.IsExistingBrowser());
-  ASSERT_EQ("abc", capabilities.use_existing_browser.host());
-  ASSERT_EQ(123, capabilities.use_existing_browser.port());
+  ASSERT_EQ("abc", capabilities.debugger_address.host());
+  ASSERT_EQ(123, capabilities.debugger_address.port());
 }

chrome/test/chromedriver/chrome/adb_impl.cc

@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/json/string_escape.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
@@ -118,13 +119,13 @@ Status AdbImpl::SetCommandLineFile(const std::string& device_serial,
                                    const std::string& exec_name,
                                    const std::string& args) {
   std::string response;
-  if (args.find("'") != std::string::npos)
-    return Status(kUnknownError,
-        "Chrome command line arguments must not contain single quotes");
+  std::string quoted_command =
+      base::GetDoubleQuotedJson(exec_name + " " + args);
   Status status = ExecuteHostShellCommand(
       device_serial,
-      "echo '" + exec_name +
-      " " + args + "'> " + command_line_file + "; echo $?",
+      base::StringPrintf("echo %s > %s; echo $?",
+                         quoted_command.c_str(),
+                         command_line_file.c_str()),
       &response);
   if (!status.IsOk())
     return status;

chrome/test/chromedriver/chrome_launcher.cc

@@ -45,7 +45,7 @@
 namespace {
 
 const char* kCommonSwitches[] = {
-  "ignore-certificate-errors", "metrics-recording-only"};
+    "ignore-certificate-errors", "metrics-recording-only"};
 
 Status UnpackAutomationExtension(const base::FilePath& temp_dir,
                                  base::FilePath* automation_extension) {
@@ -68,66 +68,58 @@ Status UnpackAutomationExtension(const base::FilePath& temp_dir,
   return Status(kOk);
 }
 
-void AddSwitches(CommandLine* command,
-                 const char* switches[],
-                 size_t switch_count,
-                 const std::set<std::string>& exclude_switches) {
-  for (size_t i = 0; i < switch_count; ++i) {
-    if (exclude_switches.find(switches[i]) == exclude_switches.end())
-      command->AppendSwitch(switches[i]);
-  }
-}
-
 Status PrepareCommandLine(int port,
                           const Capabilities& capabilities,
                           CommandLine* prepared_command,
                           base::ScopedTempDir* user_data_dir,
                           base::ScopedTempDir* extension_dir,
                           std::vector<std::string>* extension_bg_pages) {
-  CommandLine command = capabilities.command;
-  base::FilePath program = command.GetProgram();
+  base::FilePath program = capabilities.binary;
   if (program.empty()) {
     if (!FindChrome(&program))
       return Status(kUnknownError, "cannot find Chrome binary");
-    command.SetProgram(program);
   } else if (!base::PathExists(program)) {
     return Status(kUnknownError,
                   base::StringPrintf("no chrome binary at %" PRFilePath,
                                      program.value().c_str()));
   }
+  CommandLine command(program);
+  Switches switches;
+
+  // TODO(chrisgao): Add "disable-sync" when chrome 30- is not supported.
+  // For chrome 30-, it leads to crash when opening chrome://settings.
+  for (size_t i = 0; i < arraysize(kCommonSwitches); ++i)
+    switches.SetSwitch(kCommonSwitches[i]);
+  switches.SetSwitch("disable-hang-monitor");
+  switches.SetSwitch("disable-prompt-on-repost");
+  switches.SetSwitch("full-memory-crash-report");
+  switches.SetSwitch("no-first-run");
+  switches.SetSwitch("disable-background-networking");
+  switches.SetSwitch("disable-web-resources");
+  switches.SetSwitch("safebrowsing-disable-auto-update");
+  switches.SetSwitch("safebrowsing-disable-download-protection");
+  switches.SetSwitch("disable-client-side-phishing-detection");
+  switches.SetSwitch("disable-component-update");
+  switches.SetSwitch("disable-default-apps");
+  switches.SetSwitch("enable-logging");
+  switches.SetSwitch("logging-level", "1");
+  switches.SetSwitch("password-store", "basic");
+  switches.SetSwitch("use-mock-keychain");
+  switches.SetSwitch("remote-debugging-port", base::IntToString(port));
+
+  for (std::set<std::string>::const_iterator iter =
+           capabilities.exclude_switches.begin();
+       iter != capabilities.exclude_switches.end();
+       ++iter) {
+    switches.RemoveSwitch(*iter);
+  }
+  switches.SetFromSwitches(capabilities.switches);
 
-  const char* excludable_switches[] = {
-      "disable-hang-monitor",
-      "disable-prompt-on-repost",
-      "full-memory-crash-report",
-      "no-first-run",
-      "disable-background-networking",
-      // TODO(chrisgao): Add "disable-sync" when chrome 30- is not supported.
-      // For chrome 30-, it leads to crash when opening chrome://settings.
-      "disable-web-resources",
-      "safebrowsing-disable-auto-update",
-      "safebrowsing-disable-download-protection",
-      "disable-client-side-phishing-detection",
-      "disable-component-update",
-      "disable-default-apps",
-  };
-
-  AddSwitches(&command, excludable_switches, arraysize(excludable_switches),
-              capabilities.exclude_switches);
-  AddSwitches(&command, kCommonSwitches, arraysize(kCommonSwitches),
-              capabilities.exclude_switches);
-
-  command.AppendSwitch("enable-logging");
-  command.AppendSwitchASCII("logging-level", "1");
-  command.AppendSwitchASCII("password-store", "basic");
-  command.AppendSwitch("use-mock-keychain");
-  command.AppendSwitchASCII("remote-debugging-port", base::IntToString(port));
-
-  if (!command.HasSwitch("user-data-dir")) {
+  if (!switches.HasSwitch("user-data-dir")) {
     command.AppendArg("about:blank");
     if (!user_data_dir->CreateUniqueTempDir())
       return Status(kUnknownError, "cannot create temp dir for user data dir");
-    command.AppendSwitchPath("user-data-dir", user_data_dir->path());
+    switches.SetSwitch("user-data-dir", user_data_dir->path().value());
     Status status = internal::PrepareUserDataDir(
         user_data_dir->path(), capabilities.prefs.get(),
         capabilities.local_state.get());
@@ -142,11 +134,11 @@ Status PrepareCommandLine(int port,
   Status status = internal::ProcessExtensions(capabilities.extensions,
                                               extension_dir->path(),
                                               true,
-                                              &command,
+                                              &switches,
                                               extension_bg_pages);
   if (status.IsError())
     return status;
-
+  switches.AppendToCommandLine(&command);
   *prepared_command = command;
   return Status(kOk);
 }
@@ -190,11 +182,11 @@ Status LaunchExistingChromeSession(
   Status status(kOk);
   scoped_ptr<DevToolsHttpClient> devtools_client;
   status = WaitForDevToolsAndCheckVersion(
-      capabilities.use_existing_browser, context_getter, socket_factory, log,
+      capabilities.debugger_address, context_getter, socket_factory, log,
       &devtools_client);
   if (status.IsError()) {
     return Status(kUnknownError, "cannot connect to chrome at " +
-                      capabilities.use_existing_browser.ToString(),
+                      capabilities.debugger_address.ToString(),
                   status);
   }
   chrome->reset(new ChromeExistingImpl(devtools_client.Pass(),
@@ -323,15 +315,15 @@ Status LaunchAndroidChrome(
   if (!status.IsOk())
     return status;
 
-  std::string args(capabilities.android_args);
-  for (size_t i = 0; i < arraysize(kCommonSwitches); i++)
-    args += "--" + std::string(kCommonSwitches[i]) + " ";
-  args += "--disable-fre --enable-remote-debugging";
-
+  Switches switches(capabilities.switches);
+  for (size_t i = 0; i < arraysize(kCommonSwitches); ++i)
+    switches.SetSwitch(kCommonSwitches[i]);
+  switches.SetSwitch("disable-fre");
+  switches.SetSwitch("enable-remote-debugging");
   status = device->StartApp(capabilities.android_package,
                             capabilities.android_activity,
                             capabilities.android_process,
-                            args, port);
+                            switches.ToString(), port);
   if (!status.IsOk()) {
     device->StopApp();
     return status;
@@ -495,10 +487,20 @@ Status ProcessExtension(const std::string& extension,
   return Status(kOk);
 }
 
+void UpdateExtensionSwitch(Switches* switches,
+                           const char name[],
+                           const base::FilePath::StringType& extension) {
+  base::FilePath::StringType value = switches->GetSwitchValueNative(name);
+  if (value.length())
+    value += FILE_PATH_LITERAL(",");
+  value += extension;
+  switches->SetSwitch(name, value);
+}
+
 Status ProcessExtensions(const std::vector<std::string>& extensions,
                          const base::FilePath& temp_dir,
                          bool include_automation_extension,
-                         CommandLine* command,
+                         Switches* switches,
                          std::vector<std::string>* bg_pages) {
   std::vector<std::string> bg_pages_tmp;
   std::vector<base::FilePath::StringType> extension_paths;
@@ -522,9 +524,9 @@ Status ProcessExtensions(const std::vector<std::string>& extensions,
     Status status = UnpackAutomationExtension(temp_dir, &automation_extension);
     if (status.IsError())
       return status;
-    if (command->HasSwitch("disable-extensions")) {
-      command->AppendSwitchNative("load-component-extension",
-                                  automation_extension.value());
+    if (switches->HasSwitch("disable-extensions")) {
+      UpdateExtensionSwitch(switches, "load-component-extension",
+                            automation_extension.value());
     } else {
       extension_paths.push_back(automation_extension.value());
     }
@@ -533,7 +535,7 @@ Status ProcessExtensions(const std::vector<std::string>& extensions,
   if (extension_paths.size()) {
     base::FilePath::StringType extension_paths_value = JoinString(
         extension_paths, FILE_PATH_LITERAL(','));
-    command->AppendSwitchNative("load-extension", extension_paths_value);
+    UpdateExtensionSwitch(switches, "load-extension", extension_paths_value);
   }
   bg_pages->swap(bg_pages_tmp);
   return Status(kOk);

chrome/test/chromedriver/chrome_launcher.h

@@ -41,7 +41,7 @@ namespace internal {
 Status ProcessExtensions(const std::vector<std::string>& extensions,
                          const base::FilePath& temp_dir,
                          bool include_automation_extension,
-                         CommandLine* command,
+                         Switches* switches,
                          std::vector<std::string>* bg_pages);
 Status PrepareUserDataDir(
     const base::FilePath& user_data_dir,

chrome/test/chromedriver/chrome_launcher_unittest.cc

@@ -18,14 +18,14 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 TEST(ProcessExtensions, NoExtension) {
-  CommandLine command(CommandLine::NO_PROGRAM);
+  Switches switches;
   std::vector<std::string> extensions;
   base::FilePath extension_dir;
   std::vector<std::string> bg_pages;
   Status status = internal::ProcessExtensions(extensions, extension_dir,
-                                              false, &command, &bg_pages);
+                                              false, &switches, &bg_pages);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_FALSE(command.HasSwitch("load-extension"));
+  ASSERT_FALSE(switches.HasSwitch("load-extension"));
   ASSERT_EQ(0u, bg_pages.size());
 }
 
@@ -53,13 +53,13 @@ TEST(ProcessExtensions, SingleExtensionWithBgPage) {
   base::ScopedTempDir extension_dir;
   ASSERT_TRUE(extension_dir.CreateUniqueTempDir());
 
-  CommandLine command(CommandLine::NO_PROGRAM);
+  Switches switches;
   std::vector<std::string> bg_pages;
   Status status = internal::ProcessExtensions(extensions, extension_dir.path(),
-                                              false, &command, &bg_pages);
+                                              false, &switches, &bg_pages);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_TRUE(command.HasSwitch("load-extension"));
-  base::FilePath temp_ext_path = command.GetSwitchValuePath("load-extension");
+  ASSERT_TRUE(switches.HasSwitch("load-extension"));
+  base::FilePath temp_ext_path(switches.GetSwitchValueNative("load-extension"));
   ASSERT_TRUE(base::PathExists(temp_ext_path));
   std::string manifest_txt;
   ASSERT_TRUE(file_util::ReadFileToString(
@@ -91,13 +91,13 @@ TEST(ProcessExtensions, MultipleExtensionsNoBgPages) {
   base::ScopedTempDir extension_dir;
   ASSERT_TRUE(extension_dir.CreateUniqueTempDir());
 
-  CommandLine command(CommandLine::NO_PROGRAM);
+  Switches switches;
   std::vector<std::string> bg_pages;
   Status status = internal::ProcessExtensions(extensions, extension_dir.path(),
-                                              false, &command, &bg_pages);
+                                              false, &switches, &bg_pages);
   ASSERT_TRUE(status.IsOk());
-  ASSERT_TRUE(command.HasSwitch("load-extension"));
-  CommandLine::StringType ext_paths = command.GetSwitchValueNative(
+  ASSERT_TRUE(switches.HasSwitch("load-extension"));
+  CommandLine::StringType ext_paths = switches.GetSwitchValueNative(
       "load-extension");
   std::vector<CommandLine::StringType> ext_path_list;
   base::SplitString(ext_paths, FILE_PATH_LITERAL(','), &ext_path_list);
@@ -107,6 +107,24 @@ TEST(ProcessExtensions, MultipleExtensionsNoBgPages) {
   ASSERT_EQ(0u, bg_pages.size());
 }
 
+TEST(ProcessExtensions, CommandLineExtensions) {
+  std::vector<std::string> extensions;
+  ASSERT_TRUE(AddExtensionForInstall("ext_test_1.crx", &extensions));
+  base::ScopedTempDir extension_dir;
+  ASSERT_TRUE(extension_dir.CreateUniqueTempDir());
+
+  Switches switches;
+  switches.SetSwitch("load-extension", "/a");
+  std::vector<std::string> bg_pages;
+  Status status = internal::ProcessExtensions(extensions, extension_dir.path(),
+                                              false, &switches, &bg_pages);
+  ASSERT_EQ(kOk, status.code());
+  base::FilePath::StringType load = switches.GetSwitchValueNative(
+      "load-extension");
+  ASSERT_EQ(FILE_PATH_LITERAL("/a,"), load.substr(0, 3));
+  ASSERT_TRUE(base::PathExists(base::FilePath(load.substr(3))));
+}
+
 namespace {
 
 void AssertEQ(const base::DictionaryValue& dict, const std::string& key,
@@ -122,7 +140,6 @@ TEST(PrepareUserDataDir, CustomPrefs) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
 
-  CommandLine command(CommandLine::NO_PROGRAM);
   base::DictionaryValue prefs;
   prefs.SetString("myPrefsKey", "ok");
   prefs.SetStringWithoutPathExpansion("pref.sub", "1");

chrome/test/chromedriver/client/chromedriver.py

@@ -62,7 +62,7 @@ class ChromeDriver(object):
 
   def __init__(self, server_url, chrome_binary=None, android_package=None,
                chrome_switches=None, chrome_extensions=None,
-               chrome_log_path=None, chrome_existing_browser=None):
+               chrome_log_path=None, debugger_address=None):
     self._executor = command_executor.CommandExecutor(server_url)
 
     options = {}
@@ -83,9 +83,9 @@ class ChromeDriver(object):
       assert type(chrome_log_path) is str
       options['logPath'] = chrome_log_path
 
-    if chrome_existing_browser:
-      assert type(chrome_existing_browser) is str
-      options['useExistingBrowser'] = chrome_existing_browser
+    if debugger_address:
+      assert type(debugger_address) is str
+      options['debuggerAddress'] = debugger_address
 
     params = {
       'desiredCapabilities': {

chrome/test/chromedriver/test/run_java_tests.py

@@ -105,8 +105,11 @@ def _Run(java_tests_src_dir, test_filter,
 
   jvm_args = []
   if debug:
-    jvm_args += ['-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,'
-                 'address=33081']
+    transport = 'dt_socket'
+    if util.IsWindows():
+      transport = 'dt_shmem'
+    jvm_args += ['-agentlib:jdwp=transport=%s,server=y,suspend=y,'
+                 'address=33081' % transport]
     # Unpack the sources into the test directory and add to the class path
     # for ease of debugging, particularly with jdb.
     util.Unzip(os.path.join(java_tests_src_dir, 'test-nodeps-srcs.jar'),

chrome/test/chromedriver/test/run_py_tests.py

@@ -707,8 +707,7 @@ class ExistingBrowserTest(ChromeDriverBaseTest):
     if process is None:
       raise RuntimeError('Chrome could not be started with debugging port')
     try:
-      hostAndPort = '127.0.0.1:%d' % port
-      driver = self.CreateDriver(chrome_existing_browser=hostAndPort)
+      driver = self.CreateDriver(debugger_address='127.0.0.1:%d' % port)
       driver.ExecuteScript('console.info("%s")' % 'connecting at %d!' % port)
       driver.Quit()
     finally: