Pass JavaRef to Java methods in cronet.

Update code in cronet to use JavaRef when calling Java methods via JNI,
instead of passing bare jobject. Various function parameter types are
converted from jobject to JavaRef to enable calls to obj() higher up the
call chain to be removed.

BUG=506850

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: Id58e5b96c08df22f81f1a205cff877523ff2dad0
Reviewed-on: https://chromium-review.googlesource.com/591617Reviewed-by: Paul Jensen <pauljensen@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490785}

components/cronet/android/cronet_bidirectional_stream_adapter.cc

@@ -33,6 +33,7 @@
 
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ConvertJavaStringToUTF8;
+using base::android::JavaRef;
 using base::android::ScopedJavaLocalRef;
 
 namespace cronet {
@@ -52,14 +53,15 @@ size_t SafeGetArrayLength(JNIEnv* env, JavaArrayType jarray) {
 
 }  // namespace
 
-PendingWriteData::PendingWriteData(JNIEnv* env,
-                                   jobjectArray jwrite_buffer_list,
-                                   jintArray jwrite_buffer_pos_list,
-                                   jintArray jwrite_buffer_limit_list,
-                                   jboolean jwrite_end_of_stream) {
-  this->jwrite_buffer_list.Reset(env, jwrite_buffer_list);
-  this->jwrite_buffer_pos_list.Reset(env, jwrite_buffer_pos_list);
-  this->jwrite_buffer_limit_list.Reset(env, jwrite_buffer_limit_list);
+PendingWriteData::PendingWriteData(
+    JNIEnv* env,
+    const JavaRef<jobjectArray>& jwrite_buffer_list,
+    const JavaRef<jintArray>& jwrite_buffer_pos_list,
+    const JavaRef<jintArray>& jwrite_buffer_limit_list,
+    jboolean jwrite_end_of_stream) {
+  this->jwrite_buffer_list.Reset(jwrite_buffer_list);
+  this->jwrite_buffer_pos_list.Reset(jwrite_buffer_pos_list);
+  this->jwrite_buffer_limit_list.Reset(jwrite_buffer_limit_list);
   this->jwrite_end_of_stream = jwrite_end_of_stream;
 }
 
@@ -200,8 +202,8 @@ jboolean CronetBidirectionalStreamAdapter::WritevData(
 
   std::unique_ptr<PendingWriteData> pending_write_data;
   pending_write_data.reset(
-      new PendingWriteData(env, jbyte_buffers.obj(), jbyte_buffers_pos.obj(),
-                           jbyte_buffers_limit.obj(), jend_of_stream));
+      new PendingWriteData(env, jbyte_buffers, jbyte_buffers_pos,
+                           jbyte_buffers_limit, jend_of_stream));
   for (size_t i = 0; i < buffers_array_size; ++i) {
     ScopedJavaLocalRef<jobject> jbuffer(
         env, env->GetObjectArrayElement(
@@ -250,7 +252,7 @@ void CronetBidirectionalStreamAdapter::OnStreamReady(
   DCHECK(context_->IsOnNetworkThread());
   JNIEnv* env = base::android::AttachCurrentThread();
   cronet::Java_CronetBidirectionalStream_onStreamReady(
-      env, owner_.obj(), request_headers_sent ? JNI_TRUE : JNI_FALSE);
+      env, owner_, request_headers_sent ? JNI_TRUE : JNI_FALSE);
 }
 
 void CronetBidirectionalStreamAdapter::OnHeadersReceived(
@@ -276,9 +278,8 @@ void CronetBidirectionalStreamAdapter::OnHeadersReceived(
   }
 
   cronet::Java_CronetBidirectionalStream_onResponseHeadersReceived(
-      env, owner_.obj(), http_status_code,
-      ConvertUTF8ToJavaString(env, protocol).obj(),
-      GetHeadersArray(env, response_headers).obj(),
+      env, owner_, http_status_code, ConvertUTF8ToJavaString(env, protocol),
+      GetHeadersArray(env, response_headers),
       bidi_stream_->GetTotalReceivedBytes());
 }
 
@@ -286,7 +287,7 @@ void CronetBidirectionalStreamAdapter::OnDataRead(int bytes_read) {
   DCHECK(context_->IsOnNetworkThread());
   JNIEnv* env = base::android::AttachCurrentThread();
   cronet::Java_CronetBidirectionalStream_onReadCompleted(
-      env, owner_.obj(), read_buffer_->byte_buffer(), bytes_read,
+      env, owner_, read_buffer_->byte_buffer(), bytes_read,
       read_buffer_->initial_position(), read_buffer_->initial_limit(),
       bidi_stream_->GetTotalReceivedBytes());
   // Free the read buffer. This lets the Java ByteBuffer be freed, if the
@@ -301,9 +302,9 @@ void CronetBidirectionalStreamAdapter::OnDataSent() {
   JNIEnv* env = base::android::AttachCurrentThread();
   // Call into Java.
   cronet::Java_CronetBidirectionalStream_onWritevCompleted(
-      env, owner_.obj(), pending_write_data_->jwrite_buffer_list.obj(),
-      pending_write_data_->jwrite_buffer_pos_list.obj(),
-      pending_write_data_->jwrite_buffer_limit_list.obj(),
+      env, owner_, pending_write_data_->jwrite_buffer_list,
+      pending_write_data_->jwrite_buffer_pos_list,
+      pending_write_data_->jwrite_buffer_limit_list,
       pending_write_data_->jwrite_end_of_stream);
   // Free the java objects. This lets the Java ByteBuffers be freed, if the
   // embedder releases it, too.
@@ -315,7 +316,7 @@ void CronetBidirectionalStreamAdapter::OnTrailersReceived(
   DCHECK(context_->IsOnNetworkThread());
   JNIEnv* env = base::android::AttachCurrentThread();
   cronet::Java_CronetBidirectionalStream_onResponseTrailersReceived(
-      env, owner_.obj(), GetHeadersArray(env, response_trailers).obj());
+      env, owner_, GetHeadersArray(env, response_trailers));
 }
 
 void CronetBidirectionalStreamAdapter::OnFailed(int error) {
@@ -325,9 +326,9 @@ void CronetBidirectionalStreamAdapter::OnFailed(int error) {
   net::NetErrorDetails net_error_details;
   bidi_stream_->PopulateNetErrorDetails(&net_error_details);
   cronet::Java_CronetBidirectionalStream_onError(
-      env, owner_.obj(), NetErrorToUrlRequestError(error), error,
+      env, owner_, NetErrorToUrlRequestError(error), error,
       net_error_details.quic_connection_error,
-      ConvertUTF8ToJavaString(env, net::ErrorToString(error)).obj(),
+      ConvertUTF8ToJavaString(env, net::ErrorToString(error)),
       bidi_stream_->GetTotalReceivedBytes());
 }
 
@@ -409,7 +410,7 @@ void CronetBidirectionalStreamAdapter::DestroyOnNetworkThread(
   DCHECK(context_->IsOnNetworkThread());
   if (send_on_canceled) {
     JNIEnv* env = base::android::AttachCurrentThread();
-    cronet::Java_CronetBidirectionalStream_onCanceled(env, owner_.obj());
+    cronet::Java_CronetBidirectionalStream_onCanceled(env, owner_);
   }
   MaybeReportMetrics();
   delete this;
@@ -441,7 +442,7 @@ void CronetBidirectionalStreamAdapter::MaybeReportMetrics() {
   base::Time start_time = load_timing_info.request_start_time;
   base::TimeTicks start_ticks = load_timing_info.request_start;
   cronet::Java_CronetBidirectionalStream_onMetricsCollected(
-      env, owner_.obj(),
+      env, owner_,
       metrics_util::ConvertTime(start_ticks, start_ticks, start_time),
       metrics_util::ConvertTime(load_timing_info.connect_timing.dns_start,
                                 start_ticks, start_time),

components/cronet/android/cronet_bidirectional_stream_adapter.h

@@ -31,11 +31,12 @@ class IOBufferWithByteBuffer;
 // Convenient wrapper to hold Java references and data to represent the pending
 // data to be written.
 struct PendingWriteData {
-  PendingWriteData(JNIEnv* env,
-                   jobjectArray jwrite_buffer_list,
-                   jintArray jwrite_buffer_pos_list,
-                   jintArray jwrite_buffer_limit_list,
-                   jboolean jwrite_end_of_stream);
+  PendingWriteData(
+      JNIEnv* env,
+      const base::android::JavaRef<jobjectArray>& jwrite_buffer_list,
+      const base::android::JavaRef<jintArray>& jwrite_buffer_pos_list,
+      const base::android::JavaRef<jintArray>& jwrite_buffer_limit_list,
+      jboolean jwrite_end_of_stream);
   ~PendingWriteData();
 
   // Arguments passed in from Java. Retain a global ref so they won't get GC-ed

components/cronet/android/cronet_upload_data_stream_adapter.cc

@@ -52,8 +52,7 @@ void CronetUploadDataStreamAdapter::Read(net::IOBuffer* buffer, int buf_len) {
   JNIEnv* env = base::android::AttachCurrentThread();
   base::android::ScopedJavaLocalRef<jobject> java_buffer(
       env, env->NewDirectByteBuffer(buffer->data(), buf_len));
-  Java_CronetUploadDataStream_readData(env, jupload_data_stream_,
-                                       java_buffer.obj());
+  Java_CronetUploadDataStream_readData(env, jupload_data_stream_, java_buffer);
 }
 
 void CronetUploadDataStreamAdapter::Rewind() {

components/cronet/android/cronet_url_request_adapter.cc

@@ -216,17 +216,15 @@ void CronetURLRequestAdapter::OnReceivedRedirect(
   DCHECK(context_->IsOnNetworkThread());
   JNIEnv* env = base::android::AttachCurrentThread();
   cronet::Java_CronetUrlRequest_onRedirectReceived(
-      env, owner_.obj(),
-      ConvertUTF8ToJavaString(env, redirect_info.new_url.spec()).obj(),
+      env, owner_, ConvertUTF8ToJavaString(env, redirect_info.new_url.spec()),
       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, GetProxy(request->response_info())).obj(),
+                              request->response_headers()->GetStatusText()),
+      GetResponseHeaders(env),
+      request->response_info().was_cached ? JNI_TRUE : JNI_FALSE,
+      ConvertUTF8ToJavaString(
+          env, request->response_info().alpn_negotiated_protocol),
+      ConvertUTF8ToJavaString(env, GetProxy(request->response_info())),
       request->GetTotalReceivedBytes());
   *defer_redirect = true;
 }
@@ -260,15 +258,14 @@ void CronetURLRequestAdapter::OnResponseStarted(net::URLRequest* request,
   }
   JNIEnv* env = base::android::AttachCurrentThread();
   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,
+      env, owner_, request->GetResponseCode(),
       ConvertUTF8ToJavaString(env,
-                              request->response_info().alpn_negotiated_protocol)
-          .obj(),
-      ConvertUTF8ToJavaString(env, GetProxy(request->response_info())).obj());
+                              request->response_headers()->GetStatusText()),
+      GetResponseHeaders(env),
+      request->response_info().was_cached ? JNI_TRUE : JNI_FALSE,
+      ConvertUTF8ToJavaString(
+          env, request->response_info().alpn_negotiated_protocol),
+      ConvertUTF8ToJavaString(env, GetProxy(request->response_info())));
 }
 
 void CronetURLRequestAdapter::OnReadCompleted(net::URLRequest* request,
@@ -284,11 +281,11 @@ void CronetURLRequestAdapter::OnReadCompleted(net::URLRequest* request,
     JNIEnv* env = base::android::AttachCurrentThread();
     MaybeReportMetrics(env);
     cronet::Java_CronetUrlRequest_onSucceeded(
-        env, owner_.obj(), url_request_->GetTotalReceivedBytes());
+        env, owner_, url_request_->GetTotalReceivedBytes());
   } else {
     JNIEnv* env = base::android::AttachCurrentThread();
     cronet::Java_CronetUrlRequest_onReadCompleted(
-        env, owner_.obj(), read_buffer_->byte_buffer(), bytes_read,
+        env, owner_, read_buffer_->byte_buffer(), bytes_read,
         read_buffer_->initial_position(), read_buffer_->initial_limit(),
         request->GetTotalReceivedBytes());
     // Free the read buffer. This lets the Java ByteBuffer be freed, if the
@@ -325,8 +322,8 @@ void CronetURLRequestAdapter::GetStatusOnNetworkThread(
   // See crbug.com/606872.
   if (url_request_)
     status = url_request_->GetLoadState().state;
-  cronet::Java_CronetUrlRequest_onStatus(env, owner_.obj(),
-                                         status_listener_ref.obj(), status);
+  cronet::Java_CronetUrlRequest_onStatus(env, owner_, status_listener_ref,
+                                         status);
 }
 
 base::android::ScopedJavaLocalRef<jobjectArray>
@@ -375,8 +372,8 @@ void CronetURLRequestAdapter::DestroyOnNetworkThread(bool send_on_canceled) {
   JNIEnv* env = base::android::AttachCurrentThread();
   MaybeReportMetrics(env);
   if (send_on_canceled)
-    cronet::Java_CronetUrlRequest_onCanceled(env, owner_.obj());
-  cronet::Java_CronetUrlRequest_onNativeAdapterDestroyed(env, owner_.obj());
+    cronet::Java_CronetUrlRequest_onCanceled(env, owner_);
+  cronet::Java_CronetUrlRequest_onNativeAdapterDestroyed(env, owner_);
   delete this;
 }
 
@@ -392,9 +389,9 @@ void CronetURLRequestAdapter::ReportError(net::URLRequest* request,
           << " on chromium request: " << initial_url_.possibly_invalid_spec();
   JNIEnv* env = base::android::AttachCurrentThread();
   cronet::Java_CronetUrlRequest_onError(
-      env, owner_.obj(), NetErrorToUrlRequestError(net_error), net_error,
+      env, owner_, NetErrorToUrlRequestError(net_error), net_error,
       net_error_details.quic_connection_error,
-      ConvertUTF8ToJavaString(env, net::ErrorToString(net_error)).obj(),
+      ConvertUTF8ToJavaString(env, net::ErrorToString(net_error)),
       request->GetTotalReceivedBytes());
 }
 
@@ -412,7 +409,7 @@ void CronetURLRequestAdapter::MaybeReportMetrics(JNIEnv* env) {
   base::Time start_time = metrics.request_start_time;
   base::TimeTicks start_ticks = metrics.request_start;
   Java_CronetUrlRequest_onMetricsCollected(
-      env, owner_.obj(),
+      env, owner_,
       metrics_util::ConvertTime(start_ticks, start_ticks, start_time),
       metrics_util::ConvertTime(metrics.connect_timing.dns_start, start_ticks,
                                 start_time),

components/cronet/android/cronet_url_request_context_adapter.cc

@@ -953,8 +953,8 @@ void CronetURLRequestContextAdapter::GetCertVerifierDataOnNetworkThread() {
   }
   JNIEnv* env = base::android::AttachCurrentThread();
   Java_CronetUrlRequestContext_onGetCertVerifierData(
-      env, jcronet_url_request_context_.obj(),
-      base::android::ConvertUTF8ToJavaString(env, encoded_data).obj());
+      env, jcronet_url_request_context_,
+      base::android::ConvertUTF8ToJavaString(env, encoded_data));
 }
 
 int CronetURLRequestContextAdapter::default_load_flags() const {
@@ -975,7 +975,7 @@ void CronetURLRequestContextAdapter::OnEffectiveConnectionTypeChanged(
     net::EffectiveConnectionType effective_connection_type) {
   DCHECK(GetNetworkTaskRunner()->BelongsToCurrentThread());
   Java_CronetUrlRequestContext_onEffectiveConnectionTypeChanged(
-      base::android::AttachCurrentThread(), jcronet_url_request_context_.obj(),
+      base::android::AttachCurrentThread(), jcronet_url_request_context_,
       effective_connection_type);
 }
 
@@ -994,7 +994,7 @@ void CronetURLRequestContextAdapter::OnRTTOrThroughputEstimatesComputed(
           : INT32_MAX;
 
   Java_CronetUrlRequestContext_onRTTOrThroughputEstimatesComputed(
-      base::android::AttachCurrentThread(), jcronet_url_request_context_.obj(),
+      base::android::AttachCurrentThread(), jcronet_url_request_context_,
       http_rtt_ms, transport_rtt_ms, downstream_throughput_kbps);
 }
 
@@ -1082,7 +1082,7 @@ void CronetURLRequestContextAdapter::StopNetLogOnNetworkThread() {
 
 void CronetURLRequestContextAdapter::StopNetLogCompleted() {
   Java_CronetUrlRequestContext_stopNetLogCompleted(
-      base::android::AttachCurrentThread(), jcronet_url_request_context_.obj());
+      base::android::AttachCurrentThread(), jcronet_url_request_context_);
 }
 
 std::unique_ptr<base::DictionaryValue>

components/cronet/android/io_buffer_with_byte_buffer.h

@@ -33,7 +33,9 @@ class IOBufferWithByteBuffer : public net::WrappedIOBuffer {
   jint initial_position() const { return initial_position_; }
   jint initial_limit() const { return initial_limit_; }
 
-  jobject byte_buffer() const { return byte_buffer_.obj(); }
+  const base::android::JavaRef<jobject>& byte_buffer() const {
+    return byte_buffer_;
+  }
 
  private:
   ~IOBufferWithByteBuffer() override;

components/cronet/android/test/test_upload_data_stream_handler.cc

@@ -172,7 +172,7 @@ void TestUploadDataStreamHandler::NotifyJavaReadCompleted() {
     data_read = std::string(read_buffer_->data(), bytes_read_);
   cronet::Java_TestUploadDataStreamHandler_onReadCompleted(
       env, jtest_upload_data_stream_handler_, bytes_read_,
-      base::android::ConvertUTF8ToJavaString(env, data_read).obj());
+      base::android::ConvertUTF8ToJavaString(env, data_read));
 }
 
 static jlong CreateTestUploadDataStreamHandler(