Decouple Metrics Initialization from IO Thread Initialization

chrome/browser/profiles/profile_impl_io_data.cc

Index: chrome/browser/profiles/profile_impl_io_data.cc
diff --git a/chrome/browser/profiles/profile_impl_io_data.cc b/chrome/browser/profiles/profile_impl_io_data.cc
index 88da0c78fdcab3da741439d4f5bbe52166002ece..d1f649e679483e2e3ba3fbe4e5abf4a8f6e38e7d 100644
--- a/chrome/browser/profiles/profile_impl_io_data.cc
+++ b/chrome/browser/profiles/profile_impl_io_data.cc
@@ -47,6 +47,8 @@
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h"
 #include "components/data_reduction_proxy/core/browser/data_store_impl.h"
 #include "components/domain_reliability/monitor.h"
+#include "components/metrics/data_use_tracker.h"
+#include "components/metrics/metrics_service.h"
 #include "components/net_log/chrome_net_log.h"
 #include "components/prefs/json_pref_store.h"
 #include "components/prefs/pref_filter.h"
@@ -111,6 +113,15 @@ net::BackendType ChooseCacheBackendType() {
 #endif
 }
 
+metrics::UpdateUsagePrefCallbackType GetMetricsDataUseForwarder() {
+  // Some unit tests use IOThread but do not initialize MetricsService. In that
+  // case it is fine not to return a real forwarder.
+  if (g_browser_process->metrics_service()) {
+    return g_browser_process->metrics_service()->GetDataUseForwardingCallback();
+  }
+  return metrics::UpdateUsagePrefCallbackType();
+}
+
 }  // namespace
 
 using content::BrowserThread;
@@ -427,7 +438,8 @@ ProfileImplIOData::ProfileImplIOData()
     : ProfileIOData(Profile::REGULAR_PROFILE),
       http_server_properties_manager_(NULL),
       app_cache_max_size_(0),
-      app_media_cache_max_size_(0) {
+      app_media_cache_max_size_(0),
+      metrics_data_use_forwarder_(GetMetricsDataUseForwarder()) {
 }
 
 ProfileImplIOData::~ProfileImplIOData() {
@@ -485,8 +497,7 @@ void ProfileImplIOData::InitializeInternal(
   main_context_storage->set_network_delegate(
       data_reduction_proxy_io_data()->CreateNetworkDelegate(
           io_thread_globals->data_use_ascriber->CreateNetworkDelegate(
-              std::move(chrome_network_delegate),
-              io_thread->GetMetricsDataUseForwarder()),
+              std::move(chrome_network_delegate), metrics_data_use_forwarder_),

[Alexei Svitkine (slow), 2016/11/28 23:04:18]

Why can't it get it from IO thread like before?

It can still be the wrapper callback that's implemented by that file.

[robliao, 2016/11/29 18:17:43]

This changes the relationship from
1. MetricsService <- IOThread <- ProfileImpl[IOData] to

1. MetricsService <- IOThread
2. MetricsService <- ProfileImpl[IOData] 

The IO Thread only holds this callback as a convenience for ProfileImplIOData.
Having this as a direct relationship simplifies IOThread and allows
MetricsService to be the source of truth.

IOThread can do the dance it needs because it calls back from the IO thread.

[Alexei Svitkine (slow), 2016/11/29 18:34:34]

So my concern with this is now it's duplicating some logic between the two.
Specifically, you now need to have "if metrics service is not null" check in
both places.

I think if you go with my other suggestion of not having MetricsService expose a
callback anymore and for the callback to exist only to the new IOThread function
you added - then it would make sense to use this same callback here that you get
from IOThread.

[robliao, 2016/11/30 02:22:46]

Acknowledged. This is no longer an issue as I've pursued a minimal change given
I'm now changing the metrics service.

           true));
 
   main_context->set_host_resolver(