Remove explicit JNI references by adding UrlRequest.readFromUploadChannel callback.

components/cronet/android/wrapped_channel_upload_element_reader.cc

Index: components/cronet/android/wrapped_channel_upload_element_reader.cc
diff --git a/components/cronet/android/wrapped_channel_upload_element_reader.cc b/components/cronet/android/wrapped_channel_upload_element_reader.cc
index 6628591722f8c26d478a589230d29462d6313be0..1828a572fcf2f9ad21d37f750ee3aa9203954ca5 100644
--- a/components/cronet/android/wrapped_channel_upload_element_reader.cc
+++ b/components/cronet/android/wrapped_channel_upload_element_reader.cc
@@ -11,46 +11,16 @@
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 
-namespace {
-jclass g_class_readablebytechannel;
-jclass g_class_channel;
-jmethodID g_method_read;
-jmethodID g_method_close;
-}  //namespace
-
 namespace cronet {
 
-bool WrappedChannelRegisterJni(JNIEnv* env) {
-  g_class_readablebytechannel = static_cast<jclass>(env->NewGlobalRef(
-      env->FindClass("java/nio/channels/ReadableByteChannel")));
-  // TODO(mef): Per previous discussions the best way to do this is to have a
-  // cronet-specific java wrapper that exposes necessary methods annotated as
-  // @AccessedByNative and use jni generator to generate a wrapper method for
-  // that.
-  g_method_read = env->GetMethodID(
-      g_class_readablebytechannel, "read", "(Ljava/nio/ByteBuffer;)I");
-
-  // Due to a bug in the version of ART that shipped with 4.4, we're looking up
-  // the close() method on the base interface. See b/15651032 for details.
-  g_class_channel = static_cast<jclass>(env->NewGlobalRef(
-      env->FindClass("java/nio/channels/Channel")));
-  g_method_close = env->GetMethodID(g_class_channel, "close", "()V");
-  if (!g_class_readablebytechannel || !g_method_read || !g_method_close) {
-    return false;
-  }
-  return true;
-}
-
-WrappedChannelElementReader::WrappedChannelElementReader(JNIEnv* env,
-                                                         jobject channel,
-                                                         uint64 length)
-    : length_(length), offset_(0) {
-  channel_ = env->NewGlobalRef(channel);
+WrappedChannelElementReader::WrappedChannelElementReader(
+    JNIEnv* env,

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

I don't think we need env, or the dependency on jni.h any more.

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

Done.

+    scoped_refptr<URLRequestPeer::URLRequestPeerDelegate> delegate,
+    uint64 length)
+    : length_(length), offset_(0), delegate_(delegate) {
 }
 
 WrappedChannelElementReader::~WrappedChannelElementReader() {
-  JNIEnv* env = base::android::AttachCurrentThread();
-  env->DeleteGlobalRef(channel_);
 }
 
 int WrappedChannelElementReader::Init(const net::CompletionCallback& callback) {
@@ -74,19 +44,11 @@ int WrappedChannelElementReader::Read(net::IOBuffer* buf,
                                       int buf_length,
                                       const net::CompletionCallback& callback) {
   DCHECK(!callback.is_null());
-  JNIEnv* env = base::android::AttachCurrentThread();
-  jobject jbuf = env->NewDirectByteBuffer(reinterpret_cast<void*>(buf->data()),
-                                          buf_length);
-  jint bytes_read = env->CallIntMethod(channel_, g_method_read, jbuf);
-  base::android::CheckException(env);
-
-  env->DeleteLocalRef(jbuf);
-  if (bytes_read <= 0) {

[mef, 2014/08/05 17:18:39]

I imagine that |bytes_read| could temporarily be 0 if data is not ready yet.

-    env->CallVoidMethod(channel_, g_method_close);

[mef, 2014/08/05 17:18:39]

Charles, why do we need to close the channel here?

[Charles, 2014/08/05 18:33:32]

Closing the channel is required because you need to ensure that if it's a file,
the bytes are written to disk. 

-    base::android::CheckException(env);
-    return bytes_read;
-  }
-  offset_ += bytes_read;
+  DCHECK(delegate_);
+  // TODO(mef): Post the read to file thread.
+  int bytes_read = delegate_->OnReadUploadData(buf, buf_length);

[mef, 2014/08/05 17:18:39]

Should I post it to file thread in this CL? 
Also, should I post it here or inside the delegate?

+  if (bytes_read > 0)
+    offset_ += bytes_read;
   return bytes_read;

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

BUG:  The return value is bytes read or a network error code.  In the Java, you
return -1 on some errors, or a Java error code instead.  Returning -1, in
particular, means ERR_IO_PENDING, which I don't think is what we want to be
returning on errors.

This looks to be a bug in the old code as well.

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

Done.

 }