Commit 40dd030f authored by anthonyvd's avatar anthonyvd Committed by Commit bot

Fix ChromeMetricsServiceAccessor::IsMetricsReportingEnabled on CrOs

Since MetricsServicesManager::IsMetricsReportingEnabled is correct, it is
called directly.

BUG=457430

Review URL: https://codereview.chromium.org/916133003

Cr-Commit-Position: refs/heads/master@{#317058}
parent 4ff204b8
......@@ -676,11 +676,7 @@ void ChromeBrowserMainParts::StartMetricsRecording() {
g_browser_process->metrics_service()->CheckForClonedInstall(
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
bool may_record = g_browser_process->GetMetricsServicesManager()->
IsMetricsReportingEnabled();
g_browser_process->GetMetricsServicesManager()->UpdatePermissions(
may_record, true);
g_browser_process->GetMetricsServicesManager()->UpdateUploadPermissions(true);
}
void ChromeBrowserMainParts::RecordBrowserStartupTime() {
......
......@@ -4,8 +4,11 @@
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "base/command_line.h"
#include "base/prefs/pref_service.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/metrics/metrics_services_manager.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "components/metrics/metrics_service.h"
#include "components/variations/metrics_util.h"
......@@ -15,28 +18,33 @@
#endif
// static
// TODO(asvitkine): This function does not report the correct value on Android,
// see http://crbug.com/362192.
bool ChromeMetricsServiceAccessor::IsMetricsReportingEnabled() {
bool result = false;
const PrefService* local_state = g_browser_process->local_state();
if (local_state) {
const PrefService::Preference* uma_pref =
local_state->FindPreference(prefs::kMetricsReportingEnabled);
if (uma_pref) {
bool success = uma_pref->GetValue()->GetAsBoolean(&result);
DCHECK(success);
}
// If the user permits metrics reporting with the checkbox in the
// prefs, we turn on recording. We disable metrics completely for
// non-official builds, or when field trials are forced.
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceFieldTrials)) {
return false;
}
return result;
}
bool ChromeMetricsServiceAccessor::IsCrashReportingEnabled() {
bool enabled = false;
#if defined(GOOGLE_CHROME_BUILD)
#if defined(OS_CHROMEOS)
bool reporting_enabled = false;
chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
&reporting_enabled);
return reporting_enabled;
#elif defined(OS_ANDROID)
&enabled);
#else
enabled = g_browser_process->local_state()->
GetBoolean(prefs::kMetricsReportingEnabled);
#endif // #if defined(OS_CHROMEOS)
#endif // defined(GOOGLE_CHROME_BUILD)
return enabled;
}
bool ChromeMetricsServiceAccessor::IsCrashReportingEnabled() {
#if defined(GOOGLE_CHROME_BUILD)
#if defined(OS_ANDROID)
// Android has its own settings for metrics / crash uploading.
const PrefService* prefs = g_browser_process->local_state();
return prefs->GetBoolean(prefs::kCrashReportingEnabled);
......
......@@ -66,6 +66,7 @@ class ChromeMetricsServiceAccessor : public metrics::MetricsServiceAccessor {
friend class options::BrowserOptionsHandler;
friend void InitiateMetricsReportingChange(
bool, const OnMetricsReportingCallbackType&);
friend class MetricsServicesManager;
FRIEND_TEST_ALL_PREFIXES(ChromeMetricsServiceAccessorTest,
MetricsReportingEnabled);
......
......@@ -26,6 +26,7 @@ class ChromeMetricsServiceAccessorTest : public testing::Test {
};
TEST_F(ChromeMetricsServiceAccessorTest, MetricsReportingEnabled) {
#if defined(GOOGLE_CHROME_BUILD)
#if !defined(OS_CHROMEOS)
GetLocalState()->SetBoolean(prefs::kMetricsReportingEnabled, false);
EXPECT_FALSE(ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
......@@ -38,6 +39,10 @@ TEST_F(ChromeMetricsServiceAccessorTest, MetricsReportingEnabled) {
// device settings for metrics reporting.
EXPECT_FALSE(ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
#endif
#else
// Metrics Reporting is never enabled when GOOGLE_CHROME_BUILD is undefined.
EXPECT_FALSE(ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
#endif
}
TEST_F(ChromeMetricsServiceAccessorTest, CrashReportingEnabled) {
......
......@@ -10,6 +10,7 @@
#include "base/logging.h"
#include "base/prefs/pref_service.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
#include "chrome/browser/metrics/chrome_metrics_service_client.h"
#include "chrome/browser/metrics/variations/variations_service.h"
#include "chrome/browser/profiles/profile.h"
......@@ -109,8 +110,7 @@ metrics::MetricsStateManager* MetricsServicesManager::GetMetricsStateManager() {
if (!metrics_state_manager_) {
metrics_state_manager_ = metrics::MetricsStateManager::Create(
local_state_,
base::Bind(&MetricsServicesManager::IsMetricsReportingEnabled,
base::Unretained(this)),
base::Bind(&ChromeMetricsServiceAccessor::IsMetricsReportingEnabled),
base::Bind(&PostStoreMetricsClientInfo),
base::Bind(&GoogleUpdateSettings::LoadMetricsClientInfo));
}
......@@ -178,24 +178,7 @@ void MetricsServicesManager::UpdatePermissions(bool may_record,
GetRapporService()->Update(GetRapporRecordingLevel(may_record), may_upload);
}
// TODO(asvitkine): This function does not report the correct value on Android,
// see http://crbug.com/362192.
bool MetricsServicesManager::IsMetricsReportingEnabled() const {
// If the user permits metrics reporting with the checkbox in the
// prefs, we turn on recording. We disable metrics completely for
// non-official builds, or when field trials are forced.
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kForceFieldTrials))
return false;
bool enabled = false;
#if defined(GOOGLE_CHROME_BUILD)
#if defined(OS_CHROMEOS)
chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
&enabled);
#else
enabled = local_state_->GetBoolean(prefs::kMetricsReportingEnabled);
#endif // #if defined(OS_CHROMEOS)
#endif // defined(GOOGLE_CHROME_BUILD)
return enabled;
void MetricsServicesManager::UpdateUploadPermissions(bool may_upload) {
return UpdatePermissions(
ChromeMetricsServiceAccessor::IsMetricsReportingEnabled(), may_upload);
}
......@@ -56,8 +56,8 @@ class MetricsServicesManager {
// metrics change.
void UpdatePermissions(bool may_record, bool may_upload);
// Returns true iff metrics reporting is enabled.
bool IsMetricsReportingEnabled() const;
// 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;
......
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