Breakpad coverage for chrome_elf start up

chrome_elf/chrome_elf_util.cc

Index: chrome_elf/chrome_elf_util.cc
diff --git a/chrome_elf/chrome_elf_util.cc b/chrome_elf/chrome_elf_util.cc
new file mode 100644
index 0000000000000000000000000000000000000000..db0884449515cb6b6b3f944bf0a6465884107f28
--- /dev/null
+++ b/chrome_elf/chrome_elf_util.cc
@@ -0,0 +1,92 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome_elf/chrome_elf_util.h"
+
+#include "base/strings/string16.h"
+#include "base/win/registry.h"

[robertshield, 2014/02/11 22:26:58]

To double check: this doesn't add any new dependencies to elf right?

[Cait (Slow), 2014/02/12 19:15:41]

It does :( I've removed it, and reinvented a few wheels instead.

+
+namespace {
+
+const wchar_t kRegPathClients[] = L"Software\\Google\\Update\\Clients";
+const wchar_t kRegPathClientState[] = L"Software\\Google\\Update\\ClientState";
+const wchar_t kRegPathClientStateMedium[] =
+    L"Software\\Google\\Update\\ClientStateMedium";
+const wchar_t kRegValueUsageStats[] = L"usagestats";
+const wchar_t kRegValuePV[] = L"pv";
+
+const wchar_t kCanaryGuid[] =
+    L"4EA16AC7-FD5A-47C3-875B-DBF4A2008C20";
+const wchar_t kChromeSingleInstallGuid[] =
+    L"8A69D345-D564-463c-AFF1-A69D9E530F96";
+const wchar_t kChromeMultiInstallGuid[] =
+    L"4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D";
+
+bool ReadKeyValue(bool system_install, const wchar_t* key_path,
+                  const wchar_t* guid, const wchar_t* value_to_read,
+                  DWORD* value_out) {
+  base::win::RegKey key;

[robertshield, 2014/02/11 22:26:58]

nit: declare |key| next to where it is used.

[Cait (Slow), 2014/02/12 19:15:41]

Done.

+  *value_out = 0;
+  bool have_value = false;
+
+  base::string16 full_key_path = base::string16(key_path).append(1, L'\\')
+                                                         .append(guid);

[robertshield, 2014/02/11 22:26:58]

This would be one less copy:

base::string16 full_key_path(key_path);
key_path.append(1, L'\\');
key_path.append(guid);

[Cait (Slow), 2014/02/12 19:15:41]

Done.

+
+  LONG result = key.Open(system_install? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
+                         full_key_path.c_str(), KEY_QUERY_VALUE);

[robertshield, 2014/02/11 22:26:58]

could shorten the last few lines to:

return key.Open(system_install? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
                          full_key_path.c_str(), KEY_QUERY_VALUE) ==
ERROR_SUCCESS &&
          key.ReadValueDW(value_to_read, value_out) == ERROR_SUCCESS;

without significant loss in readability

+
+  if (result != ERROR_SUCCESS) {
+    return false;
+  }
+
+  return key.ReadValueDW(value_to_read, value_out) == ERROR_SUCCESS;
+}
+
+}  // namespace
+
+bool IsCanary(LPWSTR exe_path) {
+  wchar_t* found = wcsstr(exe_path, L"Google\\Chrome SxS");
+  return !!found;
+}
+
+bool IsSystemInstall(LPWSTR exe_path) {
+  bool found = wcsncmp(exe_path, L"C:\\Program Files", 15) == 0;

[robertshield, 2014/02/11 22:26:58]

We can't use shell apis to get the thing the system thinks is the program files
path here can we?

If not, perhaps use ::GetEnvironmentVariable("PROGRAMFILES") or
::GetEnvironmentVariable("PROGRAMFILES(X86)")?

[Cait (Slow), 2014/02/12 19:15:41]

Unfortunately this check will be one of the first things that happens in
chrome_elf's DLLMain(), so Shell apis can't be used yet..

[robertshield, 2014/02/12 21:12:37]

GetEnvironmentVariable is a kernel32 function.

+  return found;
+}
+
+bool IsMultiInstall(LPWSTR exe_path) {
+  DWORD value = 0;
+  return ReadKeyValue(IsSystemInstall(exe_path), kRegPathClients,
+                      kChromeMultiInstallGuid, kRegValuePV, &value);
+}
+
+bool AreUsageStatsEnabled() {
+  DWORD out_value = 0;
+  wchar_t exe_path[MAX_PATH] = {};
+  if (!GetModuleFileNameW(NULL, exe_path, MAX_PATH))
+    return false;
+
+  bool system_install = IsSystemInstall(exe_path);
+  bool is_canary = IsCanary(exe_path);

[robertshield, 2014/02/11 22:26:58]

is_canary is only used once, could drop the temp var and make the call below

[Cait (Slow), 2014/02/12 19:15:41]

Done.

+
+  const wchar_t* chrome_guid = IsMultiInstall(exe_path) ?
+      kChromeMultiInstallGuid : kChromeSingleInstallGuid;
+
+  if (is_canary) {
+    return ReadKeyValue(false /*user level install*/, kRegPathClientState,
+                        kCanaryGuid, kRegValueUsageStats, &out_value) &&
+           out_value == 1;
+  } else if (system_install) {
+      return (ReadKeyValue(system_install, kRegPathClientStateMedium,
+                  chrome_guid, kRegValueUsageStats, &out_value) ||
+              ReadKeyValue(system_install, kRegPathClientState,
+                  chrome_guid, kRegValueUsageStats, &out_value)) &&
+              out_value == 1;
+  } else { // user-level install:
+      return ReadKeyValue(system_install, kRegPathClientState,
+                          chrome_guid, kRegValueUsageStats, &out_value) &&
+             out_value == 1;
+  }
+  return false;
+}