Skip to content

GitLab

T
tangled
Commit 543a38d7 authored by Harry Cutts's avatar Harry Cutts Committed by Commit Bot
Browse files
Options

chromeos: use utility functions for prefs UMA reporting 

This makes the `if` statements in `ApplyPreferences` easier to read.

Bug: none
Test: change a preference and check chrome://histograms shows the sample
Change-Id: I99fdb9ad29d4f6df33444f7b12c20a1a5784f4d9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2491973Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Harry Cutts <hcutts@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826927}
parent 39ba84d4
Hide whitespace changes
Inline Side-by-side
Showing with 72 additions and 80 deletions
chrome/browser/chromeos/preferences.cc
View file @ 543a38d7
...@@ -643,6 +643,30 @@ void Preferences::OnPreferenceChanged(const std::string& pref_name) { ...@@ -643,6 +643,30 @@ void Preferences::OnPreferenceChanged(const std::string& pref_name) {
ApplyPreferences(REASON_PREF_CHANGED, pref_name); ApplyPreferences(REASON_PREF_CHANGED, pref_name);
} }
void Preferences::ReportBooleanPrefApplication(
ApplyReason reason,
const std::string& changed_histogram_name,
const std::string& started_histogram_name,
bool sample) {
if (reason == REASON_PREF_CHANGED)
base::UmaHistogramBoolean(changed_histogram_name, sample);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean(started_histogram_name, sample);
}
void Preferences::ReportSensitivityPrefApplication(
ApplyReason reason,
const std::string& changed_histogram_name,
const std::string& started_histogram_name,
int sensitivity_int) {
system::PointerSensitivity sensitivity =
static_cast<system::PointerSensitivity>(sensitivity_int);
if (reason == REASON_PREF_CHANGED)
base::UmaHistogramEnumeration(changed_histogram_name, sensitivity);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramEnumeration(started_histogram_name, sensitivity);
}
void Preferences::ApplyPreferences(ApplyReason reason, void Preferences::ApplyPreferences(ApplyReason reason,
const std::string& pref_name) { const std::string& pref_name) {
DCHECK(reason != REASON_PREF_CHANGED || !pref_name.empty()); DCHECK(reason != REASON_PREF_CHANGED || !pref_name.empty());
...@@ -668,10 +692,8 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -668,10 +692,8 @@ void Preferences::ApplyPreferences(ApplyReason reason,
const bool enabled = tap_to_click_enabled_.GetValue(); const bool enabled = tap_to_click_enabled_.GetValue();
if (user_is_active) if (user_is_active)
touchpad_settings.SetTapToClick(enabled); touchpad_settings.SetTapToClick(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Touchpad.TapToClick.Changed",
base::UmaHistogramBoolean("Touchpad.TapToClick.Changed", enabled); "Touchpad.TapToClick.Started", enabled);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Touchpad.TapToClick.Started", enabled);
// Save owner preference in local state to use on login screen. // Save owner preference in local state to use on login screen.
if (user_is_owner) { if (user_is_owner) {
...@@ -685,10 +707,8 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -685,10 +707,8 @@ void Preferences::ApplyPreferences(ApplyReason reason,
const bool enabled = three_finger_click_enabled_.GetValue(); const bool enabled = three_finger_click_enabled_.GetValue();
if (user_is_active) if (user_is_active)
touchpad_settings.SetThreeFingerClick(enabled); touchpad_settings.SetThreeFingerClick(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Touchpad.ThreeFingerClick.Changed",
base::UmaHistogramBoolean("Touchpad.ThreeFingerClick.Changed", enabled); "Touchpad.ThreeFingerClick.Started", enabled);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Touchpad.ThreeFingerClick.Started", enabled);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kUnifiedDesktopEnabledByDefault) { pref_name == ::prefs::kUnifiedDesktopEnabledByDefault) {
...@@ -710,20 +730,16 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -710,20 +730,16 @@ void Preferences::ApplyPreferences(ApplyReason reason,
DVLOG(1) << "Natural scroll set to " << enabled; DVLOG(1) << "Natural scroll set to " << enabled;
if (user_is_active) if (user_is_active)
touchpad_settings.SetNaturalScroll(enabled); touchpad_settings.SetNaturalScroll(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Touchpad.NaturalScroll.Changed",
base::UmaHistogramBoolean("Touchpad.NaturalScroll.Changed", enabled); "Touchpad.NaturalScroll.Started", enabled);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Touchpad.NaturalScroll.Started", enabled);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ash::prefs::kMouseReverseScroll) { pref_name == ash::prefs::kMouseReverseScroll) {
const bool enabled = mouse_reverse_scroll_.GetValue(); const bool enabled = mouse_reverse_scroll_.GetValue();
if (user_is_active) if (user_is_active)
mouse_settings.SetReverseScroll(enabled); mouse_settings.SetReverseScroll(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Mouse.ReverseScroll.Changed",
base::UmaHistogramBoolean("Mouse.ReverseScroll.Changed", enabled); "Mouse.ReverseScroll.Started", enabled);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Mouse.ReverseScroll.Started", enabled);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
...@@ -737,15 +753,9 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -737,15 +753,9 @@ void Preferences::ApplyPreferences(ApplyReason reason,
if (!AreScrollSettingsAllowed()) if (!AreScrollSettingsAllowed())
mouse_settings.SetScrollSensitivity(sensitivity_int); mouse_settings.SetScrollSensitivity(sensitivity_int);
} }
system::PointerSensitivity sensitivity = ReportSensitivityPrefApplication(reason, "Mouse.PointerSensitivity.Changed",
static_cast<system::PointerSensitivity>(sensitivity_int); "Mouse.PointerSensitivity.Started",
if (reason == REASON_PREF_CHANGED) { sensitivity_int);
base::UmaHistogramEnumeration("Mouse.PointerSensitivity.Changed",
sensitivity);
} else if (reason == REASON_INITIALIZATION) {
base::UmaHistogramEnumeration("Mouse.PointerSensitivity.Started",
sensitivity);
}
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kMouseScrollSensitivity) { pref_name == ::prefs::kMouseScrollSensitivity) {
...@@ -756,15 +766,9 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -756,15 +766,9 @@ void Preferences::ApplyPreferences(ApplyReason reason,
: mouse_sensitivity_.GetValue(); : mouse_sensitivity_.GetValue();
if (user_is_active) if (user_is_active)
mouse_settings.SetScrollSensitivity(sensitivity_int); mouse_settings.SetScrollSensitivity(sensitivity_int);
system::PointerSensitivity sensitivity = ReportSensitivityPrefApplication(reason, "Mouse.ScrollSensitivity.Changed",
static_cast<system::PointerSensitivity>(sensitivity_int); "Mouse.ScrollSensitivity.Started",
if (reason == REASON_PREF_CHANGED) { sensitivity_int);
base::UmaHistogramEnumeration("Mouse.ScrollSensitivity.Changed",
sensitivity);
} else if (reason == REASON_INITIALIZATION) {
base::UmaHistogramEnumeration("Mouse.ScrollSensitivity.Started",
sensitivity);
}
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kTouchpadSensitivity) { pref_name == ::prefs::kTouchpadSensitivity) {
...@@ -777,15 +781,9 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -777,15 +781,9 @@ void Preferences::ApplyPreferences(ApplyReason reason,
if (!AreScrollSettingsAllowed()) if (!AreScrollSettingsAllowed())
touchpad_settings.SetScrollSensitivity(sensitivity_int); touchpad_settings.SetScrollSensitivity(sensitivity_int);
} }
system::PointerSensitivity sensitivity = ReportSensitivityPrefApplication(
static_cast<system::PointerSensitivity>(sensitivity_int); reason, "Touchpad.PointerSensitivity.Changed",
if (reason == REASON_PREF_CHANGED) { "Touchpad.PointerSensitivity.Started", sensitivity_int);
base::UmaHistogramEnumeration("Touchpad.PointerSensitivity.Changed",
sensitivity);
} else if (reason == REASON_INITIALIZATION) {
base::UmaHistogramEnumeration("Touchpad.PointerSensitivity.Started",
sensitivity);
}
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kTouchpadScrollSensitivity) { pref_name == ::prefs::kTouchpadScrollSensitivity) {
...@@ -796,25 +794,17 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -796,25 +794,17 @@ void Preferences::ApplyPreferences(ApplyReason reason,
: touchpad_sensitivity_.GetValue(); : touchpad_sensitivity_.GetValue();
if (user_is_active) if (user_is_active)
touchpad_settings.SetScrollSensitivity(sensitivity_int); touchpad_settings.SetScrollSensitivity(sensitivity_int);
system::PointerSensitivity sensitivity = ReportSensitivityPrefApplication(
static_cast<system::PointerSensitivity>(sensitivity_int); reason, "Touchpad.ScrollSensitivity.Changed",
if (reason == REASON_PREF_CHANGED) { "Touchpad.ScrollSensitivity.Started", sensitivity_int);
base::UmaHistogramEnumeration("Touchpad.ScrollSensitivity.Changed",
sensitivity);
} else if (reason == REASON_INITIALIZATION) {
base::UmaHistogramEnumeration("Touchpad.ScrollSensitivity.Started",
sensitivity);
}
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kPrimaryMouseButtonRight) { pref_name == ::prefs::kPrimaryMouseButtonRight) {
const bool right = primary_mouse_button_right_.GetValue(); const bool right = primary_mouse_button_right_.GetValue();
if (user_is_active) if (user_is_active)
mouse_settings.SetPrimaryButtonRight(right); mouse_settings.SetPrimaryButtonRight(right);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Mouse.PrimaryButtonRight.Changed",
base::UmaHistogramBoolean("Mouse.PrimaryButtonRight.Changed", right); "Mouse.PrimaryButtonRight.Started", right);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Mouse.PrimaryButtonRight.Started", right);
// Save owner preference in local state to use on login screen. // Save owner preference in local state to use on login screen.
if (user_is_owner) { if (user_is_owner) {
PrefService* prefs = g_browser_process->local_state(); PrefService* prefs = g_browser_process->local_state();
...@@ -827,53 +817,42 @@ void Preferences::ApplyPreferences(ApplyReason reason, ...@@ -827,53 +817,42 @@ void Preferences::ApplyPreferences(ApplyReason reason,
const bool enabled = mouse_acceleration_.GetValue(); const bool enabled = mouse_acceleration_.GetValue();
if (user_is_active) if (user_is_active)
mouse_settings.SetAcceleration(enabled); mouse_settings.SetAcceleration(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Mouse.Acceleration.Changed",
base::UmaHistogramBoolean("Mouse.Acceleration.Changed", enabled); "Mouse.Acceleration.Started", enabled);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Mouse.Acceleration.Started", enabled);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kMouseScrollAcceleration) { pref_name == ::prefs::kMouseScrollAcceleration) {
const bool enabled = mouse_scroll_acceleration_.GetValue(); const bool enabled = mouse_scroll_acceleration_.GetValue();
if (user_is_active) if (user_is_active)
mouse_settings.SetScrollAcceleration(enabled); mouse_settings.SetScrollAcceleration(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Mouse.ScrollAcceleration.Changed",
base::UmaHistogramBoolean("Mouse.ScrollAcceleration.Changed", enabled); "Mouse.ScrollAcceleration.Started", enabled);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Mouse.ScrollAcceleration.Started", enabled);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kTouchpadAcceleration) { pref_name == ::prefs::kTouchpadAcceleration) {
const bool enabled = touchpad_acceleration_.GetValue(); const bool enabled = touchpad_acceleration_.GetValue();
if (user_is_active) if (user_is_active)
touchpad_settings.SetAcceleration(enabled); touchpad_settings.SetAcceleration(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Touchpad.Acceleration.Changed",
base::UmaHistogramBoolean("Touchpad.Acceleration.Changed", enabled); "Touchpad.Acceleration.Started", enabled);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean("Touchpad.Acceleration.Started", enabled);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kTouchpadScrollAcceleration) { pref_name == ::prefs::kTouchpadScrollAcceleration) {
const bool enabled = touchpad_scroll_acceleration_.GetValue(); const bool enabled = touchpad_scroll_acceleration_.GetValue();
if (user_is_active) if (user_is_active)
touchpad_settings.SetScrollAcceleration(enabled); touchpad_settings.SetScrollAcceleration(enabled);
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(reason, "Touchpad.ScrollAcceleration.Changed",
base::UmaHistogramBoolean("Touchpad.ScrollAcceleration.Changed", enabled); "Touchpad.ScrollAcceleration.Started",
else if (reason == REASON_INITIALIZATION) enabled);
base::UmaHistogramBoolean("Touchpad.ScrollAcceleration.Started", enabled);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
pref_name == ::prefs::kDownloadDefaultDirectory) { pref_name == ::prefs::kDownloadDefaultDirectory) {
const bool default_download_to_drive = drive::util::IsUnderDriveMountPoint( const bool default_download_to_drive = drive::util::IsUnderDriveMountPoint(
download_default_directory_.GetValue()); download_default_directory_.GetValue());
if (reason == REASON_PREF_CHANGED) ReportBooleanPrefApplication(
base::UmaHistogramBoolean( reason, "FileBrowser.DownloadDestination.IsGoogleDrive.Changed",
"FileBrowser.DownloadDestination.IsGoogleDrive.Changed", "FileBrowser.DownloadDestination.IsGoogleDrive.Started",
default_download_to_drive); default_download_to_drive);
else if (reason == REASON_INITIALIZATION)
base::UmaHistogramBoolean(
"FileBrowser.DownloadDestination.IsGoogleDrive.Started",
default_download_to_drive);
} }
if (reason != REASON_PREF_CHANGED || if (reason != REASON_PREF_CHANGED ||
......
chrome/browser/chromeos/preferences.h
View file @ 543a38d7
...@@ -76,6 +76,19 @@ class Preferences : public sync_preferences::PrefServiceSyncableObserver, ...@@ -76,6 +76,19 @@ class Preferences : public sync_preferences::PrefServiceSyncableObserver,
// Callback method for preference changes. // Callback method for preference changes.
void OnPreferenceChanged(const std::string& pref_name); void OnPreferenceChanged(const std::string& pref_name);
// Add a sample to the appropriate UMA histogram for a boolean preference.
void ReportBooleanPrefApplication(ApplyReason reason,
const std::string& changed_histogram_name,
const std::string& started_histogram_name,
bool sample);
// Add a sample to the appropriate UMA histogram for a sensitivity preference.
void ReportSensitivityPrefApplication(
ApplyReason reason,
const std::string& changed_histogram_name,
const std::string& started_histogram_name,
int sensitivity_int);
// This will set the OS settings when the preference changed or the user // This will set the OS settings when the preference changed or the user
// owning these preferences became active. Also this method is called on // owning these preferences became active. Also this method is called on
// initialization. The reason of the call is stored as the |reason| parameter. // initialization. The reason of the call is stored as the |reason| parameter.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment