Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for DataReductionProxy

Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for DataReductionProxy

This involves removing two methods off the NetworkDelegate interface (and four off LayeredNetworkDelegate). DataReductionProxy's logic for these two methods will be moved to the DataReductionProxyDelegate from the DataReductionProxyNetworkDelegate. This also involved cleaning up some unit tests and moving calls to use ProxyDelegate instead of NetworkDelegate where relevant.

Refactoring proxy related calls from NetworkDelegate to ProxyDelegate

BUG=583369
Committed: https://crrev.com/7de050c68ce2a97c0165d09d06c89b8aff79fb77
Cr-Commit-Position: refs/heads/master@{#376860}

[RyanSturm, 2016-02-08 20:59:36]

Description was changed from

==========
Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for
DataReductionProxy

This involves removing two methods off the NetworkDelegate interface (and four
off LayeredNetworkDelegate). DataReductionProxy's logic for these two methods
will be moved to the DataReductionProxyDelegate from the
DataReductionProxyNetworkDelegate. This also involved cleaning up some unit
tests and moving calls to use ProxyDelegate instead of NetworkDelegate where
relevant.

Refactoring proxy related calls from NetworkDelegate to ProxyDelegate

BUG=583369
==========

to

==========
Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for
DataReductionProxy

This involves removing two methods off the NetworkDelegate interface (and four
off LayeredNetworkDelegate). DataReductionProxy's logic for these two methods
will be moved to the DataReductionProxyDelegate from the
DataReductionProxyNetworkDelegate. This also involved cleaning up some unit
tests and moving calls to use ProxyDelegate instead of NetworkDelegate where
relevant.

Refactoring proxy related calls from NetworkDelegate to ProxyDelegate

BUG=583369
==========

[RyanSturm, 2016-02-08 20:59:36]

ryansturm@chromium.org changed reviewers:
+ bengr@chromium.org

[RyanSturm, 2016-02-08 21:00:31]

Refactoring the proxy logic out of NetworkDelegate and into ProxyDelegate.

[bengr, 2016-02-08 23:23:30]

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:808:
scoped_ptr<net::ProxyDelegate> proxy_delegate_;
#include "net/base/proxy_delegate.h"

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc:704:
scoped_ptr<net::ProxyDelegate> proxy_delegate_;
#include "net/base/proxy_delegate.h"

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc:290:
net::TestURLRequestContext context_;
I believe this needs #include "net/url_request/url_request_test_util.h";

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc:297:
scoped_ptr<DataReductionProxyDelegate> delegate_;
#include ".../data_reduction_proxy_network_delegate.h"

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc:76:
if (bypass_stats_) {
Remove curly braces.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc:121:
if (data_reduction_proxy_config.is_valid() &&
Suggestion: I know you copied and pasted, but I find it slightly more readable
to return early. I.e.:

if (!d_r_p_c.is_valid() || ...) {
  return;
}

...

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h:78:
void OnResolveProxyHandler(const GURL& url,
Why is this visible? Can you just put this in an anonymous namespace in the .cc?
Also, add a comment explaining what this function does.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc:299:
scoped_ptr<net::ProxyDelegate> proxy_delegate_;
#include "net/base/proxy_delegate.h"

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc:387:
// Without DataCompressionProxyCriticalBypass Finch trial set, the
See https://code.google.com/p/chromium/issues/detail?id=585200

You can kill two birds with one stone.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc:511:
}  // namespace data_reduction_proxy
Did you add whitespace?

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc:241:
scoped_ptr<DataReductionProxyDelegate> proxy_delegate(
Can you return make_scoped_ptr(new DRPD...)?

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h:86:
scoped_ptr<DataReductionProxyDelegate> CreateProxyDelegate();
Can this method be const?

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h:64:
// Constructs a DataReductionProxyNetworkdelegate object with the given
DataReductionProxyNetworkdelegate -> DataReductionProxyNetworkDelegate

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
File net/base/layered_network_delegate.h (right):

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
net/base/layered_network_delegate.h:29: class ProxyService;
You don't need ProxyServer or ProxyService anymore.

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
File net/base/layered_network_delegate_unittest.cc (right):

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
net/base/layered_network_delegate_unittest.cc:23: #include
"net/proxy/proxy_service.h"
You don't need this anymore.

https://codereview.chromium.org/1680893002/diff/1/net/url_request/url_request...
File net/url_request/url_request_test_util.cc (right):

https://codereview.chromium.org/1680893002/diff/1/net/url_request/url_request...
net/url_request/url_request_test_util.cc:63: client_socket_factory_(NULL),
Suggest changing every NULL to a nullptr.

https://codereview.chromium.org/1680893002/diff/1/net/url_request/url_request...
net/url_request/url_request_test_util.cc:105: if (http_network_session_params_)
{
Remove curly braces. Chromium doesn't use them on single line blocks.

[RyanSturm, 2016-02-09 00:46:03]

Took all suggested changes except DataCompressionProxyCriticalBypass, as there
are some other changes needed for that outside the scope of this change (i.e.
removing the plumbing for load_flags)

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:808:
scoped_ptr<net::ProxyDelegate> proxy_delegate_;
On 2016/02/08 23:23:29, bengr wrote:
> #include "net/base/proxy_delegate.h"

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc:704:
scoped_ptr<net::ProxyDelegate> proxy_delegate_;
On 2016/02/08 23:23:29, bengr wrote:
> #include "net/base/proxy_delegate.h"

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc:290:
net::TestURLRequestContext context_;
On 2016/02/08 23:23:29, bengr wrote:
> I believe this needs #include "net/url_request/url_request_test_util.h";

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc:297:
scoped_ptr<DataReductionProxyDelegate> delegate_;
On 2016/02/08 23:23:29, bengr wrote:
> #include ".../data_reduction_proxy_network_delegate.h"

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc:76:
if (bypass_stats_) {
On 2016/02/08 23:23:29, bengr wrote:
> Remove curly braces.

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc:121:
if (data_reduction_proxy_config.is_valid() &&
On 2016/02/08 23:23:29, bengr wrote:
> Suggestion: I know you copied and pasted, but I find it slightly more readable
> to return early. I.e.:
> 
> if (!d_r_p_c.is_valid() || ...) {
>   return;
> }
> 
> ... 

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h:78:
void OnResolveProxyHandler(const GURL& url,
On 2016/02/08 23:23:29, bengr wrote:
> Why is this visible? Can you just put this in an anonymous namespace in the
.cc?
> Also, add a comment explaining what this function does.

This is tested directly in a couple unittests:
https://code.google.com/p/chromium/codesearch#chromium/src/components/data_re...

I'll add the comment though.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc:299:
scoped_ptr<net::ProxyDelegate> proxy_delegate_;
On 2016/02/08 23:23:29, bengr wrote:
> #include "net/base/proxy_delegate.h"

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc:387:
// Without DataCompressionProxyCriticalBypass Finch trial set, the
On 2016/02/08 23:23:29, bengr wrote:
> See https://code.google.com/p/chromium/issues/detail?id=585200
> 
> You can kill two birds with one stone.

Acknowledged.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc:511:
}  // namespace data_reduction_proxy
On 2016/02/08 23:23:29, bengr wrote:
> Did you add whitespace?

Eclipse did. Seems appropriate though. Before there was no new line after //
namespace data_reduction_proxy. Let me know if it should be removed.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc:241:
scoped_ptr<DataReductionProxyDelegate> proxy_delegate(
On 2016/02/08 23:23:29, bengr wrote:
> Can you return make_scoped_ptr(new DRPD...)?

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h:86:
scoped_ptr<DataReductionProxyDelegate> CreateProxyDelegate();
On 2016/02/08 23:23:29, bengr wrote:
> Can this method be const?

Done.

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h
(right):

https://codereview.chromium.org/1680893002/diff/1/components/data_reduction_p...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.h:64:
// Constructs a DataReductionProxyNetworkdelegate object with the given
On 2016/02/08 23:23:29, bengr wrote:
> DataReductionProxyNetworkdelegate -> DataReductionProxyNetworkDelegate

Done.

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
File net/base/layered_network_delegate.h (right):

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
net/base/layered_network_delegate.h:29: class ProxyService;
On 2016/02/08 23:23:30, bengr wrote:
> You don't need ProxyServer or ProxyService anymore.

Done.

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
File net/base/layered_network_delegate_unittest.cc (right):

https://codereview.chromium.org/1680893002/diff/1/net/base/layered_network_de...
net/base/layered_network_delegate_unittest.cc:23: #include
"net/proxy/proxy_service.h"
On 2016/02/08 23:23:30, bengr wrote:
> You don't need this anymore.

Done.

https://codereview.chromium.org/1680893002/diff/1/net/url_request/url_request...
File net/url_request/url_request_test_util.cc (right):

https://codereview.chromium.org/1680893002/diff/1/net/url_request/url_request...
net/url_request/url_request_test_util.cc:63: client_socket_factory_(NULL),
On 2016/02/08 23:23:30, bengr wrote:
> Suggest changing every NULL to a nullptr.

Done.

https://codereview.chromium.org/1680893002/diff/1/net/url_request/url_request...
net/url_request/url_request_test_util.cc:105: if (http_network_session_params_)
{
On 2016/02/08 23:23:30, bengr wrote:
> Remove curly braces. Chromium doesn't use them on single line blocks.

Done.

[RyanSturm, 2016-02-09 22:50:31]

ryansturm@chromium.org changed reviewers:
+ asanka@chromium.org

[RyanSturm, 2016-02-09 22:50:31]

Rebased change and adding asanka as a reviewer for /net/

[asanka, 2016-02-10 23:47:49]

lgtm. only nits.

cc += mmenke FYI since he's also CC'd on the bug.

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service.cc
File net/proxy/proxy_service.cc (right):

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
net/proxy/proxy_service.cc:1090: NULL /* pac_request*/, proxy_delegate,
nit: nullptr

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
net/proxy/proxy_service.cc:1381: // Allow the network delegate to interpose on
the resolution decision,
s/network delegate/proxy delegate/ here and elsewhere

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
File net/proxy/proxy_service_unittest.cc (right):

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
net/proxy/proxy_service_unittest.cc:171: proxy_service_(NULL) {}
nullptr

[mmenke, 2016-02-10 23:51:35]

Awesome!  Thanks so much for doing this!

[RyanSturm, 2016-02-11 17:46:59]

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service.cc
File net/proxy/proxy_service.cc (right):

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
net/proxy/proxy_service.cc:1090: NULL /* pac_request*/, proxy_delegate,
On 2016/02/10 23:47:48, asanka wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
net/proxy/proxy_service.cc:1381: // Allow the network delegate to interpose on
the resolution decision,
On 2016/02/10 23:47:49, asanka wrote:
> s/network delegate/proxy delegate/ here and elsewhere

Done.

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
File net/proxy/proxy_service_unittest.cc (right):

https://codereview.chromium.org/1680893002/diff/30001/net/proxy/proxy_service...
net/proxy/proxy_service_unittest.cc:171: proxy_service_(NULL) {}
On 2016/02/10 23:47:49, asanka wrote:
> nullptr

Done.

[bengr, 2016-02-16 19:06:17]

lgtm. Thanks for doing this. One nit.

https://codereview.chromium.org/1680893002/diff/50001/net/proxy/proxy_service...
File net/proxy/proxy_service_unittest.cc (right):

https://codereview.chromium.org/1680893002/diff/50001/net/proxy/proxy_service...
net/proxy/proxy_service_unittest.cc:232: // ProxyDelegate implementation:
Add a blank line above.

[RyanSturm, 2016-02-17 00:08:31]

https://codereview.chromium.org/1680893002/diff/50001/net/proxy/proxy_service...
File net/proxy/proxy_service_unittest.cc (right):

https://codereview.chromium.org/1680893002/diff/50001/net/proxy/proxy_service...
net/proxy/proxy_service_unittest.cc:232: // ProxyDelegate implementation:
On 2016/02/16 19:06:17, bengr wrote:
> Add a blank line above.

Done.

[RyanSturm, 2016-02-22 19:27:37]

The CQ bit was checked by ryansturm@chromium.org

[RyanSturm, 2016-02-22 19:27:37]

The patchset sent to the CQ was uploaded after l-g-t-m from asanka@chromium.org,
bengr@chromium.org
Link to the patchset: https://codereview.chromium.org/1680893002/#ps70001
(title: "rebase and add new line")

[commit-bot: I haz the power, 2016-02-22 19:30:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1680893002/70001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1680893002/70001

[commit-bot: I haz the power, 2016-02-22 19:41:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-02-22 19:41:01]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[RyanSturm, 2016-02-22 23:59:22]

The CQ bit was checked by ryansturm@chromium.org

[RyanSturm, 2016-02-22 23:59:23]

The patchset sent to the CQ was uploaded after l-g-t-m from asanka@chromium.org,
bengr@chromium.org
Link to the patchset: https://codereview.chromium.org/1680893002/#ps90001
(title: "Rebase")

[commit-bot: I haz the power, 2016-02-22 23:59:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1680893002/90001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1680893002/90001

[commit-bot: I haz the power, 2016-02-23 00:10:28]

Description was changed from

==========
Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for
DataReductionProxy

This involves removing two methods off the NetworkDelegate interface (and four
off LayeredNetworkDelegate). DataReductionProxy's logic for these two methods
will be moved to the DataReductionProxyDelegate from the
DataReductionProxyNetworkDelegate. This also involved cleaning up some unit
tests and moving calls to use ProxyDelegate instead of NetworkDelegate where
relevant.

Refactoring proxy related calls from NetworkDelegate to ProxyDelegate

BUG=583369
==========

to

==========
Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for
DataReductionProxy

This involves removing two methods off the NetworkDelegate interface (and four
off LayeredNetworkDelegate). DataReductionProxy's logic for these two methods
will be moved to the DataReductionProxyDelegate from the
DataReductionProxyNetworkDelegate. This also involved cleaning up some unit
tests and moving calls to use ProxyDelegate instead of NetworkDelegate where
relevant.

Refactoring proxy related calls from NetworkDelegate to ProxyDelegate

BUG=583369
==========

[commit-bot: I haz the power, 2016-02-23 00:10:28]

Committed patchset #6 (id:90001)

[commit-bot: I haz the power, 2016-02-23 00:11:29]

Description was changed from

==========
Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for
DataReductionProxy

This involves removing two methods off the NetworkDelegate interface (and four
off LayeredNetworkDelegate). DataReductionProxy's logic for these two methods
will be moved to the DataReductionProxyDelegate from the
DataReductionProxyNetworkDelegate. This also involved cleaning up some unit
tests and moving calls to use ProxyDelegate instead of NetworkDelegate where
relevant.

Refactoring proxy related calls from NetworkDelegate to ProxyDelegate

BUG=583369
==========

to

==========
Moving proxy resolution logic out of NetworkDelegate and into ProxyDelegate for
DataReductionProxy

This involves removing two methods off the NetworkDelegate interface (and four
off LayeredNetworkDelegate). DataReductionProxy's logic for these two methods
will be moved to the DataReductionProxyDelegate from the
DataReductionProxyNetworkDelegate. This also involved cleaning up some unit
tests and moving calls to use ProxyDelegate instead of NetworkDelegate where
relevant.

Refactoring proxy related calls from NetworkDelegate to ProxyDelegate

BUG=583369

Committed: https://crrev.com/7de050c68ce2a97c0165d09d06c89b8aff79fb77
Cr-Commit-Position: refs/heads/master@{#376860}
==========

[commit-bot: I haz the power, 2016-02-23 00:11:30]

Patchset 6 (id:??) landed as
https://crrev.com/7de050c68ce2a97c0165d09d06c89b8aff79fb77
Cr-Commit-Position: refs/heads/master@{#376860}

[xunjieli, 2016-02-23 15:07:49]

On 2016/02/23 00:11:30, commit-bot: I haz the power wrote:
> Patchset 6 (id:??) landed as
> https://crrev.com/7de050c68ce2a97c0165d09d06c89b8aff79fb77
> Cr-Commit-Position: refs/heads/master@{#376860}

Hello, looks like your CL might have broken Cronet test. See:
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Dat...

[chromium-reviews, 2016-02-23 16:34:06]

I agree that this looks like it is caused by my CL. I'll look at it today
and should have a fix soon.

On Tue, Feb 23, 2016 at 7:07 AM, <xunjieli@chromium.org> wrote:

> On 2016/02/23 00:11:30, commit-bot: I haz the power wrote:
> > Patchset 6 (id:??) landed as
> > https://crrev.com/7de050c68ce2a97c0165d09d06c89b8aff79fb77
> > Cr-Commit-Position: refs/heads/master@{#376860}
>
> Hello, looks like your CL might have broken Cronet test. See:
>
>
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Dat...
>
>
> https://codereview.chromium.org/1680893002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[RyanSturm, 2016-02-24 02:13:57]

I have a fix for the Cronet test, and it will be in code review tomorrow
morning.

On 2016/02/23 16:34:06, chromium-reviews wrote:
> I agree that this looks like it is caused by my CL. I'll look at it today
> and should have a fix soon.
> 
> On Tue, Feb 23, 2016 at 7:07 AM, <mailto:xunjieli@chromium.org> wrote:
> 
> > On 2016/02/23 00:11:30, commit-bot: I haz the power wrote:
> > > Patchset 6 (id:??) landed as
> > > https://crrev.com/7de050c68ce2a97c0165d09d06c89b8aff79fb77
> > > Cr-Commit-Position: refs/heads/master@{#376860}
> >
> > Hello, looks like your CL might have broken Cronet test. See:
> >
> >
>
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Dat...
> >
> >
> > https://codereview.chromium.org/1680893002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[RyanSturm, 2016-02-24 02:14:00]

I have a fix for the Cronet test, and it will be in code review tomorrow
morning.

On 2016/02/23 16:34:06, chromium-reviews wrote:
> I agree that this looks like it is caused by my CL. I'll look at it today
> and should have a fix soon.
> 
> On Tue, Feb 23, 2016 at 7:07 AM, <mailto:xunjieli@chromium.org> wrote:
> 
> > On 2016/02/23 00:11:30, commit-bot: I haz the power wrote:
> > > Patchset 6 (id:??) landed as
> > > https://crrev.com/7de050c68ce2a97c0165d09d06c89b8aff79fb77
> > > Cr-Commit-Position: refs/heads/master@{#376860}
> >
> > Hello, looks like your CL might have broken Cronet test. See:
> >
> >
>
https://build.chromium.org/p/chromium.android/builders/Android%20Cronet%20Dat...
> >
> >
> > https://codereview.chromium.org/1680893002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[agrieve, 2016-02-25 17:06:53]

A revert of this CL (patchset #6 id:90001) has been created in
https://codereview.chromium.org/1735203002/ by agrieve@chromium.org.

The reason for reverting is: Most likely cause of failing test on bot:
https://bugs.chromium.org/p/chromium/issues/detail?id=589832
.

[RyanSturm, 2016-02-25 17:15:38]

On 2016/02/25 17:06:53, agrieve wrote:
> A revert of this CL (patchset #6 id:90001) has been created in
> https://codereview.chromium.org/1735203002/ by mailto:agrieve@chromium.org.
> 
> The reason for reverting is: Most likely cause of failing test on bot:
> https://bugs.chromium.org/p/chromium/issues/detail?id=589832
> .
Fix for failing cronet test hlin code review here:
https://codereview.chromium.org/1725123005/