CommandLine: eliminate wstring-accepting AppendLooseValue

Instead use AppendArg variants which accept a FilePath or an ASCII string.

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

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

18 files changed, 63 insertions, 53 deletions

diff --git a/base/command_line.cc b/base/command_line.cc
index 5767669..9b531fa 100644
--- a/base/command_line.cc
+++ b/base/command_line.cc
@@ -422,11 +422,14 @@ void CommandLine::AppendSwitchNative(const std::string& switch_string,
   switches_[switch_string] = value;
 }
 
-void CommandLine::AppendLooseValue(const std::wstring& value) {
-  // TODO(evan): the quoting here is wrong, but current callers rely on it
-  // being wrong.  I have another branch which fixes all the callers.
+void CommandLine::AppendArg(const std::string& value) {
+  DCHECK(IsStringUTF8(value));
+  AppendArgNative(UTF8ToWide(value));
+}
+
+void CommandLine::AppendArgNative(const std::wstring& value) {
   command_line_string_.append(L" ");
-  command_line_string_.append(value);
+  command_line_string_.append(WindowsStyleQuote(value));
   args_.push_back(value);
 }
 
@@ -472,8 +475,13 @@ void CommandLine::AppendSwitchASCII(const std::string& switch_string,
   AppendSwitchNative(switch_string, value_string);
 }
 
-void CommandLine::AppendLooseValue(const std::wstring& value) {
-  argv_.push_back(base::SysWideToNativeMB(value));
+void CommandLine::AppendArg(const std::string& value) {
+  AppendArgNative(value);
+}
+
+void CommandLine::AppendArgNative(const std::string& value) {
+  DCHECK(IsStringUTF8(value));
+  argv_.push_back(value);
 }
 
 void CommandLine::AppendArguments(const CommandLine& other,
@@ -499,6 +507,10 @@ void CommandLine::PrependWrapper(const std::string& wrapper) {
 
 #endif
 
+void CommandLine::AppendArgPath(const FilePath& path) {
+  AppendArgNative(path.value());
+}
+
 void CommandLine::AppendSwitchPath(const std::string& switch_string,
                                    const FilePath& path) {
   AppendSwitchNative(switch_string, path.value());
diff --git a/base/command_line.h b/base/command_line.h
index 10fbc92..e4ac08b 100644
--- a/base/command_line.h
+++ b/base/command_line.h
@@ -141,8 +141,14 @@ class CommandLine {
   void AppendSwitchASCII(const std::string& switch_string,
                          const std::string& value);
 
-  // Append a loose value to the command line.
-  void AppendLooseValue(const std::wstring& value);
+  // Append an argument to the command line.
+  // Note on quoting: the argument will be quoted properly such that it is
+  // interpreted as one argument to the target command.
+  // AppendArg is primarily for ASCII; non-ASCII input will be
+  // interpreted as UTF-8.
+  void AppendArg(const std::string& value);
+  void AppendArgPath(const FilePath& value);
+  void AppendArgNative(const StringType& value);
 
   // Append the arguments from another command line to this one.
   // If |include_program| is true, include |other|'s program as well.
diff --git a/base/process_util_unittest.cc b/base/process_util_unittest.cc
index 8da89b0..6f4ed39 100644
--- a/base/process_util_unittest.cc
+++ b/base/process_util_unittest.cc
@@ -205,18 +205,16 @@ TEST_F(ProcessUtilTest, GetAppOutput) {
                                  .Append(FILE_PATH_LITERAL("python.exe"));
 
   CommandLine cmd_line(python_runtime);
-  cmd_line.AppendLooseValue(L"-c");
-  cmd_line.AppendLooseValue(L"\"import sys; sys.stdout.write('" +
-      ASCIIToWide(message) + L"');\"");
+  cmd_line.AppendArg("-c");
+  cmd_line.AppendArg("import sys; sys.stdout.write('" + message + "');");
   std::string output;
   ASSERT_TRUE(base::GetAppOutput(cmd_line, &output));
   EXPECT_EQ(message, output);
 
   // Let's make sure stderr is ignored.
   CommandLine other_cmd_line(python_runtime);
-  other_cmd_line.AppendLooseValue(L"-c");
-  other_cmd_line.AppendLooseValue(
-      L"\"import sys; sys.stderr.write('Hello!');\"");
+  other_cmd_line.AppendArg("-c");
+  other_cmd_line.AppendArg("import sys; sys.stderr.write('Hello!');");
   output.clear();
   ASSERT_TRUE(base::GetAppOutput(other_cmd_line, &output));
   EXPECT_EQ("", output);
diff --git a/chrome/app/chrome_main_uitest.cc b/chrome/app/chrome_main_uitest.cc
index 8d13b89..851a592 100644
--- a/chrome/app/chrome_main_uitest.cc
+++ b/chrome/app/chrome_main_uitest.cc
@@ -64,7 +64,7 @@ TEST_F(ChromeMainTest, ReuseBrowserInstanceWhenOpeningFile) {
   FilePath test_file = test_data_directory_.AppendASCII("empty.html");
 
   CommandLine command_line(CommandLine::ARGUMENTS_ONLY);
-  command_line.AppendLooseValue(test_file.ToWStringHack());
+  command_line.AppendArgPath(test_file);
   ASSERT_TRUE(LaunchAnotherBrowserBlockUntilClosed(command_line));
 
   ASSERT_TRUE(automation()->IsURLDisplayed(net::FilePathToFileURL(test_file)));
diff --git a/chrome/browser/browser_init_browsertest.cc b/chrome/browser/browser_init_browsertest.cc
index b6ad563..b9f8514 100644
--- a/chrome/browser/browser_init_browsertest.cc
+++ b/chrome/browser/browser_init_browsertest.cc
@@ -66,13 +66,13 @@ IN_PROC_BROWSER_TEST_F(BrowserInitTest, FLAKY_OpenURLsPopup) {
 // to start on most BuildBot runs and I don't want to add longer delays to
 // the test. I'll circle back and make this work properly when i get a chance.
 IN_PROC_BROWSER_TEST_F(BrowserInitTest, FLAKY_BlockBadURLs) {
-  const std::wstring testurlstr(L"http://localhost/");
-  const GURL testurl(WideToUTF16Hack(testurlstr));
+  const char* testurlstr = "http://localhost/";
+  const GURL testurl(testurlstr);
   CommandLine cmdline(CommandLine::ARGUMENTS_ONLY);
-  cmdline.AppendLooseValue(testurlstr);
-  cmdline.AppendLooseValue(std::wstring(L"javascript:alert('boo')"));
-  cmdline.AppendLooseValue(testurlstr);
-  cmdline.AppendLooseValue(std::wstring(L"view-source:http://localhost/"));
+  cmdline.AppendArg(testurlstr);
+  cmdline.AppendArg("javascript:alert('boo')");
+  cmdline.AppendArg(testurlstr);
+  cmdline.AppendArg("view-source:http://localhost/");
 
   // This will pick up the current browser instance.
   BrowserInit::LaunchWithProfile launch(FilePath(), cmdline);
diff --git a/chrome/browser/chromeos/login/apply_services_customization.cc b/chrome/browser/chromeos/login/apply_services_customization.cc
index d310d32..d455ded 100644
--- a/chrome/browser/chromeos/login/apply_services_customization.cc
+++ b/chrome/browser/chromeos/login/apply_services_customization.cc
@@ -136,8 +136,8 @@ void ApplyServicesCustomization::Apply(const std::string& manifest) {
   if (!customization.initial_start_page_url().empty()) {
     // Append partner's start page url to command line so it gets opened
     // on browser startup.
-    CommandLine::ForCurrentProcess()->AppendLooseValue(
-        UTF8ToWide(customization.initial_start_page_url()));
+    CommandLine::ForCurrentProcess()->AppendArg(
+        customization.initial_start_page_url());
     LOG(INFO) << "initial_start_page_url: "
               << customization.initial_start_page_url();
   }
diff --git a/chrome/browser/chromeos/login/existing_user_controller.cc b/chrome/browser/chromeos/login/existing_user_controller.cc
index 2aa1ede..6907a3c 100644
--- a/chrome/browser/chromeos/login/existing_user_controller.cc
+++ b/chrome/browser/chromeos/login/existing_user_controller.cc
@@ -333,10 +333,8 @@ void ExistingUserController::OnLoginFailure(const std::string& error) {
 }
 
 void ExistingUserController::AppendStartUrlToCmdline() {
-  if (start_url_.is_valid()) {
-    CommandLine::ForCurrentProcess()->AppendLooseValue(
-        UTF8ToWide(start_url_.spec()));
-  }
+  if (start_url_.is_valid())
+    CommandLine::ForCurrentProcess()->AppendArg(start_url_.spec());
 }
 
 void ExistingUserController::ClearCaptchaState() {
diff --git a/chrome/browser/chromeos/login/login_screen.cc b/chrome/browser/chromeos/login/login_screen.cc
index ddd71df..f150163 100644
--- a/chrome/browser/chromeos/login/login_screen.cc
+++ b/chrome/browser/chromeos/login/login_screen.cc
@@ -104,10 +104,8 @@ void LoginScreen::OnOffTheRecordLoginSuccess() {
 }
 
 void LoginScreen::AppendStartUrlToCmdline() {
-  if (start_url_.is_valid()) {
-    CommandLine::ForCurrentProcess()->AppendLooseValue(
-        UTF8ToWide(start_url_.spec()));
-  }
+  if (start_url_.is_valid())
+    CommandLine::ForCurrentProcess()->AppendArg(start_url_.spec());
 }
 
 void LoginScreen::ShowError(int error_id, const std::string& details) {
diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
index eda8d60..9d6df17 100644
--- a/chrome/browser/chromeos/login/login_utils.cc
+++ b/chrome/browser/chromeos/login/login_utils.cc
@@ -205,7 +205,7 @@ void LoginUtilsImpl::CompleteOffTheRecordLogin(const GURL& start_url) {
         switches::kLoginUser,
         UserManager::Get()->logged_in_user().email());
     if (start_url.is_valid())
-      command_line.AppendLooseValue(UTF8ToWide(start_url.spec()));
+      command_line.AppendArg(start_url.spec());
     CrosLibrary::Get()->GetLoginLibrary()->RestartJob(
         getpid(),
         command_line.command_line_string());
diff --git a/chrome/browser/process_singleton_linux_uitest.cc b/chrome/browser/process_singleton_linux_uitest.cc
index 9b77d71..a770023 100644
--- a/chrome/browser/process_singleton_linux_uitest.cc
+++ b/chrome/browser/process_singleton_linux_uitest.cc
@@ -72,7 +72,7 @@ CommandLine CommandLineForUrl(const std::string& url) {
     cmd_line->AppendSwitch(switches::kNoProcessSingletonDialog);
 
   CommandLine new_cmd_line(*cmd_line);
-  new_cmd_line.AppendLooseValue(ASCIIToWide(url));
+  new_cmd_line.AppendArg(url);
   return new_cmd_line;
 }
 
diff --git a/chrome/common/sandbox_policy.cc b/chrome/common/sandbox_policy.cc
index 5fcb98e..b8d9b8a 100644
--- a/chrome/common/sandbox_policy.cc
+++ b/chrome/common/sandbox_policy.cc
@@ -479,7 +479,7 @@ base::ProcessHandle StartProcessWithAccess(CommandLine* cmd_line,
   // Prefetch hints on windows:
   // Using a different prefetch profile per process type will allow Windows
   // to create separate pretetch settings for browser, renderer etc.
-  cmd_line->AppendLooseValue(StringPrintf(L"/prefetch:%d", type));
+  cmd_line->AppendArg(StringPrintf("/prefetch:%d", type));
 
   if (!in_sandbox) {
     base::LaunchApp(*cmd_line, false, false, &process);
diff --git a/chrome/test/automation/automation_proxy_uitest.cc b/chrome/test/automation/automation_proxy_uitest.cc
index 1a187b9..7f530bc 100644
--- a/chrome/test/automation/automation_proxy_uitest.cc
+++ b/chrome/test/automation/automation_proxy_uitest.cc
@@ -343,8 +343,8 @@ class AutomationProxyTest2 : public AutomationProxyVisibleTest {
 
     document2_ = test_data_directory_.AppendASCII("title2.html");
     launch_arguments_ = CommandLine(CommandLine::ARGUMENTS_ONLY);
-    launch_arguments_.AppendLooseValue(document1_.ToWStringHack());
-    launch_arguments_.AppendLooseValue(document2_.ToWStringHack());
+    launch_arguments_.AppendArgPath(document1_);
+    launch_arguments_.AppendArgPath(document2_);
   }
 
   FilePath document1_;
@@ -560,7 +560,7 @@ class AutomationProxyTest3 : public UITest {
 
     dom_automation_enabled_ = true;
     launch_arguments_ = CommandLine(CommandLine::ARGUMENTS_ONLY);
-    launch_arguments_.AppendLooseValue(document1_.ToWStringHack());
+    launch_arguments_.AppendArgPath(document1_);
   }
 
   FilePath document1_;
diff --git a/chrome/test/startup/startup_test.cc b/chrome/test/startup/startup_test.cc
index 1422e49..d2bff3d 100644
--- a/chrome/test/startup/startup_test.cc
+++ b/chrome/test/startup/startup_test.cc
@@ -50,7 +50,7 @@ class StartupTest : public UITest {
         FilePath(FilePath::kCurrentDirectory),
         FilePath(FILE_PATH_LITERAL("simple.html")));
     ASSERT_TRUE(file_util::PathExists(file_url));
-    launch_arguments_.AppendLooseValue(file_url.ToWStringHack());
+    launch_arguments_.AppendArgPath(file_url);
   }
 
   // Load a complex html file on startup represented by |which_tab|.
@@ -68,7 +68,7 @@ class StartupTest : public UITest {
         .AppendASCII("page_cycler").AppendASCII("moz")
         .AppendASCII(this_domain).AppendASCII("index.html");
     GURL file_url = net::FilePathToFileURL(page_cycler_path).Resolve("?skip");
-    launch_arguments_.AppendLooseValue(ASCIIToWide(file_url.spec()));
+    launch_arguments_.AppendArg(file_url.spec());
   }
 
   // Use the given profile in the test data extensions/profiles dir.  This tests
diff --git a/chrome/test/ui/ui_test.cc b/chrome/test/ui/ui_test.cc
index fc7eb42..0fc08ad 100644
--- a/chrome/test/ui/ui_test.cc
+++ b/chrome/test/ui/ui_test.cc
@@ -314,7 +314,7 @@ static CommandLine* CreateHttpServerCommandLine() {
   script_path = script_path.AppendASCII("new-run-webkit-httpd");
 
   CommandLine* cmd_line = CreatePythonCommandLine();
-  cmd_line->AppendLooseValue(script_path.ToWStringHack());
+  cmd_line->AppendArgPath(script_path);
   return cmd_line;
 }
 
@@ -1117,14 +1117,15 @@ bool UITestBase::LaunchBrowserHelper(const CommandLine& arguments,
   CommandLine command_line(command);
 
   // Add any explicit command line flags passed to the process.
-  std::wstring extra_chrome_flags =
-      CommandLine::ForCurrentProcess()->GetSwitchValue(kExtraChromeFlagsSwitch);
+  CommandLine::StringType extra_chrome_flags =
+      CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+          kExtraChromeFlagsSwitch);
   if (!extra_chrome_flags.empty()) {
     // Split by spaces and append to command line
-    std::vector<std::wstring> flags;
+    std::vector<CommandLine::StringType> flags;
     SplitString(extra_chrome_flags, ' ', &flags);
     for (size_t i = 0; i < flags.size(); ++i)
-      command_line.AppendLooseValue(flags[i]);
+      command_line.AppendArgNative(flags[i]);
   }
 
   // No first-run dialogs, please.
diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index a6dde7c..cdd6087 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -655,7 +655,7 @@ CommandLine* TestWebSocketServer::CreateWebSocketServerCommandLine() {
   script_path = script_path.AppendASCII("new-run-webkit-websocketserver");
 
   CommandLine* cmd_line = CreatePythonCommandLine();
-  cmd_line->AppendLooseValue(script_path.ToWStringHack());
+  cmd_line->AppendArgPath(script_path);
   return cmd_line;
 }
 
diff --git a/chrome_frame/test/chrome_frame_test_utils.cc b/chrome_frame/test/chrome_frame_test_utils.cc
index 5f59f6c..f5b1315 100644
--- a/chrome_frame/test/chrome_frame_test_utils.cc
+++ b/chrome_frame/test/chrome_frame_test_utils.cc
@@ -159,7 +159,7 @@ base::ProcessHandle LaunchExecutable(const std::wstring& executable,
     }
   } else {
     CommandLine cmdline((FilePath(path)));
-    cmdline.AppendLooseValue(argument);
+    cmdline.AppendArgNative(argument);
     if (!base::LaunchApp(cmdline, false, false, &process)) {
       LOG(ERROR) << "LaunchApp failed: " << ::GetLastError();
     }
@@ -181,11 +181,8 @@ base::ProcessHandle LaunchChrome(const std::wstring& url) {
   path = path.AppendASCII(kChromeImageName);
 
   CommandLine cmd(path);
-  std::wstring args = L"--";
-  args += ASCIIToWide(switches::kNoFirstRun);
-  args += L" ";
-  args += url;
-  cmd.AppendLooseValue(args);
+  cmd.AppendSwitch(switches::kNoFirstRun);
+  cmd.AppendArgNative(url);
 
   base::ProcessHandle process = NULL;
   base::LaunchApp(cmd, false, false, &process);
diff --git a/net/disk_cache/stress_cache.cc b/net/disk_cache/stress_cache.cc
index c7f469c..ade7427 100644
--- a/net/disk_cache/stress_cache.cc
+++ b/net/disk_cache/stress_cache.cc
@@ -44,7 +44,7 @@ int RunSlave(int iteration) {
   PathService::Get(base::FILE_EXE, &exe);
 
   CommandLine cmdline(exe);
-  cmdline.AppendLooseValue(UTF8ToWide(base::IntToString(iteration)));
+  cmdline.AppendArg(base::IntToString(iteration));
 
   base::ProcessHandle handle;
   if (!base::LaunchApp(cmdline, false, false, &handle)) {
diff --git a/net/tools/crash_cache/crash_cache.cc b/net/tools/crash_cache/crash_cache.cc
index 561f092..88d7d87 100644
--- a/net/tools/crash_cache/crash_cache.cc
+++ b/net/tools/crash_cache/crash_cache.cc
@@ -45,7 +45,7 @@ int RunSlave(RankCrashes action) {
   PathService::Get(base::FILE_EXE, &exe);
 
   CommandLine cmdline(exe);
-  cmdline.AppendLooseValue(UTF8ToWide(base::IntToString(action)));
+  cmdline.AppendArg(base::IntToString(action));
 
   base::ProcessHandle handle;
   if (!base::LaunchApp(cmdline, false, false, &handle)) {