summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorgayane <gayane@chromium.org>2015-11-19 13:06:15 -0800
committerCommit bot <commit-bot@chromium.org>2015-11-19 21:06:54 +0000
commite7e32dc0a88082d9fa8a85cca1eb36d74cc1966d (patch)
tree24a057343660d96aa6320c845f1caffbf1ef8ff6
parent1af0ff405f9a198534487fb56521a908a226022d (diff)
downloadchromium_src-e7e32dc0a88082d9fa8a85cca1eb36d74cc1966d.zip
chromium_src-e7e32dc0a88082d9fa8a85cca1eb36d74cc1966d.tar.gz
chromium_src-e7e32dc0a88082d9fa8a85cca1eb36d74cc1966d.tar.bz2
Use kMetricsReportingEnabled instead of kStatsReporingPref on metrics side
Main changes: - mapping kMetricsReportingEnabled to platform specific policy - change metrics code to use kMetricsReportingEnabled instead of kStatsReporingPref - For ChromeOS, initialize kMetricsReportingEnabled with kStatsReporingPref - Update both kMetricsReportingEnabled and kStatsReporingPref, when user toggles the checkbox - remove outdated migration code for kStatsReporingPref Here are the scenarios that I tested manually: - Start Chrome OS on fresh dir. Checkbox shows checked or unchecked based on what user selected during EULA accept. Check that its the same after restarting Chrome OS - Start Chrome OS on fresh user dir, test with non owner. Shows disabled checkbox with "controled by owner" icon. Its checked or unchecked based on what user selected on EULA accept - Start on Chrome OS on fresh dir, enroll to enterprise policy. Regardless of what was selected during EULA phase the checkbox is checked and disabled with "controled by policy" icon - Start Chrome OS without current changes on clean user dir, uncheck the checkbox. Start Chrome OS which includes current changes. Check that checkbox is still unchecked. BUG=532084 Review URL: https://codereview.chromium.org/1411863002 Cr-Commit-Position: refs/heads/master@{#360657}
-rw-r--r--chrome/browser/browser_process_impl.cc9
-rw-r--r--chrome/browser/chromeos/settings/device_settings_provider.cc60
-rw-r--r--chrome/browser/chromeos/settings/device_settings_provider.h13
-rw-r--r--chrome/browser/chromeos/settings/device_settings_provider_unittest.cc34
-rw-r--r--chrome/browser/metrics/chrome_metrics_service_accessor.cc25
-rw-r--r--chrome/browser/metrics/chrome_metrics_services_manager_client.cc2
-rw-r--r--chrome/browser/metrics/metrics_reporting_state.cc45
-rw-r--r--chrome/browser/metrics/metrics_reporting_state.h8
-rw-r--r--chrome/browser/policy/configuration_policy_handler_list_factory.cc14
-rw-r--r--chrome/browser/profiles/profile_io_data.cc17
-rw-r--r--chrome/browser/profiles/profile_io_data.h7
-rw-r--r--chrome/browser/resources/options/browser_options.html15
-rw-r--r--chrome/browser/resources/options/browser_options.js90
-rw-r--r--chrome/browser/ui/webui/options/browser_options_handler.cc58
-rw-r--r--chrome/browser/ui/webui/options/browser_options_handler.h12
15 files changed, 195 insertions, 214 deletions
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 79333e4..e09d2cf 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -831,11 +831,8 @@ void BrowserProcessImpl::RegisterPrefs(PrefRegistrySimple* registry) {
registry->RegisterStringPref(prefs::kHardwareKeyboardLayout,
std::string());
#endif // defined(OS_CHROMEOS)
-#if !defined(OS_CHROMEOS)
registry->RegisterBooleanPref(metrics::prefs::kMetricsReportingEnabled,
GoogleUpdateSettings::GetCollectStatsConsent());
-#endif // !defined(OS_CHROMEOS)
-
#if defined(OS_ANDROID)
registry->RegisterBooleanPref(
prefs::kCrashReportingEnabled, false);
@@ -1013,7 +1010,7 @@ void BrowserProcessImpl::CreateLocalState() {
// This preference must be kept in sync with external values; update them
// whenever the preference or its controlling policy changes.
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID) && !defined(OS_IOS)
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
pref_change_registrar_.Add(
metrics::prefs::kMetricsReportingEnabled,
base::Bind(&BrowserProcessImpl::ApplyMetricsReportingPolicy,
@@ -1052,7 +1049,7 @@ void BrowserProcessImpl::PreMainMessageLoopRun() {
if (local_state_->IsManagedPreference(prefs::kDefaultBrowserSettingEnabled))
ApplyDefaultBrowserPolicy();
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID) && !defined(OS_IOS)
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
ApplyMetricsReportingPolicy();
#endif
@@ -1215,7 +1212,7 @@ void BrowserProcessImpl::ApplyAllowCrossOriginAuthPromptPolicy() {
}
void BrowserProcessImpl::ApplyMetricsReportingPolicy() {
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID) && !defined(OS_IOS)
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
CHECK(BrowserThread::PostTask(
BrowserThread::FILE, FROM_HERE,
base::Bind(
diff --git a/chrome/browser/chromeos/settings/device_settings_provider.cc b/chrome/browser/chromeos/settings/device_settings_provider.cc
index df7694f..c78e9fe 100644
--- a/chrome/browser/chromeos/settings/device_settings_provider.cc
+++ b/chrome/browser/chromeos/settings/device_settings_provider.cc
@@ -87,15 +87,6 @@ const char* const kKnownSettings[] = {
kVariationsRestrictParameter,
};
-bool HasOldMetricsFile() {
- // TODO(pastarmovj): Remove this once migration is not needed anymore.
- // If the value is not set we should try to migrate legacy consent file.
- // Loading consent file state causes us to do blocking IO on UI thread.
- // Temporarily allow it until we fix http://crbug.com/62626
- base::ThreadRestrictions::ScopedAllowIO allow_io;
- return GoogleUpdateSettings::GetCollectStatsConsent();
-}
-
void DecodeLoginPolicies(
const em::ChromeDeviceSettingsProto& policy,
PrefValueMap* new_values_cache) {
@@ -373,7 +364,7 @@ void DecodeGenericPolicies(
new_values_cache->SetBoolean(kStatsReportingPref,
policy.metrics_enabled().metrics_enabled());
} else {
- new_values_cache->SetBoolean(kStatsReportingPref, HasOldMetricsFile());
+ new_values_cache->SetBoolean(kStatsReportingPref, false);
}
if (!policy.has_release_channel() ||
@@ -554,9 +545,6 @@ void DeviceSettingsProvider::DoSet(const std::string& path,
}
}
- bool metrics_value;
- if (path == kStatsReportingPref && in_value.GetAsBoolean(&metrics_value))
- ApplyMetricsSetting(false, metrics_value);
}
void DeviceSettingsProvider::OwnershipStatusChanged() {
@@ -596,9 +584,6 @@ void DeviceSettingsProvider::OwnershipStatusChanged() {
}
}
- // The owner key might have become available, allowing migration to happen.
- AttemptMigration();
-
ownership_status_ = new_ownership_status;
}
@@ -684,36 +669,6 @@ void DeviceSettingsProvider::UpdateValuesCache(
NotifyObservers(notifications[i]);
}
-void DeviceSettingsProvider::ApplyMetricsSetting(bool use_file,
- bool new_value) {
- // TODO(pastarmovj): Remove this once migration is not needed anymore.
- // If the value is not set we should try to migrate legacy consent file.
- if (use_file) {
- new_value = HasOldMetricsFile();
- // Make sure the values will get eventually written to the policy file.
- migration_values_.SetBoolean(kStatsReportingPref, new_value);
- AttemptMigration();
- VLOG(1) << "No metrics policy set will revert to checking "
- << "consent file which is "
- << (new_value ? "on." : "off.");
- UMA_HISTOGRAM_COUNTS("DeviceSettings.MetricsMigrated", 1);
- }
- VLOG(1) << "Metrics policy is being set to : " << new_value
- << "(use file : " << use_file << ")";
- // TODO(pastarmovj): Remove this once we don't need to regenerate the
- // consent file for the GUID anymore.
- InitiateMetricsReportingChange(new_value, OnMetricsReportingCallbackType());
-}
-
-void DeviceSettingsProvider::ApplySideEffects(
- const em::ChromeDeviceSettingsProto& settings) {
- // First migrate metrics settings as needed.
- if (settings.has_metrics_enabled())
- ApplyMetricsSetting(false, settings.metrics_enabled().metrics_enabled());
- else
- ApplyMetricsSetting(true, false);
-}
-
bool DeviceSettingsProvider::MitigateMissingPolicy() {
// First check if the device has been owned already and if not exit
// immediately.
@@ -794,10 +749,6 @@ bool DeviceSettingsProvider::UpdateFromService() {
UpdateValuesCache(*policy_data, *device_settings, TRUSTED);
device_settings_ = *device_settings;
- // TODO(pastarmovj): Make those side effects responsibility of the
- // respective subsystems.
- ApplySideEffects(*device_settings);
-
settings_loaded = true;
} else {
// Initial policy load is still pending.
@@ -839,13 +790,4 @@ bool DeviceSettingsProvider::UpdateFromService() {
return settings_loaded;
}
-void DeviceSettingsProvider::AttemptMigration() {
- if (device_settings_service_->HasPrivateOwnerKey()) {
- PrefValueMap::const_iterator i;
- for (i = migration_values_.begin(); i != migration_values_.end(); ++i)
- DoSet(i->first, *i->second);
- migration_values_.Clear();
- }
-}
-
} // namespace chromeos
diff --git a/chrome/browser/chromeos/settings/device_settings_provider.h b/chrome/browser/chromeos/settings/device_settings_provider.h
index 4705e61..c5eddfb 100644
--- a/chrome/browser/chromeos/settings/device_settings_provider.h
+++ b/chrome/browser/chromeos/settings/device_settings_provider.h
@@ -77,19 +77,11 @@ class DeviceSettingsProvider
const enterprise_management::ChromeDeviceSettingsProto& settings,
TrustedStatus trusted_status);
- // Applies the metrics policy and if not set migrates the legacy file.
- void ApplyMetricsSetting(bool use_file, bool new_value);
-
// Applies the data roaming policy.
void ApplyRoamingSetting(bool new_value);
void ApplyRoamingSettingFromProto(
const enterprise_management::ChromeDeviceSettingsProto& settings);
- // Applies any changes of the policies that are not handled by the respective
- // subsystems.
- void ApplySideEffects(
- const enterprise_management::ChromeDeviceSettingsProto& settings);
-
// In case of missing policy blob we should verify if this is upgrade of
// machine owned from pre version 12 OS and the user never touched the device
// settings. In this case revert to defaults and let people in until the owner
@@ -109,11 +101,6 @@ class DeviceSettingsProvider
// if new settings have been loaded.
bool UpdateFromService();
- // Checks the current ownership status to see whether the device owner is
- // logged in and writes the data accumulated in |migration_values_| to proper
- // device settings.
- void AttemptMigration();
-
// Pending callbacks that need to be invoked after settings verification.
std::vector<base::Closure> callbacks_;
diff --git a/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc b/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc
index 754e8ff..7315a5d4 100644
--- a/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc
+++ b/chrome/browser/chromeos/settings/device_settings_provider_unittest.cc
@@ -111,6 +111,18 @@ class DeviceSettingsProviderTest : public DeviceSettingsTestBase {
Mock::VerifyAndClearExpectations(this);
}
+ // Helper routine to enable/disable metrics report upload settings in policy.
+ void SetMetricsReportingSettings(bool enable_metrics_reporting) {
+ EXPECT_CALL(*this, SettingChanged(_)).Times(AtLeast(1));
+ em::MetricsEnabledProto* proto =
+ device_policy_.payload().mutable_metrics_enabled();
+ proto->set_metrics_enabled(enable_metrics_reporting);
+ device_policy_.Build();
+ device_settings_test_helper_.set_policy_blob(device_policy_.GetBlob());
+ ReloadDeviceSettings();
+ Mock::VerifyAndClearExpectations(this);
+ }
+
// Helper routine to ensure all heartbeat policies have been correctly
// decoded.
void VerifyHeartbeatSettings(bool expected_enable_state,
@@ -241,6 +253,8 @@ TEST_F(DeviceSettingsProviderTest, InitializationTestUnowned) {
}
TEST_F(DeviceSettingsProviderTest, SetPrefFailed) {
+ SetMetricsReportingSettings(false);
+
// If we are not the owner no sets should work.
base::FundamentalValue value(true);
EXPECT_CALL(*this, SettingChanged(kStatsReportingPref)).Times(1);
@@ -366,26 +380,6 @@ TEST_F(DeviceSettingsProviderTest, PolicyLoadNotification) {
Mock::VerifyAndClearExpectations(this);
}
-TEST_F(DeviceSettingsProviderTest, StatsReportingMigration) {
- // Create the legacy consent file.
- base::FilePath consent_file;
- ASSERT_TRUE(PathService::Get(chrome::DIR_USER_DATA, &consent_file));
- consent_file = consent_file.AppendASCII("Consent To Send Stats");
- ASSERT_EQ(1, base::WriteFile(consent_file, "0", 1));
-
- // This should trigger migration because the metrics policy isn't in the blob.
- device_settings_test_helper_.set_policy_blob(std::string());
- FlushDeviceSettings();
- EXPECT_EQ(std::string(), device_settings_test_helper_.policy_blob());
-
- // Verify that migration has kicked in.
- const base::Value* saved_value = provider_->Get(kStatsReportingPref);
- ASSERT_TRUE(saved_value);
- bool bool_value;
- EXPECT_TRUE(saved_value->GetAsBoolean(&bool_value));
- EXPECT_FALSE(bool_value);
-}
-
TEST_F(DeviceSettingsProviderTest, LegacyDeviceLocalAccounts) {
EXPECT_CALL(*this, SettingChanged(_)).Times(AnyNumber());
em::DeviceLocalAccountInfoProto* account =
diff --git a/chrome/browser/metrics/chrome_metrics_service_accessor.cc b/chrome/browser/metrics/chrome_metrics_service_accessor.cc
index 5f61b0f..e4416f7 100644
--- a/chrome/browser/metrics/chrome_metrics_service_accessor.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_accessor.cc
@@ -21,31 +21,28 @@ bool ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled() {
!content::BrowserThread::IsMessageLoopValid(content::BrowserThread::UI) ||
content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
+ // This is only possible during unit tests. If the unit test didn't set the
+ // local_state then it doesn't care about pref value and therefore we return
+ // false.
+ if (!g_browser_process->local_state()) {
+ DLOG(WARNING) << "Local state has not been set and pref cannot be read";
+ return false;
+ }
+
+#if !defined(OS_ANDROID)
return IsMetricsReportingEnabled(g_browser_process->local_state());
#else
- // ChromeOS and Android currently obtain the value for whether the user has
+ // Android currently obtain the value for whether the user has
// obtain metrics reporting in non-standard ways.
// TODO(gayane): Consolidate metric prefs on all platforms and eliminate this
// special-case code, instead having all platforms go through the above flow.
// http://crbug.com/362192, http://crbug.com/532084
bool pref_value = false;
-#if defined(OS_CHROMEOS)
- // TODO(gayane): The check is added as a temporary fix for unittests. It's
- // not expected to happen from production code. This should be cleaned up
- // soon when metrics pref from cros will be eliminated.
- if (chromeos::CrosSettings::IsInitialized()) {
- chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
- &pref_value);
- }
-#else // Android
pref_value = g_browser_process->local_state()->GetBoolean(
prefs::kCrashReportingEnabled);
-#endif // defined(OS_CHROMEOS)
-
return IsMetricsReportingEnabledWithPrefValue(pref_value);
-#endif // !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
+#endif // !defined(OS_ANDROID)
}
// static
diff --git a/chrome/browser/metrics/chrome_metrics_services_manager_client.cc b/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
index 83dcd36..7e69f9a 100644
--- a/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
@@ -36,6 +36,8 @@ ChromeMetricsServicesManagerClient::ChromeMetricsServicesManagerClient(
PrefService* local_state)
: local_state_(local_state) {
DCHECK(local_state);
+
+ SetupMetricsStateForChromeOS();
}
ChromeMetricsServicesManagerClient::~ChromeMetricsServicesManagerClient() {}
diff --git a/chrome/browser/metrics/metrics_reporting_state.cc b/chrome/browser/metrics/metrics_reporting_state.cc
index 0666443a..c3d0fbb 100644
--- a/chrome/browser/metrics/metrics_reporting_state.cc
+++ b/chrome/browser/metrics/metrics_reporting_state.cc
@@ -14,6 +14,11 @@
#include "components/metrics/metrics_service.h"
#include "content/public/browser/browser_thread.h"
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chromeos/settings/cros_settings_names.h"
+#endif // defined(OS_CHROMEOS)
+
namespace {
enum MetricsReportingChangeHistogramValue {
@@ -58,10 +63,12 @@ void SetMetricsReporting(bool to_update_pref,
else
metrics->Stop();
}
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
+
+#if !defined(OS_ANDROID)
g_browser_process->local_state()->SetBoolean(
metrics::prefs::kMetricsReportingEnabled, updated_pref);
-#endif
+#endif // !defined(OS_ANDROID)
+
// 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.
@@ -78,13 +85,27 @@ void SetMetricsReporting(bool to_update_pref,
callback_fn.Run(updated_pref);
}
+#if defined(OS_CHROMEOS)
+// Callback function for Chrome OS device settings change, so that the update is
+// applied to metrics reporting state.
+void OnDeviceSettingChange() {
+ bool enable_metrics = false;
+ chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
+ &enable_metrics);
+ InitiateMetricsReportingChange(enable_metrics,
+ OnMetricsReportingCallbackType());
+}
+#endif
+
} // namespace
+// TODO(gayane): Instead of checking policy before setting the metrics pref set
+// the pref and register for notifications for the rest of the changes.
void InitiateMetricsReportingChange(
bool enabled,
const OnMetricsReportingCallbackType& callback_fn) {
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
- if (!IsMetricsReportingUserChangable()) {
+#if !defined(OS_ANDROID)
+ if (IsMetricsReportingPolicyManaged()) {
if (!callback_fn.is_null()) {
callback_fn.Run(
ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled());
@@ -100,9 +121,21 @@ void InitiateMetricsReportingChange(
base::Bind(&SetMetricsReporting, enabled, callback_fn));
}
-bool IsMetricsReportingUserChangable() {
+bool IsMetricsReportingPolicyManaged() {
const PrefService* pref_service = g_browser_process->local_state();
const PrefService::Preference* pref =
pref_service->FindPreference(metrics::prefs::kMetricsReportingEnabled);
- return pref && !pref->IsManaged();
+ return pref && pref->IsManaged();
+}
+
+// TODO(gayane): Add unittest which will check that observer on device settings
+// will trigger this function and kMetricsReportinEnabled as well as metrics
+// service state will be updated accordingly.
+void SetupMetricsStateForChromeOS() {
+#if defined(OS_CHROMEOS)
+ chromeos::CrosSettings::Get()->AddSettingsObserver(
+ chromeos::kStatsReportingPref, base::Bind(&OnDeviceSettingChange));
+
+ OnDeviceSettingChange();
+#endif // defined(OS_CHROMEOS)
}
diff --git a/chrome/browser/metrics/metrics_reporting_state.h b/chrome/browser/metrics/metrics_reporting_state.h
index 4716929..d45fa5f 100644
--- a/chrome/browser/metrics/metrics_reporting_state.h
+++ b/chrome/browser/metrics/metrics_reporting_state.h
@@ -19,8 +19,12 @@ void InitiateMetricsReportingChange(
bool enabled,
const OnMetricsReportingCallbackType& callback_fn);
-// Returns whether MetricsReporting can be modified by the user (except CrOS and
+// Returns whether MetricsReporting can be modified by the user (except
// Android).
-bool IsMetricsReportingUserChangable();
+bool IsMetricsReportingPolicyManaged();
+
+// Initialize kMetricsReportingEnabled based on kStatsReportingPref device
+// setting and add an observer as it is the source of truth on Chrome OS.
+void SetupMetricsStateForChromeOS();
#endif // CHROME_BROWSER_METRICS_METRICS_REPORTING_STATE_H_
diff --git a/chrome/browser/policy/configuration_policy_handler_list_factory.cc b/chrome/browser/policy/configuration_policy_handler_list_factory.cc
index 6c097ad..6de0567 100644
--- a/chrome/browser/policy/configuration_policy_handler_list_factory.cc
+++ b/chrome/browser/policy/configuration_policy_handler_list_factory.cc
@@ -126,9 +126,6 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
{ key::kDefaultPrinterSelection,
prefs::kPrintPreviewDefaultDestinationSelectionRules,
base::Value::TYPE_STRING },
- { key::kMetricsReportingEnabled,
- metrics::prefs::kMetricsReportingEnabled,
- base::Value::TYPE_BOOLEAN },
{ key::kApplicationLocaleValue,
prefs::kApplicationLocale,
base::Value::TYPE_STRING },
@@ -493,6 +490,17 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
base::Value::TYPE_BOOLEAN },
#endif // defined(OS_CHROMEOS)
+// Metrics reporting is controlled by a platform specific policy for ChromeOS
+#if defined(OS_CHROMEOS)
+ { key::kDeviceMetricsReportingEnabled,
+ metrics::prefs::kMetricsReportingEnabled,
+ base::Value::TYPE_BOOLEAN },
+#else
+ { key::kMetricsReportingEnabled,
+ metrics::prefs::kMetricsReportingEnabled,
+ base::Value::TYPE_BOOLEAN },
+#endif
+
#if !defined(OS_MACOSX) && !defined(OS_CHROMEOS)
{ key::kBackgroundModeEnabled,
prefs::kBackgroundModeEnabled,
diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc
index 83fe26b..4a2bd2b 100644
--- a/chrome/browser/profiles/profile_io_data.cc
+++ b/chrome/browser/profiles/profile_io_data.cc
@@ -866,14 +866,7 @@ bool ProfileIOData::IsOffTheRecord() const {
void ProfileIOData::InitializeMetricsEnabledStateOnUIThread() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
-#if defined(OS_CHROMEOS)
- // Just fetch the value from ChromeOS' settings while we're on the UI thread.
- // TODO(stevet): For now, this value is only set on profile initialization.
- // We will want to do something similar to the PrefMember method below in the
- // future to more accurately capture this state.
- chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
- &enable_metrics_);
-#elif defined(OS_ANDROID)
+#if defined(OS_ANDROID)
// TODO(dwkang): rename or unify the pref for UMA once we have conclusion
// in crbugs.com/246495.
// Android has it's own preferences for metrics / crash uploading.
@@ -888,16 +881,12 @@ void ProfileIOData::InitializeMetricsEnabledStateOnUIThread() {
g_browser_process->local_state());
enable_metrics_.MoveToThread(
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
-#endif // defined(OS_CHROMEOS)
+#endif // defined(OS_ANDROID)
}
bool ProfileIOData::GetMetricsEnabledStateOnIOThread() const {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
-#if defined(OS_CHROMEOS)
- return enable_metrics_;
-#else
return enable_metrics_.GetValue();
-#endif // defined(OS_CHROMEOS)
}
bool ProfileIOData::IsDataReductionProxyEnabled() const {
@@ -1258,9 +1247,7 @@ void ProfileIOData::ShutdownOnUIThread(
enable_do_not_track_.Destroy();
force_google_safesearch_.Destroy();
force_youtube_safety_mode_.Destroy();
-#if !defined(OS_CHROMEOS)
enable_metrics_.Destroy();
-#endif
safe_browsing_enabled_.Destroy();
sync_disabled_.Destroy();
signin_allowed_.Destroy();
diff --git a/chrome/browser/profiles/profile_io_data.h b/chrome/browser/profiles/profile_io_data.h
index 350ba73..34a014e 100644
--- a/chrome/browser/profiles/profile_io_data.h
+++ b/chrome/browser/profiles/profile_io_data.h
@@ -523,14 +523,7 @@ class ProfileIOData {
mutable BooleanPrefMember quick_check_enabled_;
mutable IntegerPrefMember incognito_availibility_pref_;
- // The state of metrics reporting in the browser that this profile runs on.
- // Unfortunately, since ChromeOS has a separate representation of this state,
- // we need to make one available based on the platform.
-#if defined(OS_CHROMEOS)
- bool enable_metrics_;
-#else
BooleanPrefMember enable_metrics_;
-#endif
#if defined(ENABLE_CONFIGURATION_POLICY)
// Pointed to by NetworkDelegate.
diff --git a/chrome/browser/resources/options/browser_options.html b/chrome/browser/resources/options/browser_options.html
index 9733e77..51a79e6 100644
--- a/chrome/browser/resources/options/browser_options.html
+++ b/chrome/browser/resources/options/browser_options.html
@@ -425,18 +425,6 @@
<if expr="_google_chrome">
<div id="metrics-reporting-setting"
class="checkbox controlled-setting-with-label">
-<if expr="chromeos">
- <label>
- <input id="metrics-reporting-enabled"
- pref="cros.metrics.reportingEnabled" type="checkbox">
- <span>
- <span i18n-content="enableLogging"></span>
- <span class="controlled-setting-indicator"
- pref="cros.metrics.reportingEnabled"></span>
- </span>
- </label>
-</if>
-<if expr="not chromeos">
<label>
<input id="metrics-reporting-enabled" type="checkbox">
<span>
@@ -445,12 +433,13 @@
class="controlled-setting-indicator"></span>
</span>
</label>
+<if expr="not chromeos">
<span id="metrics-reporting-reset-restart" hidden>
<!-- Text filled by JavaScript -->
<span></span><a is="action-link" role="button"
class="standalone-action-link"></a><span></span>
</span>
-</if>
+</if> <!-- not chromeos -->
</div>
</if> <!-- _google_chrome -->
<div class="checkbox">
diff --git a/chrome/browser/resources/options/browser_options.js b/chrome/browser/resources/options/browser_options.js
index 36a1a66..4af8014 100644
--- a/chrome/browser/resources/options/browser_options.js
+++ b/chrome/browser/resources/options/browser_options.js
@@ -450,47 +450,53 @@ cr.define('options', function() {
chrome.send('coreOptionsUserMetricsAction', ['Options_ClearData']);
};
$('privacyClearDataButton').hidden = OptionsPage.isSettingsApp();
- // 'metricsReportingEnabled' element is only present on Chrome branded
- // builds, and the 'metricsReportingCheckboxAction' message is only
- // handled on ChromeOS.
- if ($('metrics-reporting-enabled') && cr.isChromeOS) {
- $('metrics-reporting-enabled').onclick = function(event) {
- chrome.send('metricsReportingCheckboxAction',
- [String(event.currentTarget.checked)]);
- };
- }
- if ($('metrics-reporting-enabled') && !cr.isChromeOS) {
- // The localized string has the | symbol on each side of the text that
- // needs to be made into a button to restart Chrome. We parse the text
- // and build the button from that.
- var restartTextFragments =
- loadTimeData.getString('metricsReportingResetRestart').split('|');
- // Assume structure is something like "starting text |link text| ending
- // text" where both starting text and ending text may or may not be
- // present, but the split should always be in three pieces.
- var restartElements =
- $('metrics-reporting-reset-restart').querySelectorAll('*');
- for (var i = 0; i < restartTextFragments.length; i++) {
- restartElements[i].textContent = restartTextFragments[i];
- }
- restartElements[1].onclick = function(event) {
- chrome.send('restartBrowser');
- };
+
+ if ($('metrics-reporting-enabled')) {
+ $('metrics-reporting-enabled').checked =
+ loadTimeData.getBoolean('metricsReportingEnabledAtStart');
+
+ // A browser restart is never needed to toggle metrics reporting,
+ // and is only needed to toggle crash reporting when using Breakpad.
+ // Crashpad, used on Mac, does not require a browser restart.
+ var togglingMetricsRequiresRestart = !cr.isMac && !cr.isChromeOS;
$('metrics-reporting-enabled').onclick = function(event) {
chrome.send('metricsReportingCheckboxChanged',
[Boolean(event.currentTarget.checked)]);
- if (cr.isMac) {
- // A browser restart is never needed to toggle metrics reporting,
- // and is only needed to toggle crash reporting when using Breakpad.
- // Crashpad, used on Mac, does not require a browser restart.
- return;
+ if (cr.isChromeOS) {
+ // 'metricsReportingEnabled' element is only present on Chrome
+ // branded builds, and the 'metricsReportingCheckboxAction' message
+ // is only handled on ChromeOS.
+ chrome.send('metricsReportingCheckboxAction',
+ [String(event.currentTarget.checked)]);
}
- $('metrics-reporting-reset-restart').hidden =
- loadTimeData.getBoolean('metricsReportingEnabledAtStart') ==
- $('metrics-reporting-enabled').checked;
+
+ if (togglingMetricsRequiresRestart) {
+ $('metrics-reporting-reset-restart').hidden =
+ loadTimeData.getBoolean('metricsReportingEnabledAtStart') ==
+ $('metrics-reporting-enabled').checked;
+ }
+
};
- $('metrics-reporting-enabled').checked =
- loadTimeData.getBoolean('metricsReportingEnabledAtStart');
+
+ // Initialize restart button if needed.
+ if (togglingMetricsRequiresRestart) {
+ // The localized string has the | symbol on each side of the text that
+ // needs to be made into a button to restart Chrome. We parse the text
+ // and build the button from that.
+ var restartTextFragments =
+ loadTimeData.getString('metricsReportingResetRestart').split('|');
+ // Assume structure is something like "starting text |link text|
+ // ending text" where both starting text and ending text may or may
+ // not be present, but the split should always be in three pieces.
+ var restartElements =
+ $('metrics-reporting-reset-restart').querySelectorAll('*');
+ for (var i = 0; i < restartTextFragments.length; i++) {
+ restartElements[i].textContent = restartTextFragments[i];
+ }
+ restartElements[1].onclick = function(event) {
+ chrome.send('restartBrowser');
+ };
+ }
}
$('networkPredictionOptions').onchange = function(event) {
var value = (event.target.checked ?
@@ -1730,18 +1736,24 @@ cr.define('options', function() {
* Set the checked state of the metrics reporting checkbox.
* @private
*/
- setMetricsReportingCheckboxState_: function(checked, disabled) {
+ setMetricsReportingCheckboxState_: function(checked,
+ policyManaged,
+ ownerManaged) {
$('metrics-reporting-enabled').checked = checked;
- $('metrics-reporting-enabled').disabled = disabled;
+ $('metrics-reporting-enabled').disabled = policyManaged || ownerManaged;
// If checkbox gets disabled then add an attribute for displaying the
// special icon. Otherwise remove the indicator attribute.
- if (disabled) {
+ if (policyManaged) {
$('metrics-reporting-disabled-icon').setAttribute('controlled-by',
'policy');
+ } else if (ownerManaged) {
+ $('metrics-reporting-disabled-icon').setAttribute('controlled-by',
+ 'owner');
} else {
$('metrics-reporting-disabled-icon').removeAttribute('controlled-by');
}
+
},
/**
diff --git a/chrome/browser/ui/webui/options/browser_options_handler.cc b/chrome/browser/ui/webui/options/browser_options_handler.cc
index b8159eb..19d3233 100644
--- a/chrome/browser/ui/webui/options/browser_options_handler.cc
+++ b/chrome/browser/ui/webui/options/browser_options_handler.cc
@@ -569,11 +569,9 @@ void BrowserOptionsHandler::GetLocalizedValues(base::DictionaryValue* values) {
values->SetString("doNotTrackLearnMoreURL", chrome::kDoNotTrackLearnMoreURL);
-#if !defined(OS_CHROMEOS)
values->SetBoolean(
"metricsReportingEnabledAtStart",
ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled());
-#endif
#if defined(OS_CHROMEOS)
// TODO(pastarmovj): replace this with a call to the CrosSettings list
@@ -2076,14 +2074,18 @@ void BrowserOptionsHandler::SetupExtensionControlledIndicators() {
}
void BrowserOptionsHandler::SetupMetricsReportingCheckbox() {
- // This function does not work for ChromeOS and non-official builds.
-#if !defined(OS_CHROMEOS) && defined(GOOGLE_CHROME_BUILD)
+// As the metrics and crash reporting checkbox only exists for official builds
+// it doesn't need to be set up for non-official builds.
+#if defined(GOOGLE_CHROME_BUILD)
bool checked =
ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled();
- bool disabled = !IsMetricsReportingUserChangable();
-
- SetMetricsReportingCheckbox(checked, disabled);
-#endif
+ bool policy_managed = IsMetricsReportingPolicyManaged();
+ bool owner_managed = false;
+#if defined(OS_CHROMEOS)
+ owner_managed = !IsDeviceOwnerProfile();
+#endif // defined(OS_CHROMEOS)
+ SetMetricsReportingCheckbox(checked, policy_managed, owner_managed);
+#endif // defined(GOOGLE_CHROME_BUILD)
}
void BrowserOptionsHandler::HandleMetricsReportingChange(
@@ -2091,23 +2093,43 @@ void BrowserOptionsHandler::HandleMetricsReportingChange(
bool enable;
if (!args->GetBoolean(0, &enable))
return;
+ // Decline the change if current user shouldn't be able to change metrics
+ // reporting.
+ if (!IsDeviceOwnerProfile() || IsMetricsReportingPolicyManaged()) {
+ NotifyUIOfMetricsReportingChange(
+ ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled());
+ return;
+ }
+// For Chrome OS updating device settings will notify an observer to update
+// metrics pref, however we still need to call |InitiateMetricsReportingChange|
+// with a proper callback so that UI gets updated in case of failure to update
+// the metrics pref.
+// TODO(gayane): Don't call |InitiateMetricsReportingChange| twice so that
+// metrics service pref changes only as a result of device settings change for
+// Chrome OS .crbug.com/552550.
+#if defined(OS_CHROMEOS)
+ chromeos::CrosSettings::Get()->SetBoolean(chromeos::kStatsReportingPref,
+ enable);
+#endif // defined(OS_CHROMEOS)
InitiateMetricsReportingChange(
enable,
- base::Bind(&BrowserOptionsHandler::MetricsReportingChangeCallback,
+ base::Bind(&BrowserOptionsHandler::NotifyUIOfMetricsReportingChange,
base::Unretained(this)));
}
-void BrowserOptionsHandler::MetricsReportingChangeCallback(bool enabled) {
- SetMetricsReportingCheckbox(enabled, !IsMetricsReportingUserChangable());
+void BrowserOptionsHandler::NotifyUIOfMetricsReportingChange(bool enabled) {
+ SetMetricsReportingCheckbox(enabled, IsMetricsReportingPolicyManaged(),
+ !IsDeviceOwnerProfile());
}
void BrowserOptionsHandler::SetMetricsReportingCheckbox(bool checked,
- bool disabled) {
+ bool policy_managed,
+ bool owner_managed) {
web_ui()->CallJavascriptFunction(
"BrowserOptions.setMetricsReportingCheckboxState",
- base::FundamentalValue(checked),
- base::FundamentalValue(disabled));
+ base::FundamentalValue(checked), base::FundamentalValue(policy_managed),
+ base::FundamentalValue(owner_managed));
}
void BrowserOptionsHandler::OnPolicyUpdated(const policy::PolicyNamespace& ns,
@@ -2119,4 +2141,12 @@ void BrowserOptionsHandler::OnPolicyUpdated(const policy::PolicyNamespace& ns,
SetupMetricsReportingCheckbox();
}
+bool BrowserOptionsHandler::IsDeviceOwnerProfile() {
+#if defined(OS_CHROMEOS)
+ return chromeos::ProfileHelper::IsOwnerProfile(Profile::FromWebUI(web_ui()));
+#else
+ return true;
+#endif
+}
+
} // namespace options
diff --git a/chrome/browser/ui/webui/options/browser_options_handler.h b/chrome/browser/ui/webui/options/browser_options_handler.h
index 2c02bc47..3644e9e 100644
--- a/chrome/browser/ui/webui/options/browser_options_handler.h
+++ b/chrome/browser/ui/webui/options/browser_options_handler.h
@@ -351,7 +351,7 @@ class BrowserOptionsHandler
void SetupExtensionControlledIndicators();
// Setup the value and the disabled property for metrics reporting for (except
- // CrOS and Android).
+ // Android).
void SetupMetricsReportingCheckbox();
// Called when the MetricsReportingEnabled checkbox values are changed.
@@ -359,10 +359,12 @@ class BrowserOptionsHandler
void HandleMetricsReportingChange(const base::ListValue* args);
// Notifies the result of MetricsReportingEnabled change to Javascript layer.
- void MetricsReportingChangeCallback(bool enabled);
+ void NotifyUIOfMetricsReportingChange(bool enabled);
// Calls a Javascript function to set the state of MetricsReporting checkbox.
- void SetMetricsReportingCheckbox(bool checked, bool disabled);
+ void SetMetricsReportingCheckbox(bool checked,
+ bool policy_managed,
+ bool owner_managed);
#if defined(OS_CHROMEOS)
// Setup the accessibility features for ChromeOS.
@@ -373,6 +375,10 @@ class BrowserOptionsHandler
// correspond to the status of sync.
scoped_ptr<base::DictionaryValue> GetSyncStateDictionary();
+ // Checks whether on Chrome OS the current user is the device owner. Returns
+ // true on other platforms.
+ bool IsDeviceOwnerProfile();
+
scoped_refptr<ShellIntegration::DefaultBrowserWorker> default_browser_worker_;
bool page_initialized_;