From 3ec50e5aef983bb135d635257150dbe2ad7b267b Mon Sep 17 00:00:00 2001 From: holte Date: Fri, 3 Apr 2015 12:31:36 -0700 Subject: Remove code for Rappor option from the settings page. This removes the code the Rappor option on the settings page, which was disabled by field trial. See the original changes: https://codereview.chromium.org/886523003/ https://codereview.chromium.org/932313002 BUG=455898 Review URL: https://codereview.chromium.org/1027033002 Cr-Commit-Position: refs/heads/master@{#323795} --- chrome/app/generated_resources.grd | 3 - chrome/browser/metrics/metrics_reporting_state.cc | 4 - chrome/browser/metrics/metrics_reporting_state.h | 3 - chrome/browser/metrics/metrics_services_manager.cc | 68 +--------- chrome/browser/metrics/metrics_services_manager.h | 7 - .../metrics/metrics_services_manager_unittest.cc | 146 --------------------- .../configuration_policy_handler_list_factory.cc | 4 - .../browser/resources/options/browser_options.html | 11 -- .../browser/resources/options/browser_options.js | 5 - .../ui/webui/options/browser_options_handler.cc | 5 - chrome/chrome_tests_unit.gypi | 1 - chrome/common/url_constants.cc | 7 - chrome/common/url_constants.h | 3 - components/policy/resources/policy_templates.json | 10 +- components/rappor/rappor_pref_names.cc | 3 - components/rappor/rappor_pref_names.h | 1 - components/rappor/rappor_prefs.cc | 1 - tools/metrics/actions/actions.xml | 10 -- 18 files changed, 11 insertions(+), 281 deletions(-) delete mode 100644 chrome/browser/metrics/metrics_services_manager_unittest.cc diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index 1c888e7..9c1d85b 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -12413,9 +12413,6 @@ The following application will be launched if you accept this request: Automatically send usage statistics and crash reports to Google - - Send <a target="_blank" href="$1">RAPPOR</a></a> statistics to Google - (requires Chrome |restart|) diff --git a/chrome/browser/metrics/metrics_reporting_state.cc b/chrome/browser/metrics/metrics_reporting_state.cc index 81e4974..a4a61fe 100644 --- a/chrome/browser/metrics/metrics_reporting_state.cc +++ b/chrome/browser/metrics/metrics_reporting_state.cc @@ -80,10 +80,6 @@ void SetMetricsReporting(bool to_update_pref, } // namespace -bool HasRapporOption() { - return base::FieldTrialList::FindFullName("RapporOption") == "Enabled"; -} - void InitiateMetricsReportingChange( bool enabled, const OnMetricsReportingCallbackType& callback_fn) { diff --git a/chrome/browser/metrics/metrics_reporting_state.h b/chrome/browser/metrics/metrics_reporting_state.h index 5d6ff4c..4716929 100644 --- a/chrome/browser/metrics/metrics_reporting_state.h +++ b/chrome/browser/metrics/metrics_reporting_state.h @@ -9,9 +9,6 @@ typedef base::Callback OnMetricsReportingCallbackType; -// Returns true if RAPPOR is controlled by a seperate option. -bool HasRapporOption(); - // Initiates a change to metrics reporting state to the new value of |enabled|. // Starts or stops the metrics service based on the new state and then runs // |callback_fn| (which can be null) with the updated state (as the operation diff --git a/chrome/browser/metrics/metrics_services_manager.cc b/chrome/browser/metrics/metrics_services_manager.cc index 9f55dae..602e34c 100644 --- a/chrome/browser/metrics/metrics_services_manager.cc +++ b/chrome/browser/metrics/metrics_services_manager.cc @@ -14,15 +14,12 @@ #include "chrome/browser/metrics/chrome_metrics_service_client.h" #include "chrome/browser/metrics/metrics_reporting_state.h" #include "chrome/browser/metrics/variations/variations_service.h" -#include "chrome/browser/profiles/profile.h" -#include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/ui/browser_otr_state.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/pref_names.h" #include "chrome/installer/util/google_update_settings.h" #include "components/metrics/metrics_service.h" #include "components/metrics/metrics_state_manager.h" -#include "components/rappor/rappor_pref_names.h" #include "components/rappor/rappor_service.h" #include "content/public/browser/browser_thread.h" @@ -30,24 +27,6 @@ #include "chrome/browser/chromeos/settings/cros_settings.h" #endif -namespace { - -// Check if the safe browsing setting is enabled for all existing profiles. -bool CheckSafeBrowsingSettings() { - bool all_enabled = true; - ProfileManager* profile_manager = g_browser_process->profile_manager(); - if (profile_manager) { - std::vector profiles = profile_manager->GetLoadedProfiles(); - for (Profile* profile : profiles) { - if (profile->IsOffTheRecord()) - continue; - all_enabled = all_enabled && profile->GetPrefs()->GetBoolean( - prefs::kSafeBrowsingEnabled); - } - } - return all_enabled; -} - // Posts |GoogleUpdateSettings::StoreMetricsClientInfo| on blocking pool thread // because it needs access to IO and cannot work from UI thread. void PostStoreMetricsClientInfo(const metrics::ClientInfo& client_info) { @@ -55,8 +34,6 @@ void PostStoreMetricsClientInfo(const metrics::ClientInfo& client_info) { base::Bind(&GoogleUpdateSettings::StoreMetricsClientInfo, client_info)); } -} // namespace - MetricsServicesManager::MetricsServicesManager(PrefService* local_state) : local_state_(local_state), may_upload_(false), @@ -121,44 +98,6 @@ metrics::MetricsStateManager* MetricsServicesManager::GetMetricsStateManager() { return metrics_state_manager_.get(); } -bool MetricsServicesManager::IsRapporEnabled(bool metrics_enabled) const { - if (!local_state_->HasPrefPath(rappor::prefs::kRapporEnabled)) { - // For upgrading users, derive an initial setting from UMA / safe browsing. - bool should_enable = metrics_enabled || CheckSafeBrowsingSettings(); - local_state_->SetBoolean(rappor::prefs::kRapporEnabled, should_enable); - } - return local_state_->GetBoolean(rappor::prefs::kRapporEnabled); -} - -rappor::RecordingLevel MetricsServicesManager::GetRapporRecordingLevel( - bool metrics_enabled) const { - rappor::RecordingLevel recording_level = rappor::RECORDING_DISABLED; -#if defined(GOOGLE_CHROME_BUILD) - if (HasRapporOption()) { - if (IsRapporEnabled(metrics_enabled)) { - recording_level = metrics_enabled ? - rappor::FINE_LEVEL : - rappor::COARSE_LEVEL; - } - } else if (metrics_enabled) { - recording_level = rappor::FINE_LEVEL; - } -#endif // defined(GOOGLE_CHROME_BUILD) - return recording_level; -} - -void MetricsServicesManager::UpdateRapporService() { - GetRapporService()->Update(GetRapporRecordingLevel(may_record_), may_upload_); - // The first time this function is called, we can start listening for - // changes to RAPPOR option. This avoids starting the RapporService in - // tests which do not intend to start services. - if (pref_change_registrar_.IsEmpty() && HasRapporOption()) { - pref_change_registrar_.Add(rappor::prefs::kRapporEnabled, - base::Bind(&MetricsServicesManager::UpdateRapporService, - base::Unretained(this))); - } -} - void MetricsServicesManager::UpdatePermissions(bool may_record, bool may_upload) { // Stash the current permissions so that we can update the RapporService @@ -194,7 +133,12 @@ void MetricsServicesManager::UpdatePermissions(bool may_record, metrics->Stop(); } - UpdateRapporService(); + rappor::RecordingLevel recording_level = rappor::RECORDING_DISABLED; +#if defined(GOOGLE_CHROME_BUILD) + if (may_record) + recording_level = rappor::FINE_LEVEL +#endif // defined(GOOGLE_CHROME_BUILD) + GetRapporService()->Update(recording_level, may_upload); } void MetricsServicesManager::UpdateUploadPermissions(bool may_upload) { diff --git a/chrome/browser/metrics/metrics_services_manager.h b/chrome/browser/metrics/metrics_services_manager.h index 72ec3f7..7a05c80 100644 --- a/chrome/browser/metrics/metrics_services_manager.h +++ b/chrome/browser/metrics/metrics_services_manager.h @@ -9,7 +9,6 @@ #include "base/memory/scoped_ptr.h" #include "base/prefs/pref_change_registrar.h" #include "base/threading/thread_checker.h" -#include "components/rappor/rappor_service.h" class ChromeMetricsServiceClient; class PrefService; @@ -60,12 +59,6 @@ class MetricsServicesManager { // Update the managed services when permissions for uploading metrics change. void UpdateUploadPermissions(bool may_upload); - // Returns true iff Rappor reporting is enabled. - bool IsRapporEnabled(bool metrics_enabled) const; - - // Returns the recording level for Rappor metrics. - rappor::RecordingLevel GetRapporRecordingLevel(bool metrics_enabled) const; - private: // Update the managed services when permissions for recording/uploading // metrics change. diff --git a/chrome/browser/metrics/metrics_services_manager_unittest.cc b/chrome/browser/metrics/metrics_services_manager_unittest.cc deleted file mode 100644 index a930c05..0000000 --- a/chrome/browser/metrics/metrics_services_manager_unittest.cc +++ /dev/null @@ -1,146 +0,0 @@ -// Copyright 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chrome/browser/metrics/metrics_services_manager.h" - -#include - -#include "base/metrics/field_trial.h" -#include "base/prefs/testing_pref_service.h" -#include "chrome/common/pref_names.h" -#include "chrome/test/base/testing_browser_process.h" -#include "chrome/test/base/testing_profile_manager.h" -#include "components/rappor/rappor_pref_names.h" -#include "components/rappor/rappor_prefs.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace { - -#if defined(GOOGLE_CHROME_BUILD) - -void UseRapporOption() { - ASSERT_TRUE(base::FieldTrialList::CreateFieldTrial( - "RapporOption", "Enabled")); -} - -#endif // defined(GOOGLE_CHROME_BUILD) - -} // namespace - -class MetricsServicesManagerTest : public testing::Test { - public: - MetricsServicesManagerTest() - : test_profile_manager_(TestingBrowserProcess::GetGlobal()), - manager_(&test_prefs_), - field_trial_list_(NULL) { - rappor::internal::RegisterPrefs(test_prefs_.registry()); - } - - void SetUp() override { - ASSERT_TRUE(test_profile_manager_.SetUp()); - } - - void CreateProfile(const std::string& name, bool sb_enabled) { - TestingProfile* profile = test_profile_manager_.CreateTestingProfile(name); - profile->GetPrefs()->SetBoolean(prefs::kSafeBrowsingEnabled, sb_enabled); - } - - TestingPrefServiceSimple* test_prefs() { return &test_prefs_; } - - MetricsServicesManager* manager() { return &manager_; } - - private: - TestingProfileManager test_profile_manager_; - TestingPrefServiceSimple test_prefs_; - MetricsServicesManager manager_; - base::FieldTrialList field_trial_list_; - - DISALLOW_COPY_AND_ASSIGN(MetricsServicesManagerTest); -}; - -TEST_F(MetricsServicesManagerTest, CheckRapporDefaultDisable) { - test_prefs()->ClearPref(rappor::prefs::kRapporEnabled); - CreateProfile("profile1", true); - CreateProfile("profile2", false); - bool uma_enabled = false; - - EXPECT_FALSE(manager()->IsRapporEnabled(uma_enabled)); - EXPECT_TRUE(test_prefs()->HasPrefPath(rappor::prefs::kRapporEnabled)); - EXPECT_FALSE(test_prefs()->GetBoolean(rappor::prefs::kRapporEnabled)); -} - -TEST_F(MetricsServicesManagerTest, CheckRapporDefaultEnabledBySafeBrowsing) { - test_prefs()->ClearPref(rappor::prefs::kRapporEnabled); - CreateProfile("profile1", true); - CreateProfile("profile2", true); - bool uma_enabled = false; - - EXPECT_TRUE(manager()->IsRapporEnabled(uma_enabled)); - EXPECT_TRUE(test_prefs()->HasPrefPath(rappor::prefs::kRapporEnabled)); - EXPECT_TRUE(test_prefs()->GetBoolean(rappor::prefs::kRapporEnabled)); -} - -TEST_F(MetricsServicesManagerTest, CheckRapporDefaultEnabledByUMA) { - test_prefs()->ClearPref(rappor::prefs::kRapporEnabled); - CreateProfile("profile1", false); - CreateProfile("profile2", false); - bool uma_enabled = true; - - EXPECT_TRUE(manager()->IsRapporEnabled(uma_enabled)); - EXPECT_TRUE(test_prefs()->HasPrefPath(rappor::prefs::kRapporEnabled)); - EXPECT_TRUE(test_prefs()->GetBoolean(rappor::prefs::kRapporEnabled)); -} - -TEST_F(MetricsServicesManagerTest, CheckRapporEnable) { - test_prefs()->SetBoolean(rappor::prefs::kRapporEnabled, true); - CreateProfile("profile1", false); - CreateProfile("profile2", false); - bool uma_enabled = false; - - EXPECT_TRUE(manager()->IsRapporEnabled(uma_enabled)); - EXPECT_TRUE(test_prefs()->HasPrefPath(rappor::prefs::kRapporEnabled)); - EXPECT_TRUE(test_prefs()->GetBoolean(rappor::prefs::kRapporEnabled)); -} - -TEST_F(MetricsServicesManagerTest, CheckRapporDisable) { - test_prefs()->SetBoolean(rappor::prefs::kRapporEnabled, false); - CreateProfile("profile1", true); - CreateProfile("profile2", true); - bool uma_enabled = true; - - EXPECT_FALSE(manager()->IsRapporEnabled(uma_enabled)); - EXPECT_TRUE(test_prefs()->HasPrefPath(rappor::prefs::kRapporEnabled)); - EXPECT_FALSE(test_prefs()->GetBoolean(rappor::prefs::kRapporEnabled)); -} - -#if defined(GOOGLE_CHROME_BUILD) - -TEST_F(MetricsServicesManagerTest, GetRecordingLevelDisabled) { - UseRapporOption(); - test_prefs()->SetBoolean(rappor::prefs::kRapporEnabled, false); - bool uma_enabled = true; - - EXPECT_EQ(rappor::RECORDING_DISABLED, - manager()->GetRapporRecordingLevel(uma_enabled)); -} - -TEST_F(MetricsServicesManagerTest, GetRecordingLevelFine) { - UseRapporOption(); - test_prefs()->SetBoolean(rappor::prefs::kRapporEnabled, true); - bool uma_enabled = true; - - EXPECT_EQ(rappor::FINE_LEVEL, - manager()->GetRapporRecordingLevel(uma_enabled)); -} - -TEST_F(MetricsServicesManagerTest, GetRecordingLevelCoarse) { - UseRapporOption(); - test_prefs()->SetBoolean(rappor::prefs::kRapporEnabled, true); - bool uma_enabled = false; - - EXPECT_EQ(rappor::COARSE_LEVEL, - manager()->GetRapporRecordingLevel(uma_enabled)); -} - -#endif // defined(GOOGLE_CHROME_BUILD) diff --git a/chrome/browser/policy/configuration_policy_handler_list_factory.cc b/chrome/browser/policy/configuration_policy_handler_list_factory.cc index f046efc..2b60aeb 100644 --- a/chrome/browser/policy/configuration_policy_handler_list_factory.cc +++ b/chrome/browser/policy/configuration_policy_handler_list_factory.cc @@ -23,7 +23,6 @@ #include "components/policy/core/common/policy_map.h" #include "components/policy/core/common/policy_pref_names.h" #include "components/policy/core/common/schema.h" -#include "components/rappor/rappor_pref_names.h" #include "components/search_engines/default_search_policy_handler.h" #include "components/translate/core/common/translate_pref_names.h" #include "policy/policy_constants.h" @@ -120,9 +119,6 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = { { key::kMetricsReportingEnabled, prefs::kMetricsReportingEnabled, base::Value::TYPE_BOOLEAN }, - { key::kMetricsReportingEnabled, - rappor::prefs::kRapporEnabled, - base::Value::TYPE_BOOLEAN }, { key::kApplicationLocaleValue, prefs::kApplicationLocale, base::Value::TYPE_STRING }, diff --git a/chrome/browser/resources/options/browser_options.html b/chrome/browser/resources/options/browser_options.html index 9e14317..5aa5e0f 100644 --- a/chrome/browser/resources/options/browser_options.html +++ b/chrome/browser/resources/options/browser_options.html @@ -460,17 +460,6 @@ -
- -