Make the proxy server HostPortPair available in document_state so that

Make the proxy server HostPortPair available in document_state so that
we can determine whether the data_reduction_proxy was used or not for
PLT UMA.

BUG=374264
Committed: https://crrev.com/ac77ecc781ff1860e3344b99d6f5a8e9d672dd73
Cr-Commit-Position: refs/heads/master@{#302150}

Committed: https://crrev.com/bdcd2a7aff20eca5832636c4b78df633cab6c61e
Cr-Commit-Position: refs/heads/master@{#302280}

[jeremyim, 2014-10-30 00:01:12]

jeremyim@chromium.org changed reviewers:
+ creis@chromium.org, mmenke@chromium.org, tsepez@chromium.org

[jeremyim, 2014-10-30 00:01:13]

mmenke => content/browser/loader/resource_loader.cc
creis => content/child* content/public/* content/renderer/*
tsepez => content/common/resource_messages.h

[mmenke, 2014-10-30 15:17:14]

On 2014/10/30 00:01:13, jeremyim wrote:
> mmenke => content/browser/loader/resource_loader.cc
> creis => content/child* content/public/* content/renderer/*
> tsepez => content/common/resource_messages.h

ResourceLoader LGTM.

[Tom Sepez, 2014-10-30 16:45:42]

Messages LGTM, though I found the name proxy_server confusing, expecting to see
some sort of more complicated object here when what you're really passing is
proxy_server's host port pair.  Maybe call all these proxy_server_hostport?

[Charlie Reis, 2014-10-30 18:05:32]

On 2014/10/30 16:45:42, Tom Sepez wrote:
> Messages LGTM, though I found the name proxy_server confusing, expecting to
see
> some sort of more complicated object here when what you're really passing is
> proxy_server's host port pair.  Maybe call all these proxy_server_hostport?

+1 to being confused at first, but it looks like it's just following the
HttpResponseInfo names.

https://codereview.chromium.org/686313002/diff/1/content/public/common/resour...
File content/public/common/resource_response_info.h (right):

https://codereview.chromium.org/686313002/diff/1/content/public/common/resour...
content/public/common/resource_response_info.h:98: net::HostPortPair
proxy_server;
It wasn't clear to me whether we need both this and |was_fetched_via_proxy|. 
Looks like they're grouped together with a comment in HttpResponseInfo.  Can we
do something similar here?

[jeremyim, 2014-10-30 19:00:36]

Also updated resource_response.cc as per the new comment at the top of
resource_response_info.h

https://codereview.chromium.org/686313002/diff/1/content/public/common/resour...
File content/public/common/resource_response_info.h (right):

https://codereview.chromium.org/686313002/diff/1/content/public/common/resour...
content/public/common/resource_response_info.h:98: net::HostPortPair
proxy_server;
On 2014/10/30 18:05:32, Charlie Reis wrote:
> It wasn't clear to me whether we need both this and |was_fetched_via_proxy|. 
> Looks like they're grouped together with a comment in HttpResponseInfo.  Can
we
> do something similar here?

Done.

[Charlie Reis, 2014-10-30 19:19:36]

LGTM with nit

https://codereview.chromium.org/686313002/diff/40001/content/public/common/re...
File content/public/common/resource_response_info.h (right):

https://codereview.chromium.org/686313002/diff/40001/content/public/common/re...
content/public/common/resource_response_info.h:97: // true, |proxy_server|
contains the name of the proxy server that was used
nit: End with a period.

[jeremyim, 2014-10-30 19:52:23]

The CQ bit was checked by jeremyim@chromium.org

[commit-bot: I haz the power, 2014-10-30 19:53:24]

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

[commit-bot: I haz the power, 2014-10-30 20:48:55]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-30 20:49:36]

Patchset 4 (id:??) landed as
https://crrev.com/ac77ecc781ff1860e3344b99d6f5a8e9d672dd73
Cr-Commit-Position: refs/heads/master@{#302150}

[Mike Wittman, 2014-10-31 01:23:21]

Speculatively reverted in e486fdebd339 under suspicion that it's causing a flaky
failure on Mac ASan / Mac ASan 64:

[ RUN      ] ResourceDispatcherHostBrowserTest.CrossSiteNavigationErrorPage
../../content/browser/loader/resource_dispatcher_host_browsertest.cc:363:
Failure
Value of: GetCookies(url)
  Actual: ""
Expected: "onunloadCookie=foo"
[19619:13319:1030/151517:6577359656556:WARNING:channel.cc(519)] Failed to send
message to remove remote message pipe endpoint ack (local ID 1, remote ID 1)
[19619:13319:1030/151517:6577359850720:WARNING:channel.cc(273)] RawChannel write
error
[19613:27395:1030/151517:6577448612243:WARNING:channel.cc(519)] Failed to send
message to remove remote message pipe endpoint ack (local ID 1, remote ID 1)
[19613:27395:1030/151517:6577448854494:WARNING:channel.cc(273)] RawChannel write
error
[  FAILED  ] ResourceDispatcherHostBrowserTest.CrossSiteNavigationErrorPage,
where TypeParam =  and GetParam() =  (3480 ms)

See
https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%2064%20Tests...
and
https://build.chromium.org/p/chromium.memory/builders/Mac%20ASan%20Tests%20%2...
for example.

[bengr, 2014-10-31 15:59:52]

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

[bengr, 2014-10-31 15:59:53]

I don't see anything obvious that's broken in this CL. IPC is always tricky
though.

https://codereview.chromium.org/686313002/diff/60001/content/child/weburlresp...
File content/child/weburlresponse_extradata_impl.h (right):

https://codereview.chromium.org/686313002/diff/60001/content/child/weburlresp...
content/child/weburlresponse_extradata_impl.h:43: void
set_proxy_server(net::HostPortPair proxy_server) {
This should probably take a const net::HostPortPair&

https://codereview.chromium.org/686313002/diff/60001/content/public/renderer/...
File content/public/renderer/document_state.h (right):

https://codereview.chromium.org/686313002/diff/60001/content/public/renderer/...
content/public/renderer/document_state.h:165: void
set_proxy_server(net::HostPortPair proxy_server) {
Likewise, const &

[jeremyim, 2014-10-31 16:36:39]

The CQ bit was checked by jeremyim@chromium.org

[commit-bot: I haz the power, 2014-10-31 16:37:23]

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

[commit-bot: I haz the power, 2014-10-31 16:38:19]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-31 16:38:51]

Patchset 4 (id:??) landed as
https://crrev.com/bdcd2a7aff20eca5832636c4b78df633cab6c61e
Cr-Commit-Position: refs/heads/master@{#302280}