[base] Use std::wstring for NativeEnvironmentString on Windows

This change modifies base::NativeEnvironmentString to be std::wstring on
Windows and updates affected code.

Bug: 911896
Change-Id: Ib39c473f42a2cd3384dd6fefd11021e4636629c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1891311Reviewed-by: Robert Liao <robliao@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712523}

base/environment.h

@@ -49,14 +49,12 @@ class BASE_EXPORT Environment {
 };
 
 #if defined(OS_WIN)
-using NativeEnvironmentString = string16;
-using EnvironmentMap =
-    std::map<NativeEnvironmentString, NativeEnvironmentString>;
+using NativeEnvironmentString = std::wstring;
 #elif defined(OS_POSIX) || defined(OS_FUCHSIA)
 using NativeEnvironmentString = std::string;
+#endif
 using EnvironmentMap =
     std::map<NativeEnvironmentString, NativeEnvironmentString>;
-#endif
 
 }  // namespace base
 

base/process/environment_internal.cc

@@ -87,14 +87,14 @@ std::unique_ptr<char* []> AlterEnvironment(const char* const* const env,
 
 #elif defined(OS_WIN)
 
-NativeEnvironmentString AlterEnvironment(const char16* env,
+NativeEnvironmentString AlterEnvironment(const wchar_t* env,
                                          const EnvironmentMap& changes) {
   NativeEnvironmentString result;
 
   // First build up all of the unchanged environment strings.
-  const char16* ptr = env;
+  const wchar_t* ptr = env;
   while (*ptr) {
-    string16 key;
+    std::wstring key;
     size_t line_length = ParseEnvLine(ptr, &key);
 
     // Keep only values not specified in the change vector.
@@ -107,8 +107,8 @@ NativeEnvironmentString AlterEnvironment(const char16* env,
   // Now append all modified and new values.
   for (const auto& i : changes) {
     // Windows environment blocks cannot handle keys or values with NULs.
-    CHECK_EQ(string16::npos, i.first.find(STRING16_LITERAL('\0')));
-    CHECK_EQ(string16::npos, i.second.find(STRING16_LITERAL('\0')));
+    CHECK_EQ(std::wstring::npos, i.first.find(L'\0'));
+    CHECK_EQ(std::wstring::npos, i.second.find(L'\0'));
     if (!i.second.empty()) {
       result += i.first;
       result.push_back('=');

base/process/environment_internal.h

@@ -43,7 +43,7 @@ BASE_EXPORT std::unique_ptr<char*[]> AlterEnvironment(
 // extra terminating null character. So, e.g., the environment A=1 B=2 is
 // represented as L"A=1\0B=2\0\0".
 BASE_EXPORT NativeEnvironmentString
-AlterEnvironment(const char16* env, const EnvironmentMap& changes);
+AlterEnvironment(const wchar_t* env, const EnvironmentMap& changes);
 #endif  // OS_*
 
 }  // namespace internal

base/process/environment_internal_unittest.cc

@@ -19,9 +19,9 @@ namespace internal {
 #if defined(OS_WIN)
 
 namespace {
-void ExpectEnvironmentBlock(const std::vector<string16>& vars,
-                            const string16& block) {
-  string16 expected;
+void ExpectEnvironmentBlock(const std::vector<std::wstring>& vars,
+                            const std::wstring& block) {
+  std::wstring expected;
   for (const auto& var : vars) {
     expected += var;
     expected.push_back('\0');
@@ -32,62 +32,60 @@ void ExpectEnvironmentBlock(const std::vector<string16>& vars,
 }  // namespace
 
 TEST_F(EnvironmentInternalTest, AlterEnvironment) {
-  const char16 empty[] = {'\0'};
-  const char16 a2[] = {'A', '=', '2', '\0', '\0'};
-  const char16 a2b3[] = {'A', '=', '2', '\0', 'B', '=', '3', '\0', '\0'};
+  const wchar_t empty[] = {'\0'};
+  const wchar_t a2[] = {'A', '=', '2', '\0', '\0'};
+  const wchar_t a2b3[] = {'A', '=', '2', '\0', 'B', '=', '3', '\0', '\0'};
   EnvironmentMap changes;
   NativeEnvironmentString e;
 
   e = AlterEnvironment(empty, changes);
   ExpectEnvironmentBlock({}, e);
 
-  changes[STRING16_LITERAL("A")] = STRING16_LITERAL("1");
+  changes[L"A"] = L"1";
   e = AlterEnvironment(empty, changes);
-  ExpectEnvironmentBlock({STRING16_LITERAL("A=1")}, e);
+  ExpectEnvironmentBlock({L"A=1"}, e);
 
   changes.clear();
-  changes[STRING16_LITERAL("A")] = string16();
+  changes[L"A"] = std::wstring();
   e = AlterEnvironment(empty, changes);
   ExpectEnvironmentBlock({}, e);
 
   changes.clear();
   e = AlterEnvironment(a2, changes);
-  ExpectEnvironmentBlock({STRING16_LITERAL("A=2")}, e);
+  ExpectEnvironmentBlock({L"A=2"}, e);
 
   changes.clear();
-  changes[STRING16_LITERAL("A")] = STRING16_LITERAL("1");
+  changes[L"A"] = L"1";
   e = AlterEnvironment(a2, changes);
-  ExpectEnvironmentBlock({STRING16_LITERAL("A=1")}, e);
+  ExpectEnvironmentBlock({L"A=1"}, e);
 
   changes.clear();
-  changes[STRING16_LITERAL("A")] = string16();
+  changes[L"A"] = std::wstring();
   e = AlterEnvironment(a2, changes);
   ExpectEnvironmentBlock({}, e);
 
   changes.clear();
-  changes[STRING16_LITERAL("A")] = string16();
-  changes[STRING16_LITERAL("B")] = string16();
+  changes[L"A"] = std::wstring();
+  changes[L"B"] = std::wstring();
   e = AlterEnvironment(a2b3, changes);
   ExpectEnvironmentBlock({}, e);
 
   changes.clear();
-  changes[STRING16_LITERAL("A")] = string16();
+  changes[L"A"] = std::wstring();
   e = AlterEnvironment(a2b3, changes);
-  ExpectEnvironmentBlock({STRING16_LITERAL("B=3")}, e);
+  ExpectEnvironmentBlock({L"B=3"}, e);
 
   changes.clear();
-  changes[STRING16_LITERAL("B")] = string16();
+  changes[L"B"] = std::wstring();
   e = AlterEnvironment(a2b3, changes);
-  ExpectEnvironmentBlock({STRING16_LITERAL("A=2")}, e);
+  ExpectEnvironmentBlock({L"A=2"}, e);
 
   changes.clear();
-  changes[STRING16_LITERAL("A")] = STRING16_LITERAL("1");
-  changes[STRING16_LITERAL("C")] = STRING16_LITERAL("4");
+  changes[L"A"] = L"1";
+  changes[L"C"] = L"4";
   e = AlterEnvironment(a2b3, changes);
   // AlterEnvironment() currently always puts changed entries at the end.
-  ExpectEnvironmentBlock({STRING16_LITERAL("B=3"), STRING16_LITERAL("A=1"),
-                          STRING16_LITERAL("C=4")},
-                         e);
+  ExpectEnvironmentBlock({L"B=3", L"A=1", L"C=4"}, e);
 }
 
 #else  // !OS_WIN

base/process/launch.h

@@ -14,6 +14,7 @@
 #include <vector>
 
 #include "base/base_export.h"
+#include "base/command_line.h"
 #include "base/environment.h"
 #include "base/macros.h"
 #include "base/process/process.h"
@@ -38,8 +39,6 @@
 
 namespace base {
 
-class CommandLine;
-
 #if defined(OS_WIN)
 typedef std::vector<HANDLE> HandlesToInheritVector;
 #elif defined(OS_FUCHSIA)
@@ -321,7 +320,7 @@ BASE_EXPORT Process LaunchProcess(const CommandLine& cmdline,
 //
 // Example (including literal quotes)
 //  cmdline = "c:\windows\explorer.exe" -foo "c:\bar\"
-BASE_EXPORT Process LaunchProcess(const string16& cmdline,
+BASE_EXPORT Process LaunchProcess(const CommandLine::StringType& cmdline,
                                   const LaunchOptions& options);
 
 // Launches a process with elevated privileges.  This does not behave exactly
@@ -379,7 +378,8 @@ BASE_EXPORT bool GetAppOutputWithExitCode(const CommandLine& cl,
 // A Windows-specific version of GetAppOutput that takes a command line string
 // instead of a CommandLine object. Useful for situations where you need to
 // control the command line arguments directly.
-BASE_EXPORT bool GetAppOutput(const StringPiece16& cl, std::string* output);
+BASE_EXPORT bool GetAppOutput(CommandLine::StringPieceType cl,
+                              std::string* output);
 #elif defined(OS_POSIX) || defined(OS_FUCHSIA)
 // A POSIX-specific version of GetAppOutput that takes an argv array
 // instead of a CommandLine.  Useful for situations where you need to

base/process/launch_win.cc

@@ -16,7 +16,6 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/command_line.h"
 #include "base/debug/activity_tracker.h"
 #include "base/debug/stack_trace.h"
 #include "base/logging.h"
@@ -36,7 +35,7 @@ namespace base {
 
 namespace {
 
-bool GetAppOutputInternal(const StringPiece16& cl,
+bool GetAppOutputInternal(CommandLine::StringPieceType cl,
                           bool include_stderr,
                           std::string* output,
                           int* exit_code) {
@@ -82,8 +81,8 @@ bool GetAppOutputInternal(const StringPiece16& cl,
 
   // Create the child process.
   PROCESS_INFORMATION temp_process_info = {};
-  if (!CreateProcess(nullptr, as_writable_wcstr(writable_command_line_string),
-                     nullptr, nullptr,
+  if (!CreateProcess(nullptr, data(writable_command_line_string), nullptr,
+                     nullptr,
                      TRUE,  // Handles are inherited.
                      0, nullptr, nullptr, &start_info, &temp_process_info)) {
     NOTREACHED() << "Failed to start process";
@@ -199,7 +198,7 @@ Process LaunchProcess(const CommandLine& cmdline,
   return LaunchProcess(cmdline.GetCommandLineString(), options);
 }
 
-Process LaunchProcess(const string16& cmdline,
+Process LaunchProcess(const CommandLine::StringType& cmdline,
                       const LaunchOptions& options) {
   // Mitigate the issues caused by loading DLLs on a background thread
   // (http://crbug/973868).
@@ -278,9 +277,9 @@ Process LaunchProcess(const string16& cmdline,
 
   LPCTSTR current_directory = options.current_directory.empty()
                                   ? nullptr
-                                  : as_wcstr(options.current_directory.value());
+                                  : options.current_directory.value().c_str();
 
-  string16 writable_cmdline(cmdline);
+  auto writable_cmdline(cmdline);
   DCHECK(!(flags & CREATE_SUSPENDED))
       << "Creating a suspended process can lead to hung processes if the "
       << "launching process is killed before it assigns the process to the"
@@ -299,21 +298,21 @@ Process LaunchProcess(const string16& cmdline,
     DCHECK(options.environment.empty());
 
     BOOL launched = CreateProcessAsUser(
-        options.as_user, nullptr, as_writable_wcstr(writable_cmdline), nullptr,
-        nullptr, inherit_handles, flags, environment_block, current_directory,
+        options.as_user, nullptr, data(writable_cmdline), nullptr, nullptr,
+        inherit_handles, flags, environment_block, current_directory,
         startup_info, &temp_process_info);
     DestroyEnvironmentBlock(environment_block);
     if (!launched) {
-      DPLOG(ERROR) << "Command line:" << std::endl << UTF16ToUTF8(cmdline)
-                   << std::endl;
+      DPLOG(ERROR) << "Command line:" << std::endl
+                   << WideToUTF8(cmdline) << std::endl;
       return Process();
     }
   } else {
-    char16* new_environment = nullptr;
-    string16 env_storage;
+    wchar_t* new_environment = nullptr;
+    std::wstring env_storage;
     if (options.clear_environment || !options.environment.empty()) {
       if (options.clear_environment) {
-        static const char16 kEmptyEnvironment[] = {0};
+        static const wchar_t kEmptyEnvironment[] = {0};
         env_storage =
             internal::AlterEnvironment(kEmptyEnvironment, options.environment);
       } else {
@@ -322,20 +321,18 @@ Process LaunchProcess(const string16& cmdline,
           DPLOG(ERROR);
           return Process();
         }
-        env_storage = internal::AlterEnvironment(as_u16cstr(old_environment),
-                                                 options.environment);
+        env_storage =
+            internal::AlterEnvironment(old_environment, options.environment);
         FreeEnvironmentStrings(old_environment);
       }
       new_environment = data(env_storage);
       flags |= CREATE_UNICODE_ENVIRONMENT;
     }
 
-    if (!CreateProcess(nullptr, as_writable_wcstr(writable_cmdline), nullptr,
-                       nullptr, inherit_handles, flags,
-                       as_writable_wcstr(new_environment), current_directory,
-                       startup_info, &temp_process_info)) {
-      DPLOG(ERROR) << "Command line:" << std::endl
-                   << UTF16ToUTF8(cmdline) << std::endl;
+    if (!CreateProcess(nullptr, data(writable_cmdline), nullptr, nullptr,
+                       inherit_handles, flags, new_environment,
+                       current_directory, startup_info, &temp_process_info)) {
+      DPLOG(ERROR) << "Command line:" << std::endl << cmdline << std::endl;
       return Process();
     }
   }
@@ -365,16 +362,16 @@ Process LaunchProcess(const string16& cmdline,
 
 Process LaunchElevatedProcess(const CommandLine& cmdline,
                               const LaunchOptions& options) {
-  const string16 file = cmdline.GetProgram().value();
-  const string16 arguments = cmdline.GetArgumentsString();
+  const FilePath::StringType file = cmdline.GetProgram().value();
+  const CommandLine::StringType arguments = cmdline.GetArgumentsString();
 
   SHELLEXECUTEINFO shex_info = {};
   shex_info.cbSize = sizeof(shex_info);
   shex_info.fMask = SEE_MASK_NOCLOSEPROCESS;
   shex_info.hwnd = GetActiveWindow();
   shex_info.lpVerb = L"runas";
-  shex_info.lpFile = as_wcstr(file);
-  shex_info.lpParameters = as_wcstr(arguments);
+  shex_info.lpFile = file.c_str();
+  shex_info.lpParameters = arguments.c_str();
   shex_info.lpDirectory = nullptr;
   shex_info.nShow = options.start_hidden ? SW_HIDE : SW_SHOWNORMAL;
   shex_info.hInstApp = nullptr;
@@ -419,7 +416,7 @@ bool GetAppOutputWithExitCode(const CommandLine& cl,
       cl.GetCommandLineString(), false, output, exit_code);
 }
 
-bool GetAppOutput(const StringPiece16& cl, std::string* output) {
+bool GetAppOutput(CommandLine::StringPieceType cl, std::string* output) {
   int exit_code;
   return GetAppOutputInternal(cl, false, output, &exit_code);
 }