Store net::ProxyServer in HttpResponseInfo object

components/cronet/android/cronet_url_request_adapter.cc

Index: components/cronet/android/cronet_url_request_adapter.cc
diff --git a/components/cronet/android/cronet_url_request_adapter.cc b/components/cronet/android/cronet_url_request_adapter.cc
index 0ef0415210e6438cb3ba7e7d5f82375b9b31fbf8..3dd5389a74e1018b1a0e1cab8ec60bc5394af91b 100644
--- a/components/cronet/android/cronet_url_request_adapter.cc
+++ b/components/cronet/android/cronet_url_request_adapter.cc
@@ -23,6 +23,7 @@
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/http/http_util.h"
+#include "net/proxy/proxy_server.h"
 #include "net/quic/core/quic_protocol.h"
 #include "net/ssl/ssl_info.h"
 #include "net/url_request/redirect_info.h"
@@ -216,6 +217,12 @@ void CronetURLRequestAdapter::OnReceivedRedirect(
     bool* defer_redirect) {
   DCHECK(context_->IsOnNetworkThread());
   JNIEnv* env = base::android::AttachCurrentThread();
+
+  const net::HostPortPair host_port_pair =

[mmenke, 2016/10/06 14:58:04]

"host_port_pair" is too generic.  proxy_host_port_pair?  Or even just proxy?

[mmenke, 2016/10/06 14:58:04]

nit:  Prevailing style is not to use const except for class members and
compile-time constants.

+      !request->response_info().proxy_server.is_valid() ||
+              request->response_info().proxy_server.is_direct()
+          ? net::HostPortPair()
+          : request->response_info().proxy_server.host_port_pair();

[mmenke, 2016/10/06 14:58:04]

I think this would be a lot easier to read as:

net::HostPortPair ;;;/
if (/* using proxy checks here */) {
  // Set HostPortPair to something here.
}

   cronet::Java_CronetUrlRequest_onRedirectReceived(
       env, owner_.obj(),
       ConvertUTF8ToJavaString(env, redirect_info.new_url.spec()).obj(),
@@ -227,9 +234,7 @@ void CronetURLRequestAdapter::OnReceivedRedirect(
       ConvertUTF8ToJavaString(env,
                               request->response_info().alpn_negotiated_protocol)
           .obj(),
-      ConvertUTF8ToJavaString(env,
-                              request->response_info().proxy_server.ToString())
-          .obj(),
+      ConvertUTF8ToJavaString(env, host_port_pair.ToString()).obj(),
       request->GetTotalReceivedBytes());
   *defer_redirect = true;
 }
@@ -267,6 +272,11 @@ void CronetURLRequestAdapter::OnResponseStarted(net::URLRequest* request,
     return;
   }
   JNIEnv* env = base::android::AttachCurrentThread();
+  const net::HostPortPair host_port_pair =
+      !request->response_info().proxy_server.is_valid() ||
+              request->response_info().proxy_server.is_direct()
+          ? net::HostPortPair()
+          : request->response_info().proxy_server.host_port_pair();

[mmenke, 2016/10/06 14:58:04]

Same comments as above...Or better, can we make a helper method, rather than
duplicating code?

// Could make this return a variety of things - a string, a
// JavaString, a HostPortPair.  Could also make it take
// response_info() or response_info().proxy_server - I'm fine
// with any of those options.
GetProxy(ResponseInfo) {
  // "Failure" case is generally first, which corresponds to no proxy here
  if (...)
    return HostPortPair();
  return response_info().proxy_server.host_port_pair();
}

   cronet::Java_CronetUrlRequest_onResponseStarted(
       env, owner_.obj(), request->GetResponseCode(),
       ConvertUTF8ToJavaString(env, request->response_headers()->GetStatusText())
@@ -276,9 +286,7 @@ void CronetURLRequestAdapter::OnResponseStarted(net::URLRequest* request,
       ConvertUTF8ToJavaString(env,
                               request->response_info().alpn_negotiated_protocol)
           .obj(),
-      ConvertUTF8ToJavaString(env,
-                              request->response_info().proxy_server.ToString())
-          .obj());
+      ConvertUTF8ToJavaString(env, host_port_pair.ToString()).obj());
 }
 
 void CronetURLRequestAdapter::OnReadCompleted(net::URLRequest* request,