Use data reduction proxy when managed proxy config returns direct

chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_configurator.h

Index: chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_configurator.h
diff --git a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_configurator.h b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_configurator.h
index 8efc3edd4ea7221091995f922acca32b3da01fff..72c7d4926a00fdf8378f49424d51c55deb5f1dd5 100644
--- a/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_configurator.h
+++ b/chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_configurator.h
@@ -9,14 +9,27 @@
 #include <vector>
 
 #include "base/gtest_prod_util.h"
+#include "base/task_runner.h"
 #include "components/data_reduction_proxy/browser/data_reduction_proxy_configurator.h"
+#include "net/proxy/proxy_config.h"
+

[Ryan Sleevi, 2014/08/14 19:19:51]

IWYU: You need to include "base/memory/ref_counted.h" to use scoped_refptr

+namespace base {
+class SequencedTaskRunner;
+}
+
+namespace net {
+class ProxyInfo;
+class ProxyService;
+}
 
 class PrefService;
 
 class DataReductionProxyChromeConfigurator
     : public data_reduction_proxy::DataReductionProxyConfigurator {
  public:
-  explicit DataReductionProxyChromeConfigurator(PrefService* prefs);
+  explicit DataReductionProxyChromeConfigurator(
+      PrefService* prefs,
+      scoped_refptr<base::SequencedTaskRunner> network_task_runner);

[Ryan Sleevi, 2014/08/14 19:19:51]

This should be a const-ref scoped_refptr

scoped_ptr is the only class in Chrome that should be passed by value. All of
the rest should be const-ref or pointers.

(For scoped_refptr<>s, it means you're adding an additional AddRef/Release call
that isn't needed)

   virtual ~DataReductionProxyChromeConfigurator();
 
   virtual void Enable(bool primary_restricted,
@@ -38,11 +51,21 @@ class DataReductionProxyChromeConfigurator
   // as a hostname pattern. Subclasses may implement other semantics.
   virtual void AddURLPatternToBypass(const std::string& pattern) OVERRIDE;
 
+  // Updates the config for use on the IO thread.

[Ryan Sleevi, 2014/08/14 19:19:51]

So, this generally represents a dangerous design patern, in that now it means
the DataReductionProxyChromeConfigurator lives on multiple threads.

The d_r_p:DRPC doesn't have this requirement, and you're exposing these methods
publicly, which makes it very easy to get ownership wrong.

Put differently, it bakes how this class is used into the design, which is
usually a sign of a layering violation occuring.

+  void UpdateProxyConfigOnIO(const net::ProxyConfig& config);
+
+  // Returns the current data reduction proxy config, even if it is not the
+  // effective configuration used by the proxy service.
+  const net::ProxyConfig& GetProxyConfigOnIO() const;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(DataReductionProxyConfigTest, TestBypassList);
 
   PrefService* prefs_;
+  scoped_refptr<base::SequencedTaskRunner> network_task_runner_;
+
   std::vector<std::string> bypass_rules_;
+  net::ProxyConfig config_;
 
   DISALLOW_COPY_AND_ASSIGN(DataReductionProxyChromeConfigurator);
 };