Wrapped data reduction proxy initialization into its own class

chrome/browser/profiles/profile_impl.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/profiles/profile_impl.h"
 
 #include <vector>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 26 matching lines @@
 #include "chrome/browser/domain_reliability/service_factory.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/history/top_sites.h"
 #include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/net/net_pref_observer.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/net/pref_proxy_config_tracker.h"
 #include "chrome/browser/net/proxy_service_factory.h"
+#include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_io_data.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h"
 #include "chrome/browser/net/ssl_config_service_manager.h"
 #include "chrome/browser/plugins/chrome_plugin_service_filter.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
 #include "chrome/browser/policy/profile_policy_connector_factory.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/chrome_pref_service_factory.h"
 #include "chrome/browser/prefs/pref_service_syncable.h"
@@ skipping 20 matching lines @@
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/chrome_version_info.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/chromium_strings.h"
 #include "components/bookmarks/browser/bookmark_model.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_prefs.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h"
 #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h"
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_params.h"
 #include "components/domain_reliability/monitor.h"
 #include "components/domain_reliability/service.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/metrics/metrics_service.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/signin/core/browser/signin_manager.h"
@@ skipping 538 matching lines @@
 #endif
   content::CookieStoreConfig::SessionCookieMode session_cookie_mode =
       content::CookieStoreConfig::PERSISTANT_SESSION_COOKIES;
   if (GetLastSessionExitType() == Profile::EXIT_CRASHED ||
       startup_pref_type == SessionStartupPref::LAST) {
     session_cookie_mode = content::CookieStoreConfig::RESTORED_SESSION_COOKIES;
   }
 
   ChromeNetLog* const net_log = g_browser_process->io_thread()->net_log();
 
-  base::Callback<void(bool)> data_reduction_proxy_unavailable;
-  scoped_ptr<data_reduction_proxy::DataReductionProxyParams>
-      data_reduction_proxy_params;
-  scoped_ptr<data_reduction_proxy::DataReductionProxyConfigurator> configurator;
-  scoped_ptr<data_reduction_proxy::DataReductionProxyStatisticsPrefs>
-      data_reduction_proxy_statistics_prefs;
-  scoped_ptr<data_reduction_proxy::DataReductionProxyEventStore> event_store;
-  DataReductionProxyChromeSettings* data_reduction_proxy_chrome_settings =
-      DataReductionProxyChromeSettingsFactory::GetForBrowserContext(this);
-  data_reduction_proxy_params =
-      data_reduction_proxy_chrome_settings->params()->Clone();
-  data_reduction_proxy_unavailable =
-      base::Bind(
-          &data_reduction_proxy::DataReductionProxySettings::SetUnreachable,
-          base::Unretained(data_reduction_proxy_chrome_settings));
-  // The event_store is used by DataReductionProxyChromeSettings, configurator,
-  // and ProfileIOData. Ownership is passed to the latter via
-  // ProfileIOData::Handle, which is only destroyed after
-  // BrowserContextKeyedServices, including DataReductionProxyChromeSettings
-  event_store.reset(
-      new data_reduction_proxy::DataReductionProxyEventStore(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI)));
-  // The configurator is used by DataReductionProxyChromeSettings and
-  // ProfileIOData. Ownership is passed to the latter via ProfileIOData::Handle,
-  // which is only destroyed after BrowserContextKeyedServices,
-  // including DataReductionProxyChromeSettings.
-  configurator.reset(
-      new data_reduction_proxy::DataReductionProxyConfigurator(
-          BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
-          net_log,
-          event_store.get()));
+  // Retain a raw pointer to use for initialization of data reduction proxy
+  // settings after ownership is passed.
+  data_reduction_proxy::DataReductionProxyConfigurator*
+      data_reduction_proxy_configurator = nullptr;
+
+  scoped_ptr<data_reduction_proxy::DataReductionProxyIOData>
+      data_reduction_proxy_io_data =
+          CreateDataReductionProxyChromeIOData(
+              net_log, this, prefs_.get(),
+              BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
+              BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI),
+              &data_reduction_proxy_configurator).Pass();

[mmenke, 2015/01/16 19:03:20]

Does any of this stuff need to be done in profile_impl?  Seems like it really
belongs in ProfileImplIOData.cc now...Either in Handle::Init or
Handle::LazyInitialize.

Could even put it in the non-off-the-record-block of
ProfileIOData::InitializeOnUIThread.

[bengr, 2015/01/17 00:20:47]

I think the answer is yes, for now, because InitDataReductionSettings below
needs byproducts of this construction.

[mmenke, 2015/01/17 00:29:54]

Right, I was thinking we could move that, too.

[bengr, 2015/01/21 23:28:02]

Done.

+
   // Retain a raw pointer to use for initialization of data reduction proxy
   // settings after ownership is passed
   data_reduction_proxy::DataReductionProxyEventStore*
-      data_reduction_proxy_event_store = event_store.get();
-  // Retain a raw pointer to use for initialization of data reduction proxy
-  // settings after ownership is passed.
-  data_reduction_proxy::DataReductionProxyConfigurator*
-      data_reduction_proxy_configurator = configurator.get();
-#if defined(OS_ANDROID) || defined(OS_IOS)
-  // On mobile we write data reduction proxy prefs directly to the pref service.
-  // On desktop we store data reduction proxy prefs in memory, writing to disk
-  // every 60 minutes and on termination. Shutdown hooks must be added for
-  // Android and iOS in order for non-zero delays to be supported.
-  // (http://crbug.com/408264)
-  base::TimeDelta commit_delay = base::TimeDelta();
-#else
-  base::TimeDelta commit_delay = base::TimeDelta::FromMinutes(60);
-#endif
-  // TODO(bengr): Remove this in M-43.
-  data_reduction_proxy::MigrateStatisticsPrefs(g_browser_process->local_state(),
-                                               prefs_.get());
-  data_reduction_proxy_statistics_prefs =
-      scoped_ptr<data_reduction_proxy::DataReductionProxyStatisticsPrefs>(
-          new data_reduction_proxy::DataReductionProxyStatisticsPrefs(
-              prefs_.get(),
-              base::MessageLoopProxy::current(),
-              commit_delay));
-  data_reduction_proxy_chrome_settings->SetDataReductionProxyStatisticsPrefs(
-      data_reduction_proxy_statistics_prefs.get());
+      data_reduction_proxy_event_store =
+          data_reduction_proxy_io_data->event_store();
 
   // Make sure we initialize the ProfileIOData after everything else has been
   // initialized that we might be reading from the IO thread.
 
   io_data_.Init(cookie_path, channel_id_path, cache_path,
                 cache_max_size, media_cache_path, media_cache_max_size,
                 extensions_cookie_path, GetPath(), infinite_cache_path,
                 predictor_, session_cookie_mode, GetSpecialStoragePolicy(),
                 CreateDomainReliabilityMonitor(local_state),
-                data_reduction_proxy_unavailable,
-                configurator.Pass(),
-                data_reduction_proxy_params.Pass(),
-                data_reduction_proxy_statistics_prefs.Pass(),
-                event_store.Pass());
-  data_reduction_proxy_chrome_settings->InitDataReductionProxySettings(
-      data_reduction_proxy_configurator,
-      prefs_.get(),
-      g_browser_process->local_state(),
-      GetRequestContext(),
-      net_log,
-      data_reduction_proxy_event_store);
+                data_reduction_proxy_io_data.Pass());
+  DataReductionProxyChromeSettingsFactory::GetForBrowserContext(this)->

[mmenke, 2015/01/16 19:03:20]

Cant this call be moved before io_data_.Init?  I assume not, because this
depends on the request context.  If we could, it could be nice if we could make
CreateDataReductionProxyChromeIOData do that call as well.

Also seems kinda weird to set up the IO thread stuff first.

[bengr, 2015/01/17 00:20:47]

Ack that this is kinda weird and not ideal. Unfortunately it can't be moved
before io_data_.Init(). I think jeremyim's upcoming changes will move the io
dependencies out of this call, so hopefully we'll be able to find a better place
for it then. 

+      InitDataReductionProxySettings(
+          data_reduction_proxy_configurator, prefs_.get(),
+          g_browser_process->local_state(), GetRequestContext(), net_log,

[mmenke, 2015/01/16 19:03:20]

Question:  Not for this CL, but for the future...Do we need to use the profile's
RequestContext here?  If we could use the system request context, we'd get rid
of the circular dependency.  We're bypassing the proxy settings anyways.

The downside is that I don't think we currently create a URLRequestContextGetter
on the UI thread for the system context (Or the PAC context, for that matter).

[bengr, 2015/01/17 00:20:47]

See above reply. Even better is that we'd remove the need for a request context
at all from DRPSettings.

+          data_reduction_proxy_event_store);
 
 #if defined(ENABLE_PLUGINS)
   ChromePluginServiceFilter::GetInstance()->RegisterResourceContext(
       PluginPrefs::GetForProfile(this).get(),
       io_data_.GetResourceContextNoInit());
 #endif
 
   // Delay README creation to not impact startup performance.
   BrowserThread::PostDelayedTask(
         BrowserThread::FILE, FROM_HERE,
@@ skipping 683 matching lines @@
 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));
 }