Make Crashpad use the user data dir, rather than always default location

chrome/install_static/product_install_details.cc

Index: chrome/install_static/product_install_details.cc
diff --git a/chrome/install_static/product_install_details.cc b/chrome/install_static/product_install_details.cc
index 81009b3882792fad860f8d36df918a30f415439a..5ccd1c71bce57eaa2e4bc8ef6b01f078f1a8dcea 100644
--- a/chrome/install_static/product_install_details.cc
+++ b/chrome/install_static/product_install_details.cc
@@ -11,6 +11,7 @@
 
 #include "chrome/install_static/install_modes.h"
 #include "chrome/install_static/install_util.h"
+#include "chrome/install_static/policy_path_parser.h"
 #include "chrome_elf/nt_registry/nt_registry.h"
 
 namespace install_static {
@@ -38,6 +39,54 @@ const InstallConstants* FindInstallMode(const std::wstring& suffix) {
   return &kInstallModes[0];
 }
 
+// Retrieves a registry policy for the user data directory from the registry, if
+// one is set. If there's none set in either HKLM or HKCU, |user_data_dir| will
+// be unmodified.
+void GetUserDataDirFromRegistryPolicyIfSet(std::wstring* user_data_dir,
+                                           const InstallDetails& details) {
+  assert(user_data_dir);
+  std::wstring policies_path = L"SOFTWARE\\Policies\\";
+  AppendChromeInstallSubDirectory(&policies_path, false /* !include_suffix */,
+                                  details);
+
+  std::wstring value;
+
+  constexpr wchar_t kUserDataDirRegistryKeyName[] = L"UserDataDir";
+
+  // First, try HKLM.
+  if (nt::QueryRegValueSZ(nt::HKLM, nt::NONE, policies_path.c_str(),
+                             kUserDataDirRegistryKeyName, &value)) {

[grt (UTC plus 2), 2016/11/22 11:37:09]

nit: indentation

[scottmg, 2016/11/22 17:16:07]

Done.

+    *user_data_dir = ExpandPathVariables(value);

[grt (UTC plus 2), 2016/11/22 11:37:09]

hmm. this is going to be called from within chrome_elf's DllMain now. is that
not what you intended? oops, did i steer you in a bad direction?

[scottmg, 2016/11/22 17:16:07]

Yup, $#@#!

I was thinking it would be ok after https://codereview.chromium.org/2475863004/
(which was the point of doing this one) but no, this will hang under the same
conditions and re-induce
https://bugs.chromium.org/p/chromium/issues/detail?id=655788.

Hmm.

I really wish I'd got this landed before your CL now. :-)

[scottmg, 2016/11/22 20:40:41]

Wow, yeah, I suck. :( Sigh. This has to be initialized later.

OK, so I think I'll not use InstallDetails after all. Since it's only used from
chrome_elf and chrome.exe (not chrome.dll) it's reasonable to just ... call a
function. So I've done that.

[grt (UTC plus 2), 2016/11/22 21:06:58]

Does this mean that the user data dir will be computed twice: once in
chrome_elf.dll and once in chrome.exe?

+    return;
+  }
+
+  // Second, try HKCU.
+  if (nt::QueryRegValueSZ(nt::HKCU, nt::NONE, policies_path.c_str(),
+                             kUserDataDirRegistryKeyName, &value)) {
+    *user_data_dir = ExpandPathVariables(value);
+    return;
+  }
+}
+
+std::wstring MakeAbsoluteFilePath(const std::wstring& input) {
+  wchar_t file_path[MAX_PATH];
+  if (!_wfullpath(file_path, input.c_str(), _countof(file_path)))
+    return std::wstring();
+  return file_path;
+}
+
+// The same as GetUserDataDirectory(), but directly queries the global command
+// line object for the --user-data-dir flag. This is the more commonly used
+// function, where GetUserDataDirectory() is used primiarily for testing.
+bool GetUserDataDirectoryUsingProcessCommandLine(
+    const InstallDetails& details,
+    std::wstring* result,
+    std::wstring* invalid_supplied_directory) {
+  return GetUserDataDirectory(
+      GetSwitchValueFromCommandLine(::GetCommandLine(), kUserDataDirSwitch),
+      details, result, invalid_supplied_directory);
+}
+
 }  // namespace
 
 void InitializeProductDetailsForPrimaryModule() {
@@ -127,6 +176,67 @@ bool IsMultiInstall(const InstallConstants& mode, bool system_level) {
          args.find(L"--multi-install") != std::wstring::npos;
 }
 
+bool GetDefaultUserDataDirectory(const InstallDetails& details,
+                                 std::wstring* result) {
+  // This environment variable should be set on Windows Vista and later
+  // (https://msdn.microsoft.com/library/windows/desktop/dd378457.aspx).
+  std::wstring user_data_dir = GetEnvironmentString16(L"LOCALAPPDATA");
+
+  if (user_data_dir.empty()) {
+    // LOCALAPPDATA was not set; fallback to the temporary files path.
+    DWORD size = ::GetTempPath(0, nullptr);
+    if (!size)
+      return false;
+    user_data_dir.resize(size + 1);
+    size = ::GetTempPath(size + 1, &user_data_dir[0]);
+    if (!size || size >= user_data_dir.size())
+      return false;
+    user_data_dir.resize(size);
+  }
+
+  result->swap(user_data_dir);
+  if ((*result)[result->length() - 1] != L'\\')
+    result->push_back(L'\\');
+  AppendChromeInstallSubDirectory(result, true /* include_suffix */, details);
+  result->push_back(L'\\');
+  constexpr wchar_t kUserDataDirname[] = L"User Data";

[grt (UTC plus 2), 2016/11/22 11:37:09]

nit: inline the literal below?

[scottmg, 2016/11/22 17:16:07]

Done.

+  result->append(kUserDataDirname);
+  return true;
+}
+
+bool GetUserDataDirectory(const std::wstring& user_data_dir_from_command_line,
+                          const InstallDetails& details,
+                          std::wstring* result,
+                          std::wstring* invalid_supplied_directory) {
+  std::wstring user_data_dir = user_data_dir_from_command_line;
+
+  GetUserDataDirFromRegistryPolicyIfSet(&user_data_dir, details);
+
+  // On Windows, trailing separators leave Chrome in a bad state. See
+  // crbug.com/464616.
+  while (!user_data_dir.empty() &&
+         (user_data_dir.back() == '\\' || user_data_dir.back() == '/')) {
+    user_data_dir.pop_back();
+  }
+
+  bool got_valid_directory =
+      !user_data_dir.empty() && RecursiveDirectoryCreate(user_data_dir);
+  if (!got_valid_directory) {
+    *invalid_supplied_directory = user_data_dir;
+    got_valid_directory = GetDefaultUserDataDirectory(details, &user_data_dir);
+  }
+
+  // The Chrome implementation CHECKs() here in the browser process. We
+  // don't as this function is used to initialize crash reporting, so
+  // we would get no report of this failure.
+  assert(got_valid_directory);
+  if (!got_valid_directory)
+    return false;
+
+  *result = MakeAbsoluteFilePath(user_data_dir);
+  return true;
+}
+
 std::unique_ptr<PrimaryInstallDetails> MakeProductDetails(
     const std::wstring& exe_path) {
   std::unique_ptr<PrimaryInstallDetails> details(new PrimaryInstallDetails());
@@ -142,6 +252,13 @@ std::unique_ptr<PrimaryInstallDetails> MakeProductDetails(
   details->set_multi_install(multi_install);
   details->set_channel(DetermineChannel(*mode, system_level, multi_install));
 
+  std::wstring user_data_dir, invalid_user_data_dir;
+  GetUserDataDirectoryUsingProcessCommandLine(*details, &user_data_dir,

[grt (UTC plus 2), 2016/11/22 11:37:09]

rather than passing a partially-constructed InstallDetails around, what about
passing around an InstallConstants (|mode|) a la DetermineChannel?

[scottmg, 2016/11/22 17:16:07]

Done.

+                                              &invalid_user_data_dir);
+  details->set_user_data_dir(user_data_dir);
+  if (!invalid_user_data_dir.empty())
+    details->set_invalid_user_data_dir(invalid_user_data_dir);
+
   return details;
 }