Skip to content

Commit

Permalink
Domain Reliability: Don't upload when metrics reporting is off.
Browse files Browse the repository at this point in the history
Tie Domain Reliability uploads to metrics reporting -- halt uploads when
reporting is turned off. (Keep recording, since we will eventually be able to
access reports through JavaScript as well.)

BUG=407170

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

Cr-Commit-Position: refs/heads/master@{#291740}
  • Loading branch information
ttuttle authored and Commit bot committed Aug 25, 2014
1 parent 0788407 commit fa8427f
Show file tree
Hide file tree
Showing 17 changed files with 264 additions and 60 deletions.
10 changes: 6 additions & 4 deletions base/prefs/pref_member.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
#include "base/callback.h"
#include "base/callback_helpers.h"
#include "base/location.h"
#include "base/message_loop/message_loop_proxy.h"
#include "base/prefs/pref_service.h"
#include "base/value_conversions.h"

using base::MessageLoopProxy;
using base::SingleThreadTaskRunner;

namespace subtle {

Expand Down Expand Up @@ -53,12 +55,12 @@ void PrefMemberBase::Destroy() {
}

void PrefMemberBase::MoveToThread(
const scoped_refptr<MessageLoopProxy>& message_loop) {
const scoped_refptr<SingleThreadTaskRunner>& task_runner) {
VerifyValuePrefName();
// Load the value from preferences if it hasn't been loaded so far.
if (!internal())
UpdateValueFromPref(base::Closure());
internal()->MoveToThread(message_loop);
internal()->MoveToThread(task_runner);
}

void PrefMemberBase::OnPreferenceChanged(PrefService* service,
Expand Down Expand Up @@ -127,9 +129,9 @@ void PrefMemberBase::Internal::UpdateValue(
}

void PrefMemberBase::Internal::MoveToThread(
const scoped_refptr<MessageLoopProxy>& message_loop) {
const scoped_refptr<SingleThreadTaskRunner>& task_runner) {
CheckOnCorrectThread();
thread_loop_ = message_loop;
thread_loop_ = task_runner;
}

bool PrefMemberVectorStringUpdate(const base::Value& value,
Expand Down
14 changes: 8 additions & 6 deletions base/prefs/pref_member.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/memory/ref_counted.h"
#include "base/message_loop/message_loop_proxy.h"
#include "base/prefs/base_prefs_export.h"
#include "base/prefs/pref_observer.h"
#include "base/single_thread_task_runner.h"
#include "base/values.h"

class PrefService;
Expand Down Expand Up @@ -66,7 +66,7 @@ class BASE_PREFS_EXPORT PrefMemberBase : public PrefObserver {
const base::Closure& callback) const;

void MoveToThread(
const scoped_refptr<base::MessageLoopProxy>& message_loop);
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);

// See PrefMember<> for description.
bool IsManaged() const {
Expand All @@ -92,7 +92,7 @@ class BASE_PREFS_EXPORT PrefMemberBase : public PrefObserver {

bool IsOnCorrectThread() const;

scoped_refptr<base::MessageLoopProxy> thread_loop_;
scoped_refptr<base::SingleThreadTaskRunner> thread_loop_;
mutable bool is_managed_;
mutable bool is_user_modifiable_;

Expand All @@ -112,7 +112,8 @@ class BASE_PREFS_EXPORT PrefMemberBase : public PrefObserver {
// See PrefMember<> for description.
void Destroy();

void MoveToThread(const scoped_refptr<base::MessageLoopProxy>& message_loop);
void MoveToThread(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner);

// PrefObserver
virtual void OnPreferenceChanged(PrefService* service,
Expand Down Expand Up @@ -197,8 +198,9 @@ class PrefMember : public subtle::PrefMemberBase {
// via PostTask.
// This method should only be used from the thread the PrefMember is currently
// on, which is the UI thread by default.
void MoveToThread(const scoped_refptr<base::MessageLoopProxy>& message_loop) {
subtle::PrefMemberBase::MoveToThread(message_loop);
void MoveToThread(
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
subtle::PrefMemberBase::MoveToThread(task_runner);
}

// Check whether the pref is managed, i.e. controlled externally through
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,9 @@ class MockDomainReliabilityService : public DomainReliabilityService {
virtual ~MockDomainReliabilityService() {}

virtual scoped_ptr<DomainReliabilityMonitor> CreateMonitor(
scoped_refptr<base::SequencedTaskRunner> network_task_runner) OVERRIDE {
scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
PrefService* local_state_pref_service,
const char* reporting_pref_name) OVERRIDE {
NOTREACHED();
return scoped_ptr<DomainReliabilityMonitor>();
}
Expand Down
8 changes: 5 additions & 3 deletions chrome/browser/profiles/profile_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ void ProfileImpl::DoFinalInit() {
cache_max_size, media_cache_path, media_cache_max_size,
extensions_cookie_path, GetPath(), infinite_cache_path,
predictor_, session_cookie_mode, GetSpecialStoragePolicy(),
CreateDomainReliabilityMonitor(),
CreateDomainReliabilityMonitor(local_state),
data_reduction_proxy_unavailable,
chrome_configurator.Pass(),
data_reduction_proxy_params.Pass());
Expand Down Expand Up @@ -1450,13 +1450,15 @@ PrefProxyConfigTracker* ProfileImpl::CreateProxyConfigTracker() {
}

scoped_ptr<domain_reliability::DomainReliabilityMonitor>
ProfileImpl::CreateDomainReliabilityMonitor() {
ProfileImpl::CreateDomainReliabilityMonitor(PrefService* local_state) {
domain_reliability::DomainReliabilityService* service =
domain_reliability::DomainReliabilityServiceFactory::GetInstance()->
GetForBrowserContext(this);
if (!service)
return scoped_ptr<domain_reliability::DomainReliabilityMonitor>();

return service->CreateMonitor(
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
local_state,
prefs::kMetricsReportingEnabled);
}
2 changes: 1 addition & 1 deletion chrome/browser/profiles/profile_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class ProfileImpl : public Profile {
PrefProxyConfigTracker* CreateProxyConfigTracker();

scoped_ptr<domain_reliability::DomainReliabilityMonitor>
CreateDomainReliabilityMonitor();
CreateDomainReliabilityMonitor(PrefService* local_state);

scoped_ptr<content::HostZoomMap::Subscription> zoom_subscription_;
PrefChangeRegistrar pref_change_registrar_;
Expand Down
16 changes: 10 additions & 6 deletions chrome/browser/profiles/profile_impl_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ ProfileImplIOData::Handle::~Handle() {
if (io_data_->http_server_properties_manager_)
io_data_->http_server_properties_manager_->ShutdownOnPrefThread();

if (io_data_->domain_reliability_monitor_)
io_data_->domain_reliability_monitor_->DestroyReportingPref();

io_data_->ShutdownOnUIThread(GetAllContextGetters().Pass());
}

Expand Down Expand Up @@ -168,6 +171,8 @@ void ProfileImplIOData::Handle::Init(
io_data_->domain_reliability_monitor_ = domain_reliability_monitor.Pass();

io_data_->InitializeMetricsEnabledStateOnUIThread();
if (io_data_->domain_reliability_monitor_)
io_data_->domain_reliability_monitor_->MoveToNetworkThread();

#if defined(SPDY_PROXY_AUTH_ORIGIN)
io_data_->data_reduction_proxy_unavailable_callback_ =
Expand Down Expand Up @@ -597,12 +602,11 @@ void ProfileImplIOData::InitializeInternal(
details));

if (domain_reliability_monitor_) {
domain_reliability_monitor_->Init(
main_context,
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
domain_reliability_monitor_->AddBakedInConfigs();
network_delegate()->set_domain_reliability_monitor(
domain_reliability_monitor_.get());
domain_reliability::DomainReliabilityMonitor* monitor =
domain_reliability_monitor_.get();
monitor->InitURLRequestContext(main_context);
monitor->AddBakedInConfigs();
network_delegate()->set_domain_reliability_monitor(monitor);
}

lazy_params_.reset();
Expand Down
1 change: 1 addition & 0 deletions components/domain_reliability.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'type': '<(component)',
'dependencies': [
'../base/base.gyp:base',
'../base/base.gyp:base_prefs',
'../components/components.gyp:keyed_service_core',
'../content/content.gyp:content_browser',
'../net/net.gyp:net',
Expand Down
106 changes: 87 additions & 19 deletions components/domain_reliability/monitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,8 @@

#include "base/command_line.h"
#include "base/logging.h"
#include "base/message_loop/message_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/task_runner.h"
#include "base/threading/thread_checker.h"
#include "base/time/time.h"
#include "components/domain_reliability/baked_in_configs.h"
#include "net/base/load_flags.h"
Expand All @@ -21,52 +19,96 @@
namespace domain_reliability {

DomainReliabilityMonitor::DomainReliabilityMonitor(
const std::string& upload_reporter_string)
const std::string& upload_reporter_string,
scoped_refptr<base::SingleThreadTaskRunner> pref_thread,
scoped_refptr<base::SingleThreadTaskRunner> network_thread,
PrefService* local_state_pref_service,
const char* reporting_pref_name)
: time_(new ActualTime()),
upload_reporter_string_(upload_reporter_string),
scheduler_params_(
DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
dispatcher_(time_.get()),
weak_factory_(this) {}
pref_task_runner_(pref_thread),
network_task_runner_(network_thread),
moved_to_network_thread_(false),
weak_factory_(this) {
DCHECK(OnPrefThread());
InitReportingPref(local_state_pref_service, reporting_pref_name);
}

DomainReliabilityMonitor::DomainReliabilityMonitor(
const std::string& upload_reporter_string,
scoped_refptr<base::SingleThreadTaskRunner> pref_thread,
scoped_refptr<base::SingleThreadTaskRunner> network_thread,
PrefService* local_state_pref_service,
const char* reporting_pref_name,
scoped_ptr<MockableTime> time)
: time_(time.Pass()),
upload_reporter_string_(upload_reporter_string),
scheduler_params_(
DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
dispatcher_(time_.get()),
weak_factory_(this) {}
pref_task_runner_(pref_thread),
network_task_runner_(network_thread),
moved_to_network_thread_(false),
weak_factory_(this) {
DCHECK(OnPrefThread());
InitReportingPref(local_state_pref_service, reporting_pref_name);
}

DomainReliabilityMonitor::~DomainReliabilityMonitor() {
if (moved_to_network_thread_)
DCHECK(OnNetworkThread());
else
DCHECK(OnPrefThread());

ClearContexts();
}

void DomainReliabilityMonitor::Init(
net::URLRequestContext* url_request_context,
const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
DCHECK(!thread_checker_);
void DomainReliabilityMonitor::MoveToNetworkThread() {
DCHECK(OnPrefThread());
DCHECK(!moved_to_network_thread_);

reporting_pref_.MoveToThread(network_task_runner_);
moved_to_network_thread_ = true;
}

void DomainReliabilityMonitor::DestroyReportingPref() {
DCHECK(OnPrefThread());

reporting_pref_.Destroy();
}

void DomainReliabilityMonitor::InitURLRequestContext(
net::URLRequestContext* url_request_context) {
DCHECK(OnNetworkThread());
DCHECK(moved_to_network_thread_);

scoped_refptr<net::URLRequestContextGetter> url_request_context_getter =
new net::TrivialURLRequestContextGetter(url_request_context,
task_runner);
Init(url_request_context_getter);
network_task_runner_);
InitURLRequestContext(url_request_context_getter);
}

void DomainReliabilityMonitor::Init(
void DomainReliabilityMonitor::InitURLRequestContext(
scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
DCHECK(!thread_checker_);
DCHECK(OnNetworkThread());
DCHECK(moved_to_network_thread_);

// Make sure the URLRequestContext actually lives on what was declared to be
// the network thread.
DCHECK(url_request_context_getter->GetNetworkTaskRunner()->
RunsTasksOnCurrentThread());

uploader_ = DomainReliabilityUploader::Create(url_request_context_getter);
thread_checker_.reset(new base::ThreadChecker());
// Make sure the uploader is sending or discarding uploads according to pref.
OnReportingPrefChanged();
}

void DomainReliabilityMonitor::AddBakedInConfigs() {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
DCHECK(OnNetworkThread());

base::Time now = base::Time::Now();
for (size_t i = 0; kBakedInJsonConfigs[i]; ++i) {
std::string json(kBakedInJsonConfigs[i]);
Expand All @@ -82,14 +124,16 @@ void DomainReliabilityMonitor::AddBakedInConfigs() {
}

void DomainReliabilityMonitor::OnBeforeRedirect(net::URLRequest* request) {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
DCHECK(OnNetworkThread());

// Record the redirect itself in addition to the final request.
OnRequestLegComplete(RequestInfo(*request));
}

void DomainReliabilityMonitor::OnCompleted(net::URLRequest* request,
bool started) {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
DCHECK(OnNetworkThread());

if (!started)
return;
RequestInfo request_info(*request);
Expand All @@ -103,7 +147,7 @@ void DomainReliabilityMonitor::OnCompleted(net::URLRequest* request,

void DomainReliabilityMonitor::ClearBrowsingData(
DomainReliabilityClearMode mode) {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
DCHECK(OnNetworkThread());

switch (mode) {
case CLEAR_BEACONS: {
Expand All @@ -121,6 +165,8 @@ void DomainReliabilityMonitor::ClearBrowsingData(
}

scoped_ptr<base::Value> DomainReliabilityMonitor::GetWebUIData() const {
DCHECK(OnNetworkThread());

base::ListValue* contexts_value = new base::ListValue();
for (ContextMap::const_iterator it = contexts_.begin();
it != contexts_.end();
Expand All @@ -136,7 +182,8 @@ scoped_ptr<base::Value> DomainReliabilityMonitor::GetWebUIData() const {

DomainReliabilityContext* DomainReliabilityMonitor::AddContextForTesting(
scoped_ptr<const DomainReliabilityConfig> config) {
DCHECK(thread_checker_ && thread_checker_->CalledOnValidThread());
DCHECK(OnNetworkThread());

return AddContext(config.Pass());
}

Expand All @@ -161,6 +208,7 @@ bool DomainReliabilityMonitor::RequestInfo::AccessedNetwork() const {

DomainReliabilityContext* DomainReliabilityMonitor::AddContext(
scoped_ptr<const DomainReliabilityConfig> config) {
DCHECK(OnNetworkThread());
DCHECK(config);
DCHECK(config->IsValid());

Expand Down Expand Up @@ -237,10 +285,30 @@ void DomainReliabilityMonitor::OnRequestLegComplete(
context->OnBeacon(request.url, beacon);
}

void DomainReliabilityMonitor::InitReportingPref(
PrefService* local_state_pref_service,
const char* reporting_pref_name) {
reporting_pref_.Init(
reporting_pref_name,
local_state_pref_service,
base::Bind(&DomainReliabilityMonitor::OnReportingPrefChanged,
base::Unretained(this)));
}

void DomainReliabilityMonitor::OnReportingPrefChanged() {
DCHECK(OnNetworkThread());

// When metrics reporting is disabled, discard Domain Reliability uploads.
if (uploader_)
uploader_->set_discard_uploads(!*reporting_pref_);
}

// TODO(ttuttle): Keep a separate wildcard_contexts_ map to avoid having to
// prepend '*.' to domains.
DomainReliabilityContext* DomainReliabilityMonitor::GetContextForHost(
const std::string& host) const {
DCHECK(OnNetworkThread());

ContextMap::const_iterator context_it;

context_it = contexts_.find(host);
Expand Down
Loading

0 comments on commit fa8427f

Please sign in to comment.