Propagate network delegate events to ChromeDataUseAscriber.

chrome/browser/io_thread.cc

 // Copyright (c) 2012 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/io_thread.h"
 
 #include <utility>
 #include <vector>
 
 #include "base/base64.h"
@@ skipping 31 matching lines @@
 #include "chrome/browser/net/sth_distributor_provider.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_content_client.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/certificate_transparency/tree_state_tracker.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.h"
 #include "components/data_usage/core/data_use_aggregator.h"
 #include "components/data_usage/core/data_use_amortizer.h"
 #include "components/data_usage/core/data_use_annotator.h"
+#include "components/data_use_measurement/core/data_use_ascriber.h"

[mmenke, 2016/09/22 14:36:45]

Not needed, since you never derefernce one of these, see other comment, though. 
Both comments apply to both files.

[Not at Google. Contact bengr, 2016/09/22 16:35:56]

Done.

+#include "components/data_use_measurement/core/data_use_network_delegate.h"
 #include "components/metrics/metrics_service.h"
 #include "components/net_log/chrome_net_log.h"
 #include "components/network_session_configurator/network_session_configurator.h"
 #include "components/policy/core/common/policy_service.h"
 #include "components/policy/policy_constants.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/proxy_config/pref_proxy_config_tracker.h"
 #include "components/variations/variations_associated_data.h"
 #include "components/version_info/version_info.h"
@@ skipping 459 matching lines @@
       true /* is_data_usage_off_the_record */);
 
 #if BUILDFLAG(ANDROID_JAVA_UI)
   globals_->external_data_use_observer.reset(
       new chrome::android::ExternalDataUseObserver(
           globals_->data_use_aggregator.get(),
           BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
           BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)));
 #endif
 
-  globals_->system_network_delegate = std::move(chrome_network_delegate);
+  globals_->system_network_delegate =
+      base::MakeUnique<data_use_measurement::DataUseNetworkDelegate>(
+          std::move(chrome_network_delegate),
+          globals_->data_use_ascriber.get());

[mmenke, 2016/09/22 14:36:45]

optional:  I'd suggest making this method:

globals_->data_use_ascriber->CreateNetworkDelegate(
    std::move(chrome_network_delegate));

Instead.  Then we only have to know about one DataUse class, and the ascriber
will (hopefully) be responsible for any changes in the API.  I only have a weak
preference for this, so if you prefer this approach, that's fine.

[Not at Google. Contact bengr, 2016/09/22 16:35:56]

Done. I like this suggestion since it makes the interface of the
data_use_measurement component cleaner as well.

+
   globals_->host_resolver = CreateGlobalHostResolver(net_log_);
 
   std::map<std::string, std::string> network_quality_estimator_params;
   variations::GetVariationParams(kNetworkQualityEstimatorFieldTrialName,
                                  &network_quality_estimator_params);
 
   std::unique_ptr<net::ExternalEstimateProvider> external_estimate_provider;
 #if BUILDFLAG(ANDROID_JAVA_UI)
   external_estimate_provider.reset(
       new chrome::android::ExternalEstimateProviderAndroid());
@@ skipping 532 matching lines @@
   // TODO(rtenneti): We should probably use HttpServerPropertiesManager for the
   // system URLRequestContext too. There's no reason this should be tied to a
   // profile.
   return context;
 }
 
 const metrics::UpdateUsagePrefCallbackType&
 IOThread::GetMetricsDataUseForwarder() {
   return metrics_data_use_forwarder_;
 }