Remove explicit JNI references by adding UrlRequest.readFromUploadChannel callback.

components/cronet/android/org_chromium_net_UrlRequest.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/org_chromium_net_UrlRequest.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/macros.h"
 #include "components/cronet/android/url_request_context_peer.h"
@@ skipping 63 matching lines @@
       cronet::Java_UrlRequest_onBytesRead(env, owner_, bytebuf);
       env->DeleteLocalRef(bytebuf);
     }
   }
 
   virtual void OnRequestFinished(URLRequestPeer* request) OVERRIDE {
     JNIEnv* env = base::android::AttachCurrentThread();
     cronet::Java_UrlRequest_finish(env, owner_);
   }
 
+  virtual int OnReadUploadData(net::IOBuffer* buf, int buf_length) {

[mmenke, 2014/08/05 19:32:41]

OVERRIDE

[mef, 2014/08/05 20:44:28]

Done.

+    JNIEnv* env = base::android::AttachCurrentThread();
+    jobject jbuf = env->NewDirectByteBuffer(

[mmenke, 2014/08/05 19:32:41]

jbuf seems to violate the C++ style guide - I'd suggest just writting out
java_buffer or java_byte_buffer, or somesuch.

[mef, 2014/08/05 20:44:28]

Done.

+        reinterpret_cast<void*>(buf->data()), buf_length);
+    jint bytes_read =
+        cronet::Java_UrlRequest_onReadUploadChannel(env, owner_, jbuf);
+    env->DeleteLocalRef(jbuf);

[mmenke, 2014/08/05 19:32:41]

Is this call actually needed?  Seems like it's not done in a lot of code, though
we do this all over the place in this file.

Fine with it here, just wondering.

[mef, 2014/08/05 20:44:28]

IIUIC this is explicit refcounting, so we have to release it, unless we hold on
to the reference or use some kind of scoped ref counter (I'll check if there is
one). 
Could you point to some places where it is not done for newly allocated objects?

[mmenke, 2014/08/05 21:08:02]

Hrm...I missed that ,ost use ScopedJavaLocalRef, which is probably what we
should use as well.  And some add them to other objects, which I guess take
ownership of the reference?  Not sure.

And that just leaves...um...net_string_util_icu_alternatives_android.  Oops.

[mef, 2014/08/05 21:38:45]

Done.

+    return bytes_read;
+  }
+
  protected:
   virtual ~JniURLRequestPeerDelegate() {
     JNIEnv* env = base::android::AttachCurrentThread();
     env->DeleteGlobalRef(owner_);
   }
 
  private:
   jobject owner_;
 
   DISALLOW_COPY_AND_ASSIGN(JniURLRequestPeerDelegate);
@@ skipping 75 matching lines @@
                                 size);
       env->ReleaseByteArrayElements(content, content_bytes, 0);
     }
   }
 }
 
 static void SetUploadChannel(JNIEnv* env,
                              jobject object,
                              jlong urlRequestPeer,
                              jstring content_type,
-                             jobject content,
                              jlong content_length) {
   URLRequestPeer* request = reinterpret_cast<URLRequestPeer*>(urlRequestPeer);
   SetPostContentType(env, request, content_type);
 
-  request->SetUploadChannel(env, content, content_length);
+  request->SetUploadChannel(env, content_length);
 }
 
 
 /* synchronized */
 static void Start(JNIEnv* env, jobject object, jlong urlRequestPeer) {
   URLRequestPeer* request = reinterpret_cast<URLRequestPeer*>(urlRequestPeer);
   if (request != NULL) {
     request->Start();
   }
 }
@@ skipping 139 matching lines @@
 
   // Some implementations (notably HttpURLConnection) include a mapping for the
   // null key; in HTTP's case, this maps to the HTTP status line.
   ScopedJavaLocalRef<jstring> status_line =
       ConvertUTF8ToJavaString(env, headers->GetStatusLine());
   Java_UrlRequest_onAppendResponseHeader(
       env, object, headersMap, NULL, status_line.Release());
 }
 
 }  // namespace cronet