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

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

Issue 506663003: Consolidates accessing and setting the UMA pref to be within metrics code (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: fix and rollback unnecessary changes Created 6 years, 3 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 6c72ba66de08962bcf8ecef206143fdbd2e00fe4..23a2696dee47f33defb80328fb7bc828daab5524 100644
--- a/chrome/browser/metrics/metrics_reporting_state.cc
+++ b/chrome/browser/metrics/metrics_reporting_state.cc
@@ -4,33 +4,99 @@
#include "chrome/browser/metrics/metrics_reporting_state.h"
-#include "base/threading/thread_restrictions.h"
+#include "base/callback.h"
+#include "base/metrics/histogram.h"
+#include "base/prefs/pref_service.h"
#include "chrome/browser/browser_process.h"
+#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/common/pref_names.h"
#include "chrome/installer/util/google_update_settings.h"
#include "components/metrics/metrics_service.h"
+#include "content/public/browser/browser_thread.h"
-bool ResolveMetricsReportingEnabled(bool enabled) {
- // GoogleUpdateSettings touches the disk from the UI thread. MetricsService
- // also calls GoogleUpdateSettings below. http://crbug/62626
- base::ThreadRestrictions::ScopedAllowIO allow_io;
+namespace {
- GoogleUpdateSettings::SetCollectStatsConsent(enabled);
- bool update_pref = GoogleUpdateSettings::GetCollectStatsConsent();
+enum MetricsReportingChangeHistogramValue {
+ METRICS_REPORTING_ERROR,
+ METRICS_REPORTING_DISABLED,
+ METRICS_REPORTING_ENABLED,
+ METRICS_REPORTING_MAX
+};
- if (enabled != update_pref)
- DVLOG(1) << "Unable to set crash report status to " << enabled;
+void RecordMetricsReportingHistogramValue(
+ MetricsReportingChangeHistogramValue value) {
+ UMA_HISTOGRAM_ENUMERATION(
+ "UMA.MetricsReporting.Toggle", value, METRICS_REPORTING_MAX);
+}
- // Only change the pref if GoogleUpdateSettings::GetCollectStatsConsent
- // succeeds.
- enabled = update_pref;
+// Tries to set metrics reporting status to |enabled| and returns whatever is
+// the result of the update.
+bool SetGoogleUpdateSettings(bool enabled) {
+ GoogleUpdateSettings::SetCollectStatsConsent(enabled);
+ bool updated_pref = GoogleUpdateSettings::GetCollectStatsConsent();
+ if (enabled != updated_pref)
+ DVLOG(1) << "Unable to set metrics reporting status to " << enabled;
+ return updated_pref;
+}
+
+// Does the necessary changes for MetricsReportingEnabled changes which needs
+// to be done in the main thread.
+// As arguments this function gets:
+// |to_update_pref| which indicates what the desired update should be,
+// |callback_fn| is the callback function to be called in the end
+// |updated_pref| is the result of attempted update.
+// Update considers to be successful if |to_update_pref| and |updated_pref| are
+// the same.
+void SetMetricsReporting(bool to_update_pref,
+ const base::Callback<void(bool)> callback_fn,
+ bool updated_pref) {
metrics::MetricsService* metrics = g_browser_process->metrics_service();
if (metrics) {
- if (enabled)
+ if (updated_pref)
metrics->Start();
else
metrics->Stop();
}
+#if !defined(OS_CHROMEOS)
+ g_browser_process->local_state()->SetBoolean(
+ prefs::kMetricsReportingEnabled, updated_pref);
+#endif
+ if (to_update_pref == updated_pref) {
+ RecordMetricsReportingHistogramValue(updated_pref ?
+ METRICS_REPORTING_ENABLED : METRICS_REPORTING_DISABLED);
+ } else {
+ RecordMetricsReportingHistogramValue(METRICS_REPORTING_ERROR);
+ }
+ if (!callback_fn.is_null())
+ callback_fn.Run(updated_pref);
+}
+
+} // namespace
+
+void InitiateMetricsReportingChange(
+ bool enabled,
+ const base::Callback<void(bool)> callback_fn) {
+ if (!IsMetricsReportingUserChangable()) {
+ if (!callback_fn.is_null()) {
+ callback_fn.Run(
+ ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
+ }
+ return;
+ }
+
+ // Posts to FILE thread as SetGoogleUpdateSettings does IO operations.
+ content::BrowserThread::PostTaskAndReplyWithResult(
+ content::BrowserThread::FILE,
+ FROM_HERE,
+ base::Bind(&SetGoogleUpdateSettings, enabled),
+ base::Bind(&SetMetricsReporting, enabled, callback_fn));
+}
+
+bool IsMetricsReportingUserChangable() {
+ const PrefService* pref_service = g_browser_process->local_state();
- return enabled;
+ const PrefService::Preference* pref =
+ pref_service->FindPreference(prefs::kMetricsReportingEnabled);
+ return pref && !pref->IsManaged();
}

Powered by Google App Engine
This is Rietveld 408576698