Wrapped data reduction proxy initialization into its own class

chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_io_data.cc

+// Copyright 2014 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/net/spdyproxy/data_reduction_proxy_chrome_io_data.h"
+
+#include "base/bind.h"
+#include "base/time/time.h"
+#include "chrome/browser/net/chrome_net_log.h"
+#include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_configurator.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/prefs/pref_service_syncable.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h"
+#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_statistics_prefs.h"
+#include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h"
+
+scoped_ptr<data_reduction_proxy::DataReductionProxyIOData>
+CreateDataReductionProxyChromeIOData(
+    ChromeNetLog* net_log,
+    content::BrowserContext* browser_context,
+    PrefServiceSyncable* prefs,
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+    const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner,
+    DataReductionProxyChromeConfigurator** configurator) {

[mmenke, 2014/12/17 21:37:22]

This still seems like too much code hooking together DRP objects - suspect a lot
of this could be moved into DRP/ as well.

[bengr, 2014/12/19 20:05:58]

After my next CL, the event store and the configurator can go in the DRPIOData,
but not yet.

+  DCHECK(net_log);
+  DCHECK(prefs);
+  DCHECK(browser_context);
+  DataReductionProxyChromeSettings* data_reduction_proxy_chrome_settings =
+      DataReductionProxyChromeSettingsFactory::GetForBrowserContext(
+          browser_context);
+
+#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
+
+  // 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.
+  data_reduction_proxy::DataReductionProxyEventStore*
+      data_reduction_proxy_event_store =
+          new data_reduction_proxy::DataReductionProxyEventStore(
+              ui_task_runner);
+
+  // 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.
+  // Retain a raw pointer to use for initialization of data reduction proxy
+  // settings after ownership is passed.
+  DataReductionProxyChromeConfigurator*
+      data_reduction_proxy_chrome_configurator =
+          new DataReductionProxyChromeConfigurator(
+              prefs,
+              io_task_runner,
+              net_log, data_reduction_proxy_event_store);
+
+  data_reduction_proxy::DataReductionProxyStatisticsPrefs*
+      data_reduction_proxy_statistics_prefs =
+          new data_reduction_proxy::DataReductionProxyStatisticsPrefs(
+              prefs, ui_task_runner,
+              commit_delay);
+
+  scoped_ptr<data_reduction_proxy::DataReductionProxyIOData>
+      data_reduction_proxy_data_io_data(
+          new data_reduction_proxy::DataReductionProxyIOData(
+              DataReductionProxyChromeSettings::GetClient(),
+              data_reduction_proxy_chrome_settings->params()->Clone(),
+              make_scoped_ptr(data_reduction_proxy_chrome_configurator),
+              make_scoped_ptr(data_reduction_proxy_statistics_prefs),
+              make_scoped_ptr(data_reduction_proxy_event_store),
+              base::Bind(
+                  &DataReductionProxyChromeConfigurator::
+                      GetProxyConfigOnIOThread,
+                  base::Unretained(data_reduction_proxy_chrome_configurator)),

[mmenke, 2014/12/17 21:37:22]

Passing a DataReductionProxyConfigurator and a bound method of it to the
DRPIOData is weird.  Instead, if it has to be a chrome/ class, you should make
an abstract interface in your component, with a virtual destructor and method to
get the proxy config, and then make the chrome/ class implement those methods,
and then just pass ownership here, without passing the callback.

If you need a configurator that always returns the same value for
CreateDataReductionProxyChromeIODataForSystem, then just make another subclass
of it that takes a configuration on completion.

[bengr, 2014/12/19 20:05:57]

Done.

+              base::Bind(
+                  &data_reduction_proxy::DataReductionProxySettings::
+                      SetUnreachable,
+              base::Unretained(data_reduction_proxy_chrome_settings)),

[mmenke, 2014/12/17 21:37:22]

+4 indent

[mmenke, 2014/12/17 21:37:22]

Again, should just pass in the settings, and have the drp call its method
explicitly.  No need for bound callbacks.

[bengr, 2014/12/19 20:05:57]

Done.

[bengr, 2014/12/19 20:05:57]

Done.

+              io_task_runner,
+              ui_task_runner));
+  data_reduction_proxy_chrome_settings->SetDataReductionProxyStatisticsPrefs(
+      data_reduction_proxy_statistics_prefs);
+
+  *configurator = data_reduction_proxy_chrome_configurator;
+  return data_reduction_proxy_data_io_data.Pass();
+}
+
+scoped_ptr<data_reduction_proxy::DataReductionProxyIOData>
+CreateDataReductionProxyChromeIODataForSystem(
+    ChromeNetLog* net_log,
+    PrefServiceSyncable* prefs,
+    const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner,
+    const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner) {
+  // TODO(kundaji): Move flags initialization to DataReductionProxyParams and
+  // merge with flag initialization in
+  // data_reduction_proxy_chrome_settings_factory.cc.
+  int flags = data_reduction_proxy::DataReductionProxyParams::kAllowed |
+      data_reduction_proxy::DataReductionProxyParams::kFallbackAllowed |
+      data_reduction_proxy::DataReductionProxyParams::kAlternativeAllowed;
+  if (data_reduction_proxy::DataReductionProxyParams::
+          IsIncludedInPromoFieldTrial()) {
+    flags |= data_reduction_proxy::DataReductionProxyParams::kPromoAllowed;
+  }
+  if (data_reduction_proxy::DataReductionProxyParams::
+          IsIncludedInHoldbackFieldTrial()) {
+    flags |= data_reduction_proxy::DataReductionProxyParams::kHoldback;
+  }
+  scoped_ptr<data_reduction_proxy::DataReductionProxyIOData>
+      data_reduction_proxy_data_io_data(
+          new data_reduction_proxy::DataReductionProxyIOData(
+              DataReductionProxyChromeSettings::GetClient(),
+              make_scoped_ptr(
+                  new data_reduction_proxy::DataReductionProxyParams(flags)),
+              scoped_ptr<
+                  data_reduction_proxy::DataReductionProxyConfigurator>(),
+              scoped_ptr<
+                  data_reduction_proxy::DataReductionProxyStatisticsPrefs>(),
+              scoped_ptr<data_reduction_proxy::DataReductionProxyEventStore>(),
+              data_reduction_proxy::DataReductionProxyNetworkDelegate::
+                  ProxyConfigGetter(),
+              base::Callback<void(bool)>(),
+              io_task_runner,
+              ui_task_runner));
+  return data_reduction_proxy_data_io_data.Pass();
+}