MD Settings does not control UMA flag |
|||||||||
Issue descriptionOpen MD design settings and go to Automatic send diagnostic and usage data to Google. Try to change this clicking on button. On each change reload chrome://settings-frame. Note that reload here is needed because old-style settings does not reflect runtime change of this option. On each reload we can see that chrome://settings-frame shows the same state of UMA, regardless of MD settings state. There is another convenient way to observe changes in UMA. Start ARC OptIn start page. At this page it has option about "send diagnostics ...". This page has observers on changing UMA option. If we change UMA option in chrome://settings-frame then ARC OptIn page reflects these changes but not for MD settings. In conclusion we have 2 bugs: 1 - chrome://settings-frame does not reflect runtime changes in UMA 2 - MD settings does not control UMA at all.
,
Mar 31 2017
,
Mar 31 2017
,
Apr 3 2017
,
Apr 3 2017
Talking to dbeam@ it sounds like we're directly monitoring the pref on Chrome OS, whereas desktop goes through some UMA middleware.
,
Apr 3 2017
,
Apr 3 2017
,
Apr 4 2017
So, there is some metrics code that appears to have never worked correctly. The old options UI changed both the chrome os setting (cros.metrics.reportingEnabled) and the chrome metrics setting (via ChangeMetricsReportingState()) explicitly. Currently an observer (actually just a callback) is added for cros.metrics.reportingEnabled, but the associated Subscription is never stored, so the observer is never called. The observer is what is intended to call ChangeMetricsReportingState() when the cros setting changes. I have a CL up to fix this and add a warning (which caught a similar bug in the old UI related to the timezone setting).
,
Apr 4 2017
,
Apr 5 2017
thank you for addressing this so quickly, stevenjb@!
,
Apr 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43c11caa73ca39e4d71f295b54abd7784a143d94 commit 43c11caa73ca39e4d71f295b54abd7784a143d94 Author: stevenjb <stevenjb@chromium.org> Date: Wed Apr 05 16:34:47 2017 CrOS settings/metrics: Correctly store Subscriptions Currently we do not store the Subscription for two CrosSettings observers. This CL: * Adds WARN_UNUSED_RESULT to catch this in the compiler. * Moves the chromeos::kStatsReportingPref observer code from metrics_reporting_state.cc to ChromeMetricsServicesManagerClient * Stores the Subscription for chromeos::kStatsReportingPref in ChromeMetricsServicesManagerClient. * Stores the Subscription for chromeos::kSystemTimezonePolicy in BrowserOptionsHander. BUG= 706920 Review-Url: https://codereview.chromium.org/2793393002 Cr-Commit-Position: refs/heads/master@{#462108} [modify] https://crrev.com/43c11caa73ca39e4d71f295b54abd7784a143d94/chrome/browser/chromeos/settings/cros_settings.h [modify] https://crrev.com/43c11caa73ca39e4d71f295b54abd7784a143d94/chrome/browser/metrics/chrome_metrics_services_manager_client.cc [modify] https://crrev.com/43c11caa73ca39e4d71f295b54abd7784a143d94/chrome/browser/metrics/chrome_metrics_services_manager_client.h [modify] https://crrev.com/43c11caa73ca39e4d71f295b54abd7784a143d94/chrome/browser/metrics/metrics_reporting_state.cc [modify] https://crrev.com/43c11caa73ca39e4d71f295b54abd7784a143d94/chrome/browser/metrics/metrics_reporting_state.h [modify] https://crrev.com/43c11caa73ca39e4d71f295b54abd7784a143d94/chrome/browser/ui/webui/options/browser_options_handler.cc [modify] https://crrev.com/43c11caa73ca39e4d71f295b54abd7784a143d94/chrome/browser/ui/webui/options/browser_options_handler.h
,
Apr 5 2017
,
Apr 20 2017
Verified on build 9460.4.0 |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by khmel@chromium.org
, Mar 30 2017