Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2207)

Unified Diff: chrome/browser/metrics/metrics_reporting_state.cc

Issue 2248793002: Remove OnRecordingDisabled() metrics client interface. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: add helper function Created 4 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/metrics/metrics_reporting_state.cc
diff --git a/chrome/browser/metrics/metrics_reporting_state.cc b/chrome/browser/metrics/metrics_reporting_state.cc
index 58f34f97938720ed7c1a65cc085d08fb7935b86f..962d37cc65f843cac7c59fbb088f86f84d965038 100644
--- a/chrome/browser/metrics/metrics_reporting_state.cc
+++ b/chrome/browser/metrics/metrics_reporting_state.cc
@@ -9,6 +9,7 @@
#include "build/build_config.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/common/crash_keys.h"
#include "chrome/common/pref_names.h"
#include "chrome/installer/util/google_update_settings.h"
#include "components/metrics/metrics_pref_names.h"
@@ -60,34 +61,16 @@ bool SetGoogleUpdateSettings(bool enabled) {
void SetMetricsReporting(bool to_update_pref,
const OnMetricsReportingCallbackType& callback_fn,
bool updated_pref) {
- metrics::MetricsService* metrics = g_browser_process->metrics_service();
-
#if !defined(OS_ANDROID)
g_browser_process->local_state()->SetBoolean(
metrics::prefs::kMetricsReportingEnabled, updated_pref);
#endif // !defined(OS_ANDROID)
- // Clear the client id pref when opting out. Note: Mirrors code in
- // uma_session_stats.cc. TODO(asvitkine): Unify.
- if (!updated_pref) {
- // Note: Clearing client id will not affect the running state (e.g. field
- // trial randomization), as the pref is only read on startup.
- g_browser_process->local_state()->ClearPref(
- metrics::prefs::kMetricsClientID);
- g_browser_process->local_state()->ClearPref(
- metrics::prefs::kMetricsReportingEnabledTimestamp);
- }
+ UpdateMetricsPrefsOnPermissionChange(updated_pref);
// Uses the current state of whether reporting is enabled to enable services.
g_browser_process->GetMetricsServicesManager()->UpdateUploadPermissions(true);
- // When a user opts in to the metrics reporting service, the previously
- // collected data should be cleared to ensure that nothing is reported before
- // a user opts in and all reported data is accurate.
- // TODO(asvitkine): This logic should be added to uma_session_stats.cc too.
- if (updated_pref && metrics)
- metrics->ClearSavedStabilityMetrics();
-
if (to_update_pref == updated_pref) {
RecordMetricsReportingHistogramValue(updated_pref ?
METRICS_REPORTING_ENABLED : METRICS_REPORTING_DISABLED);
@@ -138,6 +121,24 @@ void ChangeMetricsReportingStateWithReply(
base::Bind(&SetMetricsReporting, enabled, callback_fn));
}
+void UpdateMetricsPrefsOnPermissionChange(bool metrics_enabled) {
jwd 2016/08/16 20:30:04 Didn't think to ask this in the last review, but t
Alexei Svitkine (slow) 2016/08/16 21:07:50 It's definitely a good question. There's a couple
+ if (metrics_enabled) {
+ // When a user opts in to the metrics reporting service, the previously
+ // collected data should be cleared to ensure that nothing is reported
+ // before a user opts in and all reported data is accurate.
+ g_browser_process->metrics_service()->ClearSavedStabilityMetrics();
+ } else {
+ // Clear the client id pref when opting out.
+ // Note: Clearing client id will not affect the running state (e.g. field
+ // trial randomization), as the pref is only read on startup.
+ g_browser_process->local_state()->ClearPref(
+ metrics::prefs::kMetricsClientID);
+ g_browser_process->local_state()->ClearPref(
+ metrics::prefs::kMetricsReportingEnabledTimestamp);
+ crash_keys::ClearMetricsClientId();
+ }
+}
+
bool IsMetricsReportingPolicyManaged() {
const PrefService* pref_service = g_browser_process->local_state();
const PrefService::Preference* pref =

Powered by Google App Engine
This is Rietveld 408576698