Domain Reliability: Don't upload when metrics reporting is off.

components/domain_reliability/monitor.cc

Index: components/domain_reliability/monitor.cc
diff --git a/components/domain_reliability/monitor.cc b/components/domain_reliability/monitor.cc
index bde4c63c6dfd1169b45a8d25b73384dec7268178..4419f704ee3a9ac87c346def4b955891f1bd4e7b 100644
--- a/components/domain_reliability/monitor.cc
+++ b/components/domain_reliability/monitor.cc
@@ -27,6 +27,7 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
       scheduler_params_(
           DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
       dispatcher_(time_.get()),
+      reporting_pref_initialized_(false),
       weak_factory_(this) {}
 
 DomainReliabilityMonitor::DomainReliabilityMonitor(
@@ -37,25 +38,47 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
       scheduler_params_(
           DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
       dispatcher_(time_.get()),
+      reporting_pref_initialized_(false),
       weak_factory_(this) {}
 
 DomainReliabilityMonitor::~DomainReliabilityMonitor() {
   ClearContexts();
 }
 
-void DomainReliabilityMonitor::Init(
+void DomainReliabilityMonitor::InitReportingPref(

[davidben, 2014/08/20 21:49:02]

Is this called anywhere outside of test code yet?

[Deprecated (see juliatuttle), 2014/08/21 18:35:28]

It is now!

+    PrefService* local_state_pref_service,
+    const char* reporting_pref_name,
+    const scoped_refptr<base::SingleThreadTaskRunner>&
+        network_message_loop_proxy) {
+  DCHECK(!reporting_pref_initialized_);
+
+  reporting_pref_.Init(
+      reporting_pref_name,
+      local_state_pref_service,
+      base::Bind(&DomainReliabilityMonitor::OnReportingPrefChanged,
+                 base::Unretained(this)));
+  reporting_pref_.MoveToThread(network_message_loop_proxy);
+
+  reporting_pref_initialized_ = true;
+
+  OnReportingPrefChanged();

[davidben, 2014/08/20 21:49:03]

Doesn't this require that |uploader_| be initialized? But |uploader_| is only
initialized in InitURLRequestContext which is called later. (Indeed the tests
seem to segfault.)

It's also strange that OnReportingPrefChanged is usually called on the IO
thread, but here, if I understand correctly, will be called on the UI thread.

Should this line be moved to InitURLRequestContext?

[Deprecated (see juliatuttle), 2014/08/21 18:35:28]

Well-spotted.

+}
+
+void DomainReliabilityMonitor::InitURLRequestContext(
     net::URLRequestContext* url_request_context,
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner) {
+  DCHECK(reporting_pref_initialized_);
   DCHECK(!thread_checker_);
 
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter =
       new net::TrivialURLRequestContextGetter(url_request_context,
                                               task_runner);
-  Init(url_request_context_getter);
+  InitURLRequestContext(url_request_context_getter);
 }
 
-void DomainReliabilityMonitor::Init(
+void DomainReliabilityMonitor::InitURLRequestContext(
     scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
+  DCHECK(reporting_pref_initialized_);
   DCHECK(!thread_checker_);
 
   DCHECK(url_request_context_getter->GetNetworkTaskRunner()->
@@ -237,6 +260,11 @@ void DomainReliabilityMonitor::OnRequestLegComplete(
   context->OnBeacon(request.url, beacon);
 }
 
+void DomainReliabilityMonitor::OnReportingPrefChanged() {
+  // When metrics reporting is disabled, discard Domain Reliability uploads.
+  uploader_->set_discard_uploads(!*reporting_pref_);
+}
+
 // TODO(ttuttle): Keep a separate wildcard_contexts_ map to avoid having to
 // prepend '*.' to domains.
 DomainReliabilityContext* DomainReliabilityMonitor::GetContextForHost(