Adding support for sampling crashes in Chrome on Windows.

chrome/browser/metrics/chrome_metrics_services_manager_client.cc

 // Copyright 2015 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/browser/metrics/chrome_metrics_services_manager_client.h"
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/metrics/chrome_metrics_service_client.h"
 #include "chrome/browser/metrics/variations/chrome_variations_service_client.h"
 #include "chrome/browser/metrics/variations/ui_string_overrider_factory.h"
 #include "chrome/browser/ui/browser_otr_state.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "components/metrics/enabled_state_provider.h"
 #include "components/metrics/metrics_state_manager.h"
 #include "components/prefs/pref_service.h"
 #include "components/rappor/rappor_service.h"
 #include "components/variations/service/variations_service.h"
 #include "components/variations/variations_associated_data.h"
 #include "content/public/browser/browser_thread.h"
 
+#if defined(OS_WIN)
+#include "base/win/registry.h"
+#include "chrome/common/chrome_constants.h"
+#include "chrome/installer/util/browser_distribution.h"
+#include "components/crash/content/app/crashpad.h"
+#endif  // OS_WIN
+
 namespace {
 
+#if defined(OS_WIN)
+// Type for the function pointer to enable and disable crash reporting on
+// windows. Needed because the function is loaded from chrome_elf.
+typedef void (*SetCrashUploadsEnabledPointer)(bool);
+
+// Registry path for metrics registry values.
+const wchar_t kChromeMetricsRegPath[] = L"\\MetricsReporting";
+
+// Registry key for sampling state. This is used to communicate with crashpad.

[Mark Mentovai, 2016/08/11 18:32:24]

Capital C here and on line 213 too.

[scottmg, 2016/08/11 21:38:03]

and line 202

[jwd, 2016/08/11 21:41:03]

Done.

[jwd, 2016/08/11 22:02:52]

Done.

+const wchar_t kRegUsageStatsInSample[] = L"UsageStatsInSample";
+
+// The name of the function used to set the uploads enabled state in
+// components/crash/content/app/crashpad.cc. This is used to call the function
+// through the chrome_elf dll.
+const char kCrashpadSetEnabledFunctionName[] = "SetUploadsEnabledImpl";
+#endif  // OS_WIN
+
 // Name of the variations param that defines the sampling rate.
 const char kRateParamName[] = "sampling_rate_per_mille";
 
 // Metrics reporting feature. This feature, along with user consent, controls if
 // recording and reporting are enabled. If the feature is enabled, but no
 // consent is given, then there will be no recording or reporting.
 const base::Feature kMetricsReportingFeature{"MetricsReporting",
                                              base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Posts |GoogleUpdateSettings::StoreMetricsClientInfo| on blocking pool thread
@@ skipping 124 matching lines @@
 bool ChromeMetricsServicesManagerClient::IsMetricsReportingEnabled() {
   return enabled_state_provider_->IsReportingEnabled();
 }
 
 bool ChromeMetricsServicesManagerClient::OnlyDoMetricsRecording() {
   const base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
   return cmdline->HasSwitch(switches::kMetricsRecordingOnly) ||
          cmdline->HasSwitch(switches::kEnableBenchmarking);
 }
 
+#if defined(OS_WIN)
+void ChromeMetricsServicesManagerClient::UpdateRunningServices(
+    bool may_record,
+    bool may_upload) {
+  // First, set the registry value so that crashpad will have the sampling state
+  // now and for subsequent runs.
+  const base::string16 registry_path =
+      BrowserDistribution::GetDistribution()->GetRegistryPath() +
+      kChromeMetricsRegPath;
+
+  base::win::RegKey reg_key;
+  if (reg_key.Create(HKEY_CURRENT_USER, registry_path.c_str(), KEY_SET_VALUE) ==
+      ERROR_SUCCESS)

[scottmg, 2016/08/11 21:38:03]

nit; {} around multiline if

[jwd, 2016/08/11 22:02:53]

Done.

+    reg_key.WriteValue(kRegUsageStatsInSample, IsClientInSample() ? 1 : 0);
+
+  // Next, get crashpad to pickup the sampling state for this session.

[scottmg, 2016/08/11 21:38:03]

pick up

[jwd, 2016/08/11 22:02:53]

Done.

+  static SetCrashUploadsEnabledPointer set_crash_enabled = []() {
+    // The crash reporting is handled by chrome_elf.dll.
+    HMODULE elf_module = GetModuleHandle(chrome::kChromeElfDllName);
+    return reinterpret_cast<SetCrashUploadsEnabledPointer>(
+        GetProcAddress(elf_module, kCrashpadSetEnabledFunctionName));
+  }();
+
+  if (set_crash_enabled) {
+    // Crashpad will use the kRegUsageStatsInSample registry value to apply
+    // sampling correctly, but may_record already reflects the sampling state.
+    // This isn't a problem though, since they will be consistent.

[scottmg, 2016/08/11 21:38:03]

It seems like instead of this comment, it'd be better just to tell the
chrome_elf side to "check again", so that we don't need to retrieve and then
send over the value that we expect to be the same. Would that work?

[jwd, 2016/08/11 22:02:53]

Not too sure exactly what you mean. The value passed to set_crash_enabled won't
always be the same as the sampling value, it also has the consent baked in. It
just happens that, by this point, MetricsServicesManager[Client] expects
may_record to be consent given && in sample.

Are you suggesting a new "check again" function in crashpad.cc and not call
SetUploadsEnabled here?
To be strictly right, it would need to apply all the same logic that
InitializeCrashpadImpl does for the enable_uploads value. This seems like more
work since it will have to retrieve the consent bit from registry and check
policy and such.

+    set_crash_enabled(may_record && may_upload);
+  }
+}
+#endif  // defined(OS_WIN)
+
 metrics::MetricsStateManager*
 ChromeMetricsServicesManagerClient::GetMetricsStateManager() {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (!metrics_state_manager_) {
     metrics_state_manager_ = metrics::MetricsStateManager::Create(
         local_state_, enabled_state_provider_.get(),
         base::Bind(&PostStoreMetricsClientInfo),
         base::Bind(&GoogleUpdateSettings::LoadMetricsClientInfo));
   }
   return metrics_state_manager_.get();
 }