Store net::ProxyServer in HttpResponseInfo object

components/cronet/android/cronet_url_request_adapter.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/cronet/android/cronet_url_request_adapter.h"
 
 #include <limits>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "components/cronet/android/cronet_url_request_context_adapter.h"
 #include "components/cronet/android/io_buffer_with_byte_buffer.h"
 #include "components/cronet/android/url_request_error.h"
 #include "jni/CronetUrlRequest_jni.h"
 #include "net/base/load_flags.h"
 #include "net/base/load_states.h"
 #include "net/base/net_errors.h"
 #include "net/base/request_priority.h"
 #include "net/cert/cert_status_flags.h"
 #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"
 #include "net/url_request/url_request_context.h"
 
 using base::android::ConvertUTF8ToJavaString;
 using base::android::JavaParamRef;
 
 namespace cronet {
 
@@ skipping 173 matching lines @@
       FROM_HERE, base::Bind(&CronetURLRequestAdapter::DestroyOnNetworkThread,
                             base::Unretained(this), jsend_on_canceled));
 }
 
 void CronetURLRequestAdapter::OnReceivedRedirect(
     net::URLRequest* request,
     const net::RedirectInfo& redirect_info,
     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(),
       redirect_info.status_code,
       ConvertUTF8ToJavaString(env, request->response_headers()->GetStatusText())
           .obj(),
       GetResponseHeaders(env).obj(),
       request->response_info().was_cached ? JNI_TRUE : JNI_FALSE,
       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;
 }
 
 void CronetURLRequestAdapter::OnCertificateRequested(
     net::URLRequest* request,
     net::SSLCertRequestInfo* cert_request_info) {
   DCHECK(context_->IsOnNetworkThread());
   // Cronet does not support client certificates.
   request->ContinueWithCertificate(nullptr, nullptr);
@@ skipping 17 matching lines @@
 void CronetURLRequestAdapter::OnResponseStarted(net::URLRequest* request,
                                                 int net_error) {
   DCHECK_NE(net::ERR_IO_PENDING, net_error);
   DCHECK(context_->IsOnNetworkThread());
 
   if (net_error != net::OK) {
     ReportError(request, net_error);
     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())
           .obj(),
       GetResponseHeaders(env).obj(),
       request->response_info().was_cached ? JNI_TRUE : JNI_FALSE,
       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,
                                               int bytes_read) {
   DCHECK(context_->IsOnNetworkThread());
 
   if (bytes_read < 0) {
     ReportError(request, bytes_read);
     return;
   }
@@ skipping 140 matching lines @@
       ConvertTime(base::TimeTicks::Now(), start_ticks, start_time),
       // TODO(mgersh): report total bytes sent
       metrics.socket_reused, 0, url_request_->GetTotalReceivedBytes());
 }
 
 net::URLRequest* CronetURLRequestAdapter::GetURLRequestForTesting() {
   return url_request_.get();
 }
 
 }  // namespace cronet