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..381b40108296ec97925ece675434719e473d93f2
--- /dev/null
+++ b/chrome_elf/chrome_elf_util.cc
@@ -0,0 +1,138 @@
+// 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"
+
+namespace {
+
+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 kUninstallArgumentsField[] = L"UninstallArguments";
+
+
+const wchar_t kAppGuidCanary[] =
+    L"{4ea16ac7-fd5a-47c3-875b-dbf4a2008c20}";
+const wchar_t kAppGuidGoogleChrome[] =
+    L"{8A69D345-D564-463c-AFF1-A69D9E530F96}";
+const wchar_t kAppGuidGoogleBinaries[] =
+    L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}";
+
+LONG ReadValue(HKEY key, const wchar_t* name, void* data, DWORD* data_size,

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

This function really just directly wraps ::RegQueryValueEx. Why not use that
directly inline instead and do away with this function?

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+               DWORD* data_type) {
+  LONG result = ::RegQueryValueEx(key, name, 0, data_type,
+                                  reinterpret_cast<LPBYTE>(data), data_size);

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

LPBYTE -> BYTE*

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  return result;
+}
+
+bool ReadKeyValueString(bool system_install, const wchar_t* key_path,
+                  const wchar_t* guid, const wchar_t* value_to_read,

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

nit: line up the indent

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+                  wchar_t* value_out) {
+  HKEY h_key;

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

= NULL;

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  value_out = L"";
+
+  base::string16 full_key_path(key_path);
+  full_key_path.append(1, L'\\');
+  full_key_path.append(guid);
+
+  if (::RegOpenKeyEx(system_install? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
+          full_key_path.c_str(), 0, KEY_QUERY_VALUE, &h_key) != ERROR_SUCCESS) {
+    return false;
+  }
+
+  const size_t kMaxStringLength = 1024;
+  wchar_t raw_value[kMaxStringLength];

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

= {}

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  DWORD type = REG_SZ;
+  DWORD size = sizeof(raw_value);
+  LONG result = ReadValue(h_key, value_to_read, raw_value, &size, &type);
+
+  value_out = raw_value;

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

Can't return a buffer allocated on the stack in an out pointer like this, it
will have gone away when the caller tries to use it. I think you want to pass a
base::string16* as the out parameter, it will make things easier.

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+
+  ::RegCloseKey(h_key);
+
+  return ERROR_SUCCESS == result;
+}
+
+bool ReadKeyValueDW(bool system_install, const wchar_t* key_path,
+                  base::string16 guid, const wchar_t* value_to_read,
+                  DWORD* value_out) {
+  HKEY h_key;

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

= NULL;

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  *value_out = 0;
+
+  base::string16 full_key_path(key_path);
+  full_key_path.append(1, L'\\');
+  full_key_path.append(guid.c_str());
+
+  if (::RegOpenKeyEx(system_install? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
+          full_key_path.c_str(), 0, KEY_QUERY_VALUE, &h_key) != ERROR_SUCCESS) {
+    return false;
+  }
+
+  DWORD type = REG_DWORD;
+  DWORD size = sizeof(DWORD);
+  LONG result = ReadValue(h_key, value_to_read, value_out, &size, &type);
+
+  DWORD buffer_size = sizeof(DWORD);

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

unused?

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+
+  ::RegCloseKey(h_key);
+
+  return ERROR_SUCCESS == result;
+}
+
+}  // namespace
+
+bool IsCanary(wchar_t* exe_path) {
+  wchar_t* found = wcsstr(exe_path, L"Google\\Chrome SxS");

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

return wcsstr(exe_path, L"Google\\Chrome SxS") != NULL;

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  return !!found;
+}
+
+bool IsSystemInstall(wchar_t* exe_path) {
+  bool found = wcsncmp(exe_path, L"C:\\Program Files", 15) == 0;

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

this can be shortened too, though I still think we should check the env var
using the kernel32 API GetEnvironmentVariable

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  return found;
+}
+
+bool IsMultiInstall(wchar_t* exe_path) {
+  wchar_t* args = L"";

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

this is setting args to a static string, which isn't what we want. Use:

base::string16 args here instead.

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  if (ERROR_SUCCESS != ReadKeyValueString(IsSystemInstall(exe_path),
+          kRegPathClientState, kAppGuidGoogleChrome, kUninstallArgumentsField,
+          args)) {
+    return false;
+  }
+  wchar_t* found = wcsstr(args, L"--multi-install");
+  return !!found;

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

return wcsstr(args, L"--multi-install") != NULL;

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+}
+
+bool AreUsageStatsEnabled() {
+#if !defined(GOOGLE_CHROME_BUILD)
+  return false;
+#else
+  wchar_t exe_path[MAX_PATH] = {};
+  if (!GetModuleFileNameW(NULL, exe_path, MAX_PATH))
+    return false;
+
+  bool system_install = IsSystemInstall(exe_path);
+  base::string16 app_guid;
+
+  if (IsCanary(exe_path)) {
+    app_guid = kAppGuidCanary;
+  } else {
+    app_guid = IsMultiInstall(exe_path)? kAppGuidGoogleBinaries :
+                                         kAppGuidGoogleChrome;
+  }
+
+  DWORD out_value = 0;
+

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

nit: drop this blank line

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  if (system_install && ReadKeyValueDW(system_install,
+          kRegPathClientStateMedium, app_guid, kRegValueUsageStats,
+          &out_value)) {
+      return out_value == 1;

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

indent is off here.

[Cait (Slow), 2014/02/13 00:03:20]

Done.

+  }
+
+  return ReadKeyValueDW(system_install, kRegPathClientState,
+      app_guid, kRegValueUsageStats, &out_value) && out_value == 1;
+#endif  // defined(GOOGLE_CHROME_BUILD)
+}