The user's consent to crash dumps reporting can now be set via the UI (Windows only). The checkbox …

remoting/host/breakpad_win.cc

Index: remoting/host/breakpad_win.cc
diff --git a/remoting/host/breakpad_win.cc b/remoting/host/breakpad_win.cc
index a2623ebb2614eec801d391bc62019a54a9bf7ab8..8f4b1016a8743a443adb4add9fe8f7b7337ac0d3 100644
--- a/remoting/host/breakpad_win.cc
+++ b/remoting/host/breakpad_win.cc
@@ -6,6 +6,7 @@
 
 #include <windows.h>
 
+#include "base/logging.h"
 #include "base/string16.h"
 #include "base/stringprintf.h"
 #include "base/win/registry.h"
@@ -15,7 +16,8 @@ namespace {
 
 // The following strings are used to construct the registry key names where
 // the user's consent to collect crash dumps is recorded.
-const wchar_t kOmahaClientStateKeyFormat[] = L"Google\\Update\\%ls\\%ls";
+const wchar_t kOmahaClientStateKeyFormat[] =
+    L"Software\\Google\\Update\\%ls\\%ls";
 const wchar_t kOmahaClientState[] = L"ClientState";
 const wchar_t kOmahaClientStateMedium[] = L"ClientStateMedium";
 const wchar_t kOmahaUsagestatsValue[] = L"usagestats";
@@ -24,7 +26,7 @@ const wchar_t kOmahaUsagestatsValue[] = L"usagestats";
 
 namespace remoting {
 
-bool IsCrashReportingEnabled() {
+bool GetUsageStatsConsent(bool* set_by_policy_opt, bool* allowed) {

[Jamie, 2012/06/14 23:43:56]

Is the _opt suffix intended to signify that the parameter is optional? If so,
then maybe it should be a prefix as is used in Javascript. Also, I think it's
more usual to make the "primary" return value the first parameter.

[alexeypa (please no reviews), 2012/06/19 23:27:29]

Done.

   // The user's consent to collect crash dumps is recored as the "usagestats"
   // value in the ClientState or ClientStateMedium key. Probe
   // the ClientStateMedium key first.
@@ -37,7 +39,11 @@ bool IsCrashReportingEnabled() {
     DWORD value = 0;
     result = key.ReadValueDW(kOmahaUsagestatsValue, &value);
     if (result == ERROR_SUCCESS) {
-      return value != 0;
+      if (set_by_policy_opt != NULL) {
+        *set_by_policy_opt = false;
+      }
+      *allowed = (value != 0);
+      return true;
     }
   }
 
@@ -49,11 +55,34 @@ bool IsCrashReportingEnabled() {
     DWORD value = 0;
     result = key.ReadValueDW(kOmahaUsagestatsValue, &value);
     if (result == ERROR_SUCCESS) {
-      return value != 0;
+      if (set_by_policy_opt != NULL) {
+        *set_by_policy_opt = false;

[Jamie, 2012/06/14 23:43:56]

You're setting |set_by_policy_opt| to false in both cases. Is that correct?

[alexeypa (please no reviews), 2012/06/19 23:27:29]

Yes. set_by_policy is not really used in this CL. It will be used when we start
checking policy to enable crash dump reporting just like Chrome does. I added it
now because it should be present in the COM interface and I'd like to avoid
changing the COM interface twice.

+      }
+      *allowed = (value != 0);
+      return true;
     }
   }
 
-  // Do not collect anything unless the user has explicitly allowed it.
+  return false;
+}
+
+bool SetUsageStatsConsent(bool allowed) {
+  DWORD value = allowed;
+  string16 client_state = StringPrintf(kOmahaClientStateKeyFormat,
+                                       kOmahaClientStateMedium,
+                                       remoting::kHostOmahaAppid);
+  base::win::RegKey key;
+  LONG result = key.Open(HKEY_LOCAL_MACHINE, client_state.c_str(),
+                           KEY_SET_VALUE);
+  if (result == ERROR_SUCCESS) {
+    result = key.WriteValue(kOmahaUsagestatsValue, value);
+    if (result == ERROR_SUCCESS) {
+      return true;
+    }
+  }
+
+  LOG_GETLASTERROR(ERROR)
+      << "Failed to record the user's consent to crash dump reporting";
   return false;
 }