Adding support for sampling crashes in Chrome on Windows.

components/crash/content/app/crashpad.cc

Index: components/crash/content/app/crashpad.cc
diff --git a/components/crash/content/app/crashpad.cc b/components/crash/content/app/crashpad.cc
index 85afe46af675145d2b1251ea5a3b9e5989123eb2..b5a3df3c3b6bbc43c33b97917d3a6fa473e0046c 100644
--- a/components/crash/content/app/crashpad.cc
+++ b/components/crash/content/app/crashpad.cc
@@ -264,18 +264,7 @@ void InitializeCrashpadImpl(bool initial_client,
     g_database =
         crashpad::CrashReportDatabase::Initialize(database_path).release();
 
-    bool enable_uploads = false;
-    if (!crash_reporter_client->ReportingIsEnforcedByPolicy(&enable_uploads)) {
-      // Breakpad provided a --disable-breakpad switch to disable crash dumping
-      // (not just uploading) here. Crashpad doesn't need it: dumping is enabled
-      // unconditionally and uploading is gated on consent, which tests/bots
-      // shouldn't have. As a precaution, uploading is also disabled on bots
-      // even if consent is present.
-      enable_uploads = crash_reporter_client->GetCollectStatsConsent() &&
-                       !crash_reporter_client->IsRunningUnattended();
-    }
-
-    SetUploadsEnabled(enable_uploads);
+    SetUploadConsent(crash_reporter_client->GetCollectStatsConsent());
   }
 }
 
@@ -292,11 +281,24 @@ void InitializeCrashpadWithEmbeddedHandler(bool initial_client,
 }
 #endif  // OS_WIN
 
-void SetUploadsEnabled(bool enable_uploads) {
-  if (g_database) {
-    crashpad::Settings* settings = g_database->GetSettings();
-    settings->SetUploadsEnabled(enable_uploads);
+void SetUploadConsent(bool consent) {
+  if (!g_database)
+    return;
+
+  bool enable_uploads = false;
+  CrashReporterClient* crash_reporter_client = GetCrashReporterClient();
+  if (!crash_reporter_client->ReportingIsEnforcedByPolicy(&enable_uploads)) {
+    // Breakpad provided a --disable-breakpad switch to disable crash dumping
+    // (not just uploading) here. Crashpad doesn't need it: dumping is enabled
+    // unconditionally and uploading is gated on consent, which tests/bots
+    // shouldn't have. As a precaution, uploading is also disabled on bots even
+    // if consent is present.
+    enable_uploads = consent && !crash_reporter_client->IsRunningUnattended();
   }
+
+  crashpad::Settings* settings = g_database->GetSettings();
+  settings->SetUploadsEnabled(enable_uploads &&
+                              crash_reporter_client->GetCollectStatsInSample());
 }
 
 bool GetUploadsEnabled() {
@@ -460,6 +462,14 @@ void ReadMainModuleAnnotationsForKasko(
 
 extern "C" {
 
+// This function is used in chrome_metrics_services_manager_client.cc to trigger
+// changes to the upload-enabled state. This is done when the metrics services
+// are initialized, and when the user changes their consent for uploads. See
+// crash_reporter::SetUploadConsent for effects.
+void __declspec(dllexport) __cdecl SetUploadConsentImpl(bool consent) {

[scottmg, 2016/08/18 19:49:10]

I'm sorry, I feel like I missed this along the way, but could you explain
(probably again) why this takes a bool instead of calling
crash_reporter_client->GetCollectStatsConsent()?

[jwd, 2016/08/18 20:21:33]

The reason I kept the argument is to avoid making it a requirement that the
consent value be fully persisted before calling this. On mac,
GetCollectStatsConsent relies on the consent file being written, on windows it
will be the registry values. The other parts of the logic don't really change
during a session (does policy?), and wouldn't trigger a call to this if they
did. 

It wouldn't be the biggest deal for Mac, just moving the call to the bottom of
GoogleUpdateSettings::SetCollectStatsConsent instead of the top, and my windows
code wouldn't need to change, it's already called after that point. 

The downsides are:
We wouldn't get crash coverage for the 21 lines of
GoogleUpdateSettings::SetCollectStatsConsent on Mac, and the windows code
doesn't make it exactly clear that it IS doing the right thing. And of course it
seemed a bit messy to me to have the required two steps.

[scottmg, 2016/08/18 20:33:18]

OK, I hadn't been looking at Mac. I sort of prefer one place that knows how to
read the consent bit, but your argument makes sense too.

Since this function is Windows-only, could you add DCHECK_EQ(consent,
crash_reporter_client->GetCollectStatsConsent()) here?

[jwd, 2016/08/18 21:07:34]

Done.

+  crash_reporter::SetUploadConsent(consent);
+}
+
 // NOTE: This function is used by SyzyASAN to annotate crash reports. If you
 // change the name or signature of this function you will break SyzyASAN
 // instrumented releases of Chrome. Please contact syzygy-team@chromium.org