[chromedriver] Load the automation extension as a component extension.

chrome/test/chromedriver/capabilities.cc

Index: chrome/test/chromedriver/capabilities.cc
diff --git a/chrome/test/chromedriver/capabilities.cc b/chrome/test/chromedriver/capabilities.cc
index 6ba3c7fe16adab35490374134f009910b42522d5..b77bedddca7bbac8931944e5d127716e18a67834 100644
--- a/chrome/test/chromedriver/capabilities.cc
+++ b/chrome/test/chromedriver/capabilities.cc
@@ -8,11 +8,13 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/json/string_escape.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/test/chromedriver/chrome/log.h"
 #include "chrome/test/chromedriver/chrome/status.h"
@@ -26,7 +28,7 @@ Status ParseBoolean(
     const base::Value& option,
     Capabilities* capabilities) {
   if (!option.GetAsBoolean(to_set))
-    return Status(kUnknownError, "value must be a boolean");
+    return Status(kUnknownError, "must be a boolean");
   return Status(kOk);
 }
 
@@ -35,13 +37,25 @@ Status ParseString(std::string* to_set,
                    Capabilities* capabilities) {
   std::string str;
   if (!option.GetAsString(&str))
-    return Status(kUnknownError, "value must be a string");
+    return Status(kUnknownError, "must be a string");
   if (str.empty())
-    return Status(kUnknownError, "value cannot be empty");
+    return Status(kUnknownError, "cannot be empty");
   *to_set = str;
   return Status(kOk);
 }
 
+Status ParseFilePath(base::FilePath* to_set,
+                     const base::Value& option,
+                     Capabilities* capabilities) {
+  base::FilePath::StringType str;
+  if (!option.GetAsString(&str))
+    return Status(kUnknownError, "must be a string");
+  if (str.empty())

[chrisgao (Use stgao instead), 2013/08/29 22:53:32]

I remember one webdriver client pass empty value for chromeOption.binary by
default.
If it isn't fixed yet, this may lead to break.

[kkania, 2013/08/30 03:14:57]

Let's have them fix their side.

+    return Status(kUnknownError, "cannot be empty");
+  *to_set = base::FilePath(str);
+  return Status(kOk);
+}
+
 Status IgnoreDeprecatedOption(
     Log* log,
     const char* option_name,
@@ -57,48 +71,22 @@ Status IgnoreCapability(const base::Value& option, Capabilities* capabilities) {
   return Status(kOk);
 }
 
-Status ParseChromeBinary(
-    const base::Value& option,
-    Capabilities* capabilities) {
-  base::FilePath::StringType path_str;
-  if (!option.GetAsString(&path_str))
-    return Status(kUnknownError, "'binary' must be a string");
-  base::FilePath chrome_exe(path_str);
-  capabilities->command.SetProgram(chrome_exe);
-  return Status(kOk);
-}
-
 Status ParseLogPath(const base::Value& option, Capabilities* capabilities) {
   if (!option.GetAsString(&capabilities->log_path))
-    return Status(kUnknownError, "'logPath' must be a string");
+    return Status(kUnknownError, "must be a string");
   return Status(kOk);
 }
 
-Status ParseArgs(bool is_android,
-                 const base::Value& option,
-                 Capabilities* capabilities) {
-  const base::ListValue* args_list = NULL;
-  if (!option.GetAsList(&args_list))
-    return Status(kUnknownError, "'args' must be a list");
-  for (size_t i = 0; i < args_list->GetSize(); ++i) {
+Status ParseSwitches(const base::Value& option,
+                     Capabilities* capabilities) {
+  const base::ListValue* switches_list = NULL;
+  if (!option.GetAsList(&switches_list))
+    return Status(kUnknownError, "must be a list");
+  for (size_t i = 0; i < switches_list->GetSize(); ++i) {
     std::string arg_string;
-    if (!args_list->GetString(i, &arg_string))
+    if (!switches_list->GetString(i, &arg_string))
       return Status(kUnknownError, "each argument must be a string");
-    if (is_android) {
-      capabilities->android_args += "--" + arg_string + " ";
-    } else {
-      size_t separator_index = arg_string.find("=");
-      if (separator_index != std::string::npos) {
-        CommandLine::StringType arg_string_native;
-        if (!args_list->GetString(i, &arg_string_native))
-          return Status(kUnknownError, "each argument must be a string");
-        capabilities->command.AppendSwitchNative(
-            arg_string.substr(0, separator_index),
-            arg_string_native.substr(separator_index + 1));
-      } else {
-        capabilities->command.AppendSwitch(arg_string);
-      }
-    }
+    capabilities->switches.SetSwitch(arg_string);

[chrisgao (Use stgao instead), 2013/08/29 22:53:32]

An arg_string could be "--load-extension=/path/to/ext".
We need to parse it though.
Using SetUnparsedSwitch instead?

[kkania, 2013/08/30 03:14:57]

Done.

   }
   return Status(kOk);
 }
@@ -106,7 +94,7 @@ Status ParseArgs(bool is_android,
 Status ParsePrefs(const base::Value& option, Capabilities* capabilities) {
   const base::DictionaryValue* prefs = NULL;
   if (!option.GetAsDictionary(&prefs))
-    return Status(kUnknownError, "'prefs' must be a dictionary");
+    return Status(kUnknownError, "must be a dictionary");
   capabilities->prefs.reset(prefs->DeepCopy());
   return Status(kOk);
 }
@@ -114,7 +102,7 @@ Status ParsePrefs(const base::Value& option, Capabilities* capabilities) {
 Status ParseLocalState(const base::Value& option, Capabilities* capabilities) {

[chrisgao (Use stgao instead), 2013/08/29 22:53:32]

Add a new function ParseDict instead of both ParsePrefs and ParseLocalState?

[kkania, 2013/08/30 03:14:57]

Done.

   const base::DictionaryValue* local_state = NULL;
   if (!option.GetAsDictionary(&local_state))
-    return Status(kUnknownError, "'localState' must be a dictionary");
+    return Status(kUnknownError, "must be a dictionary");
   capabilities->local_state.reset(local_state->DeepCopy());
   return Status(kOk);
 }
@@ -122,7 +110,7 @@ Status ParseLocalState(const base::Value& option, Capabilities* capabilities) {
 Status ParseExtensions(const base::Value& option, Capabilities* capabilities) {
   const base::ListValue* extensions = NULL;
   if (!option.GetAsList(&extensions))
-    return Status(kUnknownError, "'extensions' must be a list");
+    return Status(kUnknownError, "must be a list");
   for (size_t i = 0; i < extensions->GetSize(); ++i) {
     std::string extension;
     if (!extensions->GetString(i, &extension)) {
@@ -137,22 +125,22 @@ Status ParseExtensions(const base::Value& option, Capabilities* capabilities) {
 Status ParseProxy(const base::Value& option, Capabilities* capabilities) {
   const base::DictionaryValue* proxy_dict;
   if (!option.GetAsDictionary(&proxy_dict))
-    return Status(kUnknownError, "'proxy' must be a dictionary");
+    return Status(kUnknownError, "must be a dictionary");
   std::string proxy_type;
   if (!proxy_dict->GetString("proxyType", &proxy_type))
     return Status(kUnknownError, "'proxyType' must be a string");
   proxy_type = StringToLowerASCII(proxy_type);
   if (proxy_type == "direct") {
-    capabilities->command.AppendSwitch("no-proxy-server");
+    capabilities->switches.SetSwitch("no-proxy-server");
   } else if (proxy_type == "system") {
     // Chrome default.
   } else if (proxy_type == "pac") {
     CommandLine::StringType proxy_pac_url;
     if (!proxy_dict->GetString("proxyAutoconfigUrl", &proxy_pac_url))
       return Status(kUnknownError, "'proxyAutoconfigUrl' must be a string");
-    capabilities->command.AppendSwitchNative("proxy-pac-url", proxy_pac_url);
+    capabilities->switches.SetSwitch("proxy-pac-url", proxy_pac_url);
   } else if (proxy_type == "autodetect") {
-    capabilities->command.AppendSwitch("proxy-auto-detect");
+    capabilities->switches.SetSwitch("proxy-auto-detect");
   } else if (proxy_type == "manual") {
     const char* proxy_servers_options[][2] = {
         {"ftpProxy", "ftp"}, {"httpProxy", "http"}, {"sslProxy", "https"}};
@@ -191,10 +179,10 @@ Status ParseProxy(const base::Value& option, Capabilities* capabilities) {
                     "proxy capabilities were found");
     }
     if (!proxy_servers.empty())
-      capabilities->command.AppendSwitchASCII("proxy-server", proxy_servers);
+      capabilities->switches.SetSwitch("proxy-server", proxy_servers);
     if (!proxy_bypass_list.empty()) {
-      capabilities->command.AppendSwitchASCII("proxy-bypass-list",
-                                              proxy_bypass_list);
+      capabilities->switches.SetSwitch("proxy-bypass-list",
+                                       proxy_bypass_list);
     }
   } else {
     return Status(kUnknownError, "unrecognized proxy type:" + proxy_type);
@@ -206,7 +194,7 @@ Status ParseExcludeSwitches(const base::Value& option,
                             Capabilities* capabilities) {
   const base::ListValue* switches = NULL;
   if (!option.GetAsList(&switches))
-    return Status(kUnknownError, "'excludeSwitches' must be a list");
+    return Status(kUnknownError, "must be a list");
   for (size_t i = 0; i < switches->GetSize(); ++i) {
     std::string switch_name;
     if (!switches->GetString(i, &switch_name)) {
@@ -240,7 +228,7 @@ Status ParseLoggingPrefs(const base::Value& option,
                          Capabilities* capabilities) {
   const base::DictionaryValue* logging_prefs_dict = NULL;
   if (!option.GetAsDictionary(&logging_prefs_dict))
-    return Status(kUnknownError, "'loggingPrefs' must be a dictionary");
+    return Status(kUnknownError, "must be a dictionary");
 
   // TODO(klm): verify log types.
   // TODO(klm): verify log levels.
@@ -254,14 +242,15 @@ Status ParseChromeOptions(
     Capabilities* capabilities) {
   const base::DictionaryValue* chrome_options = NULL;
   if (!capability.GetAsDictionary(&chrome_options))
-    return Status(kUnknownError, "'chromeOptions' must be a dictionary");
+    return Status(kUnknownError, "must be a dictionary");
 
   bool is_android = chrome_options->HasKey("androidPackage");
   bool is_existing = chrome_options->HasKey("useExistingBrowser");
 
   std::map<std::string, Parser> parser_map;
-  // Ignore 'binary' and 'extensions' capability, since the Java client
-  // always passes them.
+  // Ignore 'args', 'binary' and 'extensions' capabilities by default, since the
+  // Java client always passes them.
+  parser_map["args"] = base::Bind(&IgnoreCapability);
   parser_map["binary"] = base::Bind(&IgnoreCapability);
   parser_map["extensions"] = base::Bind(&IgnoreCapability);

[chrisgao (Use stgao instead), 2013/08/29 22:53:32]

This function is for chromeOptions.*
But args, binary, and extensions here are in the same level as chromeOptions,
right?
They should be in Capabilities::Parse instead.

[kkania, 2013/08/30 03:14:57]

talked offline

   if (is_android) {
@@ -273,23 +262,24 @@ Status ParseChromeOptions(
         base::Bind(&ParseString, &capabilities->android_package);
     parser_map["androidProcess"] =
         base::Bind(&ParseString, &capabilities->android_process);
-    parser_map["args"] = base::Bind(&ParseArgs, true);
+    parser_map["args"] = base::Bind(&ParseSwitches);
+    parser_map["switches"] = base::Bind(&ParseSwitches);
   } else if (is_existing) {
-    parser_map["args"] = base::Bind(&IgnoreCapability);
     parser_map["useExistingBrowser"] = base::Bind(&ParseUseExistingBrowser);

[chrisgao (Use stgao instead), 2013/08/29 22:53:32]

not "debugger_address"?

[kkania, 2013/08/30 03:14:57]

Done.

   } else {
-    parser_map["forceDevToolsScreenshot"] = base::Bind(
-        &ParseBoolean, &capabilities->force_devtools_screenshot);
-    parser_map["args"] = base::Bind(&ParseArgs, false);
-    parser_map["binary"] = base::Bind(&ParseChromeBinary);
+    parser_map["args"] = base::Bind(&ParseSwitches);
+    parser_map["binary"] = base::Bind(&ParseFilePath, &capabilities->binary);
     parser_map["detach"] = base::Bind(&ParseBoolean, &capabilities->detach);
     parser_map["excludeSwitches"] = base::Bind(&ParseExcludeSwitches);
     parser_map["extensions"] = base::Bind(&ParseExtensions);
+    parser_map["forceDevToolsScreenshot"] = base::Bind(
+        &ParseBoolean, &capabilities->force_devtools_screenshot);
     parser_map["loadAsync"] =
         base::Bind(&IgnoreDeprecatedOption, log, "loadAsync");
     parser_map["localState"] = base::Bind(&ParseLocalState);
     parser_map["logPath"] = base::Bind(&ParseLogPath);
     parser_map["prefs"] = base::Bind(&ParsePrefs);
+    parser_map["switches"] = base::Bind(&ParseSwitches);

[chrisgao (Use stgao instead), 2013/08/29 22:53:32]

What's this new "switches" for? Equivalent to "args"?

[kkania, 2013/08/30 03:14:57]

It's a synonym for args. Deleted.

   }
 
   for (base::DictionaryValue::Iterator it(*chrome_options); !it.IsAtEnd();
@@ -307,11 +297,105 @@ Status ParseChromeOptions(
 
 }  // namespace
 
+Switches::Switches() {}
+
+Switches::~Switches() {}
+
+void Switches::SetSwitch(const std::string& name) {
+  SetSwitch(name, NativeString());
+}
+
+void Switches::SetSwitch(const std::string& name, const std::string& value) {
+#if defined(OS_WIN)
+  SetSwitch(name, UTF8ToUTF16(value));
+#else
+  switch_map_[name] = value;
+#endif
+}
+
+void Switches::SetSwitch(const std::string& name, const string16& value) {
+#if defined(OS_WIN)
+  switch_map_[name] = value;
+#else
+  SetSwitch(name, UTF16ToUTF8(value));
+#endif
+}
+
+void Switches::SetSwitch(const std::string& name, const base::FilePath& value) {
+  SetSwitch(name, value.value());
+}
+
+void Switches::SetUnparsedSwitch(const std::string& unparsed_switch) {
+  std::string value;
+  size_t equals_index = unparsed_switch.find('=');
+  if (equals_index != std::string::npos)
+    value = unparsed_switch.substr(equals_index + 1);
+
+  std::string name;
+  size_t start_index = 0;
+  if (unparsed_switch.substr(0, 2) == "--")
+    start_index = 2;
+  name = unparsed_switch.substr(start_index, equals_index - start_index);
+
+  SetSwitch(name, value);
+}
+
+bool Switches::HasSwitch(const std::string& name) const {
+  return switch_map_.count(name) > 0;
+}
+
+std::string Switches::GetSwitchValue(const std::string& name) const {
+  NativeString value = GetSwitchValueNative(name);
+#if defined(OS_WIN)
+  return UTF16ToUTF8(value);
+#else
+  return value;
+#endif
+}
+
+Switches::NativeString Switches::GetSwitchValueNative(
+    const std::string& name) const {
+  SwitchMap::const_iterator iter = switch_map_.find(name);
+  if (iter == switch_map_.end())
+    return NativeString();
+  return iter->second;
+}
+
+size_t Switches::GetSize() const {
+  return switch_map_.size();
+}
+
+void Switches::AppendToCommandLine(CommandLine* command) const {
+  for (SwitchMap::const_iterator iter = switch_map_.begin();
+       iter != switch_map_.end();
+       ++iter) {
+    command->AppendSwitchNative(iter->first, iter->second);
+  }
+}
+
+std::string Switches::ToString() const {
+  std::string str;
+  SwitchMap::const_iterator iter = switch_map_.begin();
+  while (iter != switch_map_.end()) {
+    str += "--" + iter->first;
+    std::string value = GetSwitchValue(iter->first);
+    if (value.length()) {
+      if (value.find(' ') != std::string::npos)
+        value = base::GetDoubleQuotedJson(value);
+      str += "=" + value;
+    }
+    ++iter;
+    if (iter == switch_map_.end())
+      break;
+    str += " ";
+  }
+  return str;
+}
+
 Capabilities::Capabilities()
     : force_devtools_screenshot(false),
       detach(false),
-      existing_browser_port(0),
-      command(CommandLine::NO_PROGRAM) {}
+      existing_browser_port(0) {}
 
 Capabilities::~Capabilities() {}