Store Proxy Servers as proxy servers and not GURLs.

Store Proxy Servers as proxy servers and not GURLs.

Chrome Data Reduction Proxy stores proxy URLs as proxy server objects
instead of GURLs. This enables us to support QUIC based proxies.

BUG=454576
Committed: https://crrev.com/0aaf80ac3e3b5b93aea28e6a3acdf108dc1baa03
Cr-Commit-Position: refs/heads/master@{#314835}

[tbansal1, 2015-02-03 00:02:08]

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

[bengr, 2015-02-04 00:49:27]

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
File chrome/browser/net/spdyproxy/data_reduction_proxy_settings_android.cc
(right):

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
chrome/browser/net/spdyproxy/data_reduction_proxy_settings_android.cc:49: return
ConvertUTF8ToJavaString(env, Settings()->params()->origin().ToURI());
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
File
chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest_android.cc
(right):

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest_android.cc:159:
EXPECT_EQ(expected_params_->DefaultOrigin().ToURI(),
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_auth_request_handler_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_auth_request_handler_unittest.cc:183:
net::ProxyServer::FromURI(params->DefaultSSLOrigin(),
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:105:
data_reduction_proxy_type_info.proxy_servers.first.host_port_pair().IsEmpty())
indentation

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:112:
!data_reduction_proxy_type_info.proxy_servers.second.host_port_pair().IsEmpty(),
indent two more and limit to 80 chars.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:170:
!data_reduction_proxy_type_info.proxy_servers.second.host_port_pair().IsEmpty(),
indent two more.

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

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:290:
net::ProxyServer proxy_server = net::ProxyServer::FromURI(
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:293:
return std::string(":65535");
why this port?

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:777:
net::ProxyServer::FromURI(
You should be able to use ProxyServer::ToPacString(). See:
https://code.google.com/p/chromium/codesearch#chromium/src/net/proxy/proxy_se...

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:808:
net::ProxyServer::FromURI(
ToPacString

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc:95:
params()->alt_origin().ToURI(), std::string(),
#include <string>
#include proxy_server.h

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc:165:
EXPECT_EQ(params()->DefaultOrigin(), config()->params()->origin().ToURI());
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc:212:
EXPECT_EQ("", configurator()->fallback_origin());
EXPECT_TRUE(...fallback_origin().empty())

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc:215:
net::ProxyServer::FromURI(configurator()->ssl_origin(),
indentation

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc:86:
std::string proxy = params->origin().ToURI();
#include <string>

#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc:190:
std::string spec = proxy_.GetURL("/").spec();
This seems wrong, but I'm not sure what you're trying to do here. Why won't
ToURI() do what you need?

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc:364:
net::ProxyServer::FromURI(
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:440:
params()->alt_origin().ToURI(),
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc:98:
EXPECT_EQ("", configurator->fallback_origin());
fallback_origin().empty()

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc:253:
&&
!data_reduction_proxy_type_info.proxy_servers.first.host_port_pair().IsEmpty())
{
move && to previous line

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:62:
proxy_origin, net::ProxyServer::SCHEME_HTTP);
#include "net/proxy/proxy_server.h"

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:372:
net::ProxyServer::FromURI("", net::ProxyServer::SCHEME_HTTP);
std::string()

and #include <string>

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:383:
net::ProxyServer::FromURI("", net::ProxyServer::SCHEME_HTTP);
std::string()

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:479:
host_port_pair = proxy_server.host_port_pair();
indentation

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:485:
&delay))
indentation

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h:49:
void set_origin(const net::ProxyServer& origin);
namespace net {
class ProxyServer;
}

[tbansal1, 2015-02-04 22:49:11]

Handled all comments and fixed formatting issues.

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
File chrome/browser/net/spdyproxy/data_reduction_proxy_settings_android.cc
(right):

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
chrome/browser/net/spdyproxy/data_reduction_proxy_settings_android.cc:49: return
ConvertUTF8ToJavaString(env, Settings()->params()->origin().ToURI());
On 2015/02/04 00:49:25, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
File
chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest_android.cc
(right):

https://codereview.chromium.org/863203009/diff/1/chrome/browser/net/spdyproxy...
chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest_android.cc:159:
EXPECT_EQ(expected_params_->DefaultOrigin().ToURI(),
On 2015/02/04 00:49:25, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_auth_request_handler_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_auth_request_handler_unittest.cc:183:
net::ProxyServer::FromURI(params->DefaultSSLOrigin(),
On 2015/02/04 00:49:25, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:105:
data_reduction_proxy_type_info.proxy_servers.first.host_port_pair().IsEmpty())
On 2015/02/04 00:49:25, bengr wrote:
> indentation

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:112:
!data_reduction_proxy_type_info.proxy_servers.second.host_port_pair().IsEmpty(),
On 2015/02/04 00:49:25, bengr wrote:
> indent two more and limit to 80 chars.

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:170:
!data_reduction_proxy_type_info.proxy_servers.second.host_port_pair().IsEmpty(),
On 2015/02/04 00:49:25, bengr wrote:
> indent two more.

Done.

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

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:290:
net::ProxyServer proxy_server = net::ProxyServer::FromURI(
On 2015/02/04 00:49:25, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:293:
return std::string(":65535");
On 2015/02/04 00:49:25, bengr wrote:
> why this port?

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:777:
net::ProxyServer::FromURI(
On 2015/02/04 00:49:25, bengr wrote:
> You should be able to use ProxyServer::ToPacString(). See:
>
https://code.google.com/p/chromium/codesearch#chromium/src/net/proxy/proxy_se...

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc:808:
net::ProxyServer::FromURI(
On 2015/02/04 00:49:25, bengr wrote:
> ToPacString

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc:95:
params()->alt_origin().ToURI(), std::string(),
On 2015/02/04 00:49:26, bengr wrote:
> #include <string>
> #include proxy_server.h

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc:165:
EXPECT_EQ(params()->DefaultOrigin(), config()->params()->origin().ToURI());
On 2015/02/04 00:49:26, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc:212:
EXPECT_EQ("", configurator()->fallback_origin());
On 2015/02/04 00:49:26, bengr wrote:
> EXPECT_TRUE(...fallback_origin().empty())

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc:215:
net::ProxyServer::FromURI(configurator()->ssl_origin(),
On 2015/02/04 00:49:26, bengr wrote:
> indentation

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc:86:
std::string proxy = params->origin().ToURI();
On 2015/02/04 00:49:26, bengr wrote:
> #include <string>
> 
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc:190:
std::string spec = proxy_.GetURL("/").spec();
On 2015/02/04 00:49:26, bengr wrote:
> This seems wrong, but I'm not sure what you're trying to do here. Why won't
> ToURI() do what you need?

proxy_ is EmbeddedTestServer. So, I have to call GetURL() on it to obtain a
GURL. Also, to convert GURL to something that ProxyServer understands, I have to
remove the trailing slash.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc:364:
net::ProxyServer::FromURI(
On 2015/02/04 00:49:26, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc:440:
params()->alt_origin().ToURI(),
On 2015/02/04 00:49:26, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc:98:
EXPECT_EQ("", configurator->fallback_origin());
On 2015/02/04 00:49:26, bengr wrote:
> fallback_origin().empty()

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc:253:
&&
!data_reduction_proxy_type_info.proxy_servers.first.host_port_pair().IsEmpty())
{
On 2015/02/04 00:49:26, bengr wrote:
> move && to previous line

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc:62:
proxy_origin, net::ProxyServer::SCHEME_HTTP);
On 2015/02/04 00:49:26, bengr wrote:
> #include "net/proxy/proxy_server.h"

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:372:
net::ProxyServer::FromURI("", net::ProxyServer::SCHEME_HTTP);
On 2015/02/04 00:49:26, bengr wrote:
> std::string()
> 
> and #include <string>

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:383:
net::ProxyServer::FromURI("", net::ProxyServer::SCHEME_HTTP);
On 2015/02/04 00:49:26, bengr wrote:
> std::string()

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:479:
host_port_pair = proxy_server.host_port_pair();
On 2015/02/04 00:49:26, bengr wrote:
> indentation

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc:485:
&delay))
On 2015/02/04 00:49:26, bengr wrote:
> indentation

Done.

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/common/data_reduction_proxy_params_test_utils.h:49:
void set_origin(const net::ProxyServer& origin);
On 2015/02/04 00:49:26, bengr wrote:
> namespace net {
> class ProxyServer;
> }

Done.

[bengr, 2015-02-05 00:31:08]

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc:190:
std::string spec = proxy_.GetURL("/").spec();
On 2015/02/04 22:49:11, tbansal1 wrote:
> On 2015/02/04 00:49:26, bengr wrote:
> > This seems wrong, but I'm not sure what you're trying to do here. Why won't
> > ToURI() do what you need?
> 
> proxy_ is EmbeddedTestServer. So, I have to call GetURL() on it to obtain a
> GURL. Also, to convert GURL to something that ProxyServer understands, I have
to
> remove the trailing slash.

Use base::TrimString

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc
(right):

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:105:
net::ProxyServer first =  data_reduction_proxy_type_info.proxy_servers.first;
Make this a reference if you want to do this.

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:111:
net::ProxyServer second = data_reduction_proxy_type_info.proxy_servers.second;
This should also be a reference.

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
(right):

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc:252:
net::ProxyServer first = data_reduction_proxy_type_info.proxy_servers.first;
reference

[tbansal1, 2015-02-05 01:21:33]

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc
(right):

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:105:
net::ProxyServer first =  data_reduction_proxy_type_info.proxy_servers.first;
On 2015/02/05 00:31:08, bengr wrote:
> Make this a reference if you want to do this. 

Done.

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol.cc:111:
net::ProxyServer second = data_reduction_proxy_type_info.proxy_servers.second;
On 2015/02/05 00:31:07, bengr wrote:
> This should also be a reference.

Done.

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc
(right):

https://codereview.chromium.org/863203009/diff/40001/components/data_reductio...
components/data_reduction_proxy/core/browser/data_reduction_proxy_usage_stats.cc:252:
net::ProxyServer first = data_reduction_proxy_type_info.proxy_servers.first;
On 2015/02/05 00:31:08, bengr wrote:
> reference

Done.

[bengr, 2015-02-05 01:31:29]

lgtm

[tbansal1, 2015-02-05 01:42:59]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2015-02-05 01:44:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/863203009/60001

[tbansal1, 2015-02-05 01:45:04]

The CQ bit was unchecked by tbansal@chromium.org

[tbansal1, 2015-02-05 01:51:21]

New patchsets have been uploaded after l-g-t-m from bengr@chromium.org

[tbansal1, 2015-02-05 01:54:09]

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
File
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
(right):

https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc:190:
std::string spec = proxy_.GetURL("/").spec();
On 2015/02/05 00:31:07, bengr wrote:
> On 2015/02/04 22:49:11, tbansal1 wrote:
> > On 2015/02/04 00:49:26, bengr wrote:
> > > This seems wrong, but I'm not sure what you're trying to do here. Why
won't
> > > ToURI() do what you need?
> > 
> > proxy_ is EmbeddedTestServer. So, I have to call GetURL() on it to obtain a
> > GURL. Also, to convert GURL to something that ProxyServer understands, I
have
> to
> > remove the trailing slash.
> 
> Use base::TrimString

Done.

[bengr, 2015-02-05 01:57:18]

On 2015/02/05 01:54:09, tbansal1 wrote:
>
https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
> File
>
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/863203009/diff/1/components/data_reduction_pr...
>
components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc:190:
> std::string spec = proxy_.GetURL("/").spec();
> On 2015/02/05 00:31:07, bengr wrote:
> > On 2015/02/04 22:49:11, tbansal1 wrote:
> > > On 2015/02/04 00:49:26, bengr wrote:
> > > > This seems wrong, but I'm not sure what you're trying to do here. Why
> won't
> > > > ToURI() do what you need?
> > > 
> > > proxy_ is EmbeddedTestServer. So, I have to call GetURL() on it to obtain
a
> > > GURL. Also, to convert GURL to something that ProxyServer understands, I
> have
> > to
> > > remove the trailing slash.
> > 
> > Use base::TrimString
> 
> Done.

Still lgtm. Thanks for fixing this.

[tbansal1, 2015-02-05 01:57:36]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2015-02-05 01:58:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/863203009/80001

[commit-bot: I haz the power, 2015-02-05 02:22:04]

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

[commit-bot: I haz the power, 2015-02-05 02:22:05]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[tbansal1, 2015-02-05 17:14:56]

New patchsets have been uploaded after l-g-t-m from bengr@chromium.org

[tbansal1, 2015-02-05 17:18:06]

The CQ bit was checked by tbansal@chromium.org

[commit-bot: I haz the power, 2015-02-05 17:19:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/863203009/100001

[commit-bot: I haz the power, 2015-02-05 18:19:24]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-05 18:20:20]

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