Commit 61b0d48b authored by blundell@chromium.org's avatar blundell@chromium.org

Remove ClonedInstallDetector/MachineIdProvider //content dependencies

ClonedInstallDetector now takes in a base::SingleThreadTaskRunner rather than
directly using content::BrowserThread::File. MachineIdProvider asserts that the
task runner on which it's operating has IO allowed rather than checking that
it's on the FILE thread.

BUG=374213
TBR=jochen

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@271637 0039d316-1c4b-4281-b951-d872f2087c98
parent 79686c5b
...@@ -629,7 +629,8 @@ void ChromeBrowserMainParts::StartMetricsRecording() { ...@@ -629,7 +629,8 @@ void ChromeBrowserMainParts::StartMetricsRecording() {
return; return;
} }
metrics->CheckForClonedInstall(); metrics->CheckForClonedInstall(
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
const bool metrics_enabled = metrics->StartIfMetricsReportingEnabled(); const bool metrics_enabled = metrics->StartIfMetricsReportingEnabled();
if (metrics_enabled) { if (metrics_enabled) {
// TODO(asvitkine): Since this function is not run on Android, RAPPOR is // TODO(asvitkine): Since this function is not run on Android, RAPPOR is
......
...@@ -5,14 +5,16 @@ ...@@ -5,14 +5,16 @@
#include "chrome/browser/metrics/cloned_install_detector.h" #include "chrome/browser/metrics/cloned_install_detector.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/location.h"
#include "base/metrics/histogram.h" #include "base/metrics/histogram.h"
#include "base/prefs/pref_registry_simple.h" #include "base/prefs/pref_registry_simple.h"
#include "base/prefs/pref_service.h" #include "base/prefs/pref_service.h"
#include "base/single_thread_task_runner.h"
#include "base/task_runner_util.h"
#include "chrome/browser/metrics/cloned_install_detector.h" #include "chrome/browser/metrics/cloned_install_detector.h"
#include "chrome/browser/metrics/machine_id_provider.h" #include "chrome/browser/metrics/machine_id_provider.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "components/metrics/metrics_hashes.h" #include "components/metrics/metrics_hashes.h"
#include "content/public/browser/browser_thread.h"
namespace metrics { namespace metrics {
...@@ -48,12 +50,13 @@ ClonedInstallDetector::ClonedInstallDetector(MachineIdProvider* raw_id_provider) ...@@ -48,12 +50,13 @@ ClonedInstallDetector::ClonedInstallDetector(MachineIdProvider* raw_id_provider)
ClonedInstallDetector::~ClonedInstallDetector() {} ClonedInstallDetector::~ClonedInstallDetector() {}
void ClonedInstallDetector::CheckForClonedInstall(PrefService* local_state) { void ClonedInstallDetector::CheckForClonedInstall(
content::BrowserThread::PostTaskAndReplyWithResult( PrefService* local_state,
content::BrowserThread::FILE, scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
base::PostTaskAndReplyWithResult(
task_runner.get(),
FROM_HERE, FROM_HERE,
base::Bind(&metrics::MachineIdProvider::GetMachineId, base::Bind(&metrics::MachineIdProvider::GetMachineId, raw_id_provider_),
raw_id_provider_),
base::Bind(&metrics::ClonedInstallDetector::SaveMachineId, base::Bind(&metrics::ClonedInstallDetector::SaveMachineId,
weak_ptr_factory_.GetWeakPtr(), weak_ptr_factory_.GetWeakPtr(),
local_state)); local_state));
......
...@@ -12,6 +12,10 @@ ...@@ -12,6 +12,10 @@
class PrefRegistrySimple; class PrefRegistrySimple;
class PrefService; class PrefService;
namespace base {
class SingleThreadTaskRunner;
}
namespace metrics { namespace metrics {
class MachineIdProvider; class MachineIdProvider;
...@@ -23,12 +27,15 @@ class ClonedInstallDetector { ...@@ -23,12 +27,15 @@ class ClonedInstallDetector {
explicit ClonedInstallDetector(MachineIdProvider* raw_id_provider); explicit ClonedInstallDetector(MachineIdProvider* raw_id_provider);
virtual ~ClonedInstallDetector(); virtual ~ClonedInstallDetector();
// Posts a task to generate a machine ID and store it to a local state pref. // Posts a task to |task_runner| to generate a machine ID and store it to a
// If the newly generated ID is different than the previously stored one, then // local state pref. If the newly generated ID is different than the
// the install is considered cloned. The ID is a 24-bit value based off of // previously stored one, then the install is considered cloned. The ID is a
// machine characteristics. This value should never be sent over the network. // 24-bit value based off of machine characteristics. This value should never
// be sent over the network.
// TODO(jwd): Implement change detection. // TODO(jwd): Implement change detection.
void CheckForClonedInstall(PrefService* local_state); void CheckForClonedInstall(
PrefService* local_state,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
static void RegisterPrefs(PrefRegistrySimple* registry); static void RegisterPrefs(PrefRegistrySimple* registry);
......
...@@ -10,8 +10,8 @@ ...@@ -10,8 +10,8 @@
#include "base/base_paths.h" #include "base/base_paths.h"
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/path_service.h" #include "base/path_service.h"
#include "base/threading/thread_restrictions.h"
#include "base/win/scoped_handle.h" #include "base/win/scoped_handle.h"
#include "content/public/browser/browser_thread.h"
namespace metrics { namespace metrics {
...@@ -21,7 +21,7 @@ MachineIdProvider::~MachineIdProvider() {} ...@@ -21,7 +21,7 @@ MachineIdProvider::~MachineIdProvider() {}
// On windows, the machine id is based on the serial number of the drive Chrome // On windows, the machine id is based on the serial number of the drive Chrome
// is running from. // is running from.
std::string MachineIdProvider::GetMachineId() { std::string MachineIdProvider::GetMachineId() {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE)); base::ThreadRestrictions::AssertIOAllowed();
// Use the program's path to get the drive used for the machine id. This means // Use the program's path to get the drive used for the machine id. This means
// that whenever the underlying drive changes, it's considered a new machine. // that whenever the underlying drive changes, it's considered a new machine.
......
...@@ -5,14 +5,11 @@ ...@@ -5,14 +5,11 @@
#include "chrome/browser/metrics/machine_id_provider.h" #include "chrome/browser/metrics/machine_id_provider.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
namespace metrics { namespace metrics {
TEST(MachineIdProviderTest, GetId) { TEST(MachineIdProviderTest, GetId) {
content::TestBrowserThreadBundle thread_bundle;
scoped_refptr<MachineIdProvider> provider( scoped_refptr<MachineIdProvider> provider(
MachineIdProvider::CreateInstance()); MachineIdProvider::CreateInstance());
std::string id1 = provider->GetMachineId(); std::string id1 = provider->GetMachineId();
......
...@@ -1721,8 +1721,9 @@ void MetricsService::RegisterSyntheticFieldTrial( ...@@ -1721,8 +1721,9 @@ void MetricsService::RegisterSyntheticFieldTrial(
synthetic_trial_groups_.push_back(trial_group); synthetic_trial_groups_.push_back(trial_group);
} }
void MetricsService::CheckForClonedInstall() { void MetricsService::CheckForClonedInstall(
state_manager_->CheckForClonedInstall(); scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
state_manager_->CheckForClonedInstall(task_runner);
} }
void MetricsService::GetCurrentSyntheticFieldTrials( void MetricsService::GetCurrentSyntheticFieldTrials(
......
...@@ -280,7 +280,8 @@ class MetricsService ...@@ -280,7 +280,8 @@ class MetricsService
// Check if this install was cloned or imaged from another machine. If a // Check if this install was cloned or imaged from another machine. If a
// clone is detected, reset the client id and low entropy source. This // clone is detected, reset the client id and low entropy source. This
// should not be called more than once. // should not be called more than once.
void CheckForClonedInstall(); void CheckForClonedInstall(
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
private: private:
// The MetricsService has a lifecycle that is stored as a state. // The MetricsService has a lifecycle that is stored as a state.
......
...@@ -109,7 +109,8 @@ void MetricsStateManager::ForceClientIdCreation() { ...@@ -109,7 +109,8 @@ void MetricsStateManager::ForceClientIdCreation() {
local_state_->ClearPref(prefs::kMetricsOldClientID); local_state_->ClearPref(prefs::kMetricsOldClientID);
} }
void MetricsStateManager::CheckForClonedInstall() { void MetricsStateManager::CheckForClonedInstall(
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
DCHECK(!cloned_install_detector_); DCHECK(!cloned_install_detector_);
MachineIdProvider* provider = MachineIdProvider::CreateInstance(); MachineIdProvider* provider = MachineIdProvider::CreateInstance();
...@@ -117,7 +118,7 @@ void MetricsStateManager::CheckForClonedInstall() { ...@@ -117,7 +118,7 @@ void MetricsStateManager::CheckForClonedInstall() {
return; return;
cloned_install_detector_.reset(new ClonedInstallDetector(provider)); cloned_install_detector_.reset(new ClonedInstallDetector(provider));
cloned_install_detector_->CheckForClonedInstall(local_state_); cloned_install_detector_->CheckForClonedInstall(local_state_, task_runner);
} }
scoped_ptr<const base::FieldTrial::EntropyProvider> scoped_ptr<const base::FieldTrial::EntropyProvider>
......
...@@ -42,7 +42,8 @@ class MetricsStateManager { ...@@ -42,7 +42,8 @@ class MetricsStateManager {
// Checks if this install was cloned or imaged from another machine. If a // Checks if this install was cloned or imaged from another machine. If a
// clone is detected, resets the client id and low entropy source. This // clone is detected, resets the client id and low entropy source. This
// should not be called more than once. // should not be called more than once.
void CheckForClonedInstall(); void CheckForClonedInstall(
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
// Returns the preferred entropy provider used to seed persistent activities // Returns the preferred entropy provider used to seed persistent activities
// based on whether or not metrics reporting is permitted on this client. // based on whether or not metrics reporting is permitted on this client.
......
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