Remove explicit JNI references by adding UrlRequest.readFromUploadChannel callback.

BUG=390267

Review URL: https://codereview.chromium.org/442803002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288139 0039d316-1c4b-4281-b951-d872f2087c98

components/cronet/android/cronet_jni.cc

@@ -8,7 +8,6 @@
 #include "base/at_exit.h"
 #include "components/cronet/android/org_chromium_net_UrlRequest.h"
 #include "components/cronet/android/org_chromium_net_UrlRequestContext.h"
-#include "components/cronet/android/wrapped_channel_upload_element_reader.h"
 #include "net/android/net_jni_registrar.h"
 #include "url/android/url_jni_registrar.h"
 
@@ -24,7 +23,6 @@ const base::android::RegistrationMethod kCronetRegisteredMethods[] = {
   {"UrlAndroid", url::android::RegisterJni},
   {"UrlRequest", cronet::UrlRequestRegisterJni},
   {"UrlRequestContext", cronet::UrlRequestContextRegisterJni},
-  {"WrappedChannel", cronet::WrappedChannelRegisterJni},
 };
 
 base::AtExitManager* g_at_exit_manager = NULL;

components/cronet/android/java/src/org/chromium/net/UrlRequest.java

@@ -35,10 +35,10 @@ public class UrlRequest {
     private final Map<String, String> mHeaders;
     private final WritableByteChannel mSink;
     private Map<String, String> mAdditionalHeaders;
-    private String mPostBodyContentType;
+    private String mUploadContentType;
     private String mMethod;
-    private byte[] mPostBody;
-    private ReadableByteChannel mPostBodyChannel;
+    private byte[] mUploadData;
+    private ReadableByteChannel mUploadChannel;
     private WritableByteChannel mOutputChannel;
     private IOException mSinkException;
     private volatile boolean mStarted;
@@ -107,9 +107,9 @@ public class UrlRequest {
     public void setUploadData(String contentType, byte[] data) {
         synchronized (mLock) {
             validateNotStarted();
-            mPostBodyContentType = contentType;
-            mPostBody = data;
-            mPostBodyChannel = null;
+            mUploadContentType = contentType;
+            mUploadData = data;
+            mUploadChannel = null;
         }
     }
 
@@ -126,10 +126,10 @@ public class UrlRequest {
             ReadableByteChannel channel, long contentLength) {
         synchronized (mLock) {
             validateNotStarted();
-            mPostBodyContentType = contentType;
-            mPostBodyChannel = channel;
+            mUploadContentType = contentType;
+            mUploadChannel = channel;
             mUploadContentLength = contentLength;
-            mPostBody = null;
+            mUploadData = null;
         }
     }
 
@@ -158,8 +158,8 @@ public class UrlRequest {
 
             String method = mMethod;
             if (method == null &&
-                    ((mPostBody != null && mPostBody.length > 0) ||
-                      mPostBodyChannel != null)) {
+                    ((mUploadData != null && mUploadData.length > 0) ||
+                      mUploadChannel != null)) {
                 // Default to POST if there is data to upload but no method was
                 // specified.
                 method = "POST";
@@ -184,12 +184,12 @@ public class UrlRequest {
               }
             }
 
-            if (mPostBody != null && mPostBody.length > 0) {
-                nativeSetUploadData(mUrlRequestPeer, mPostBodyContentType,
-                                    mPostBody);
-            } else if (mPostBodyChannel != null) {
-                nativeSetUploadChannel(mUrlRequestPeer, mPostBodyContentType,
-                                       mPostBodyChannel, mUploadContentLength);
+            if (mUploadData != null && mUploadData.length > 0) {
+                nativeSetUploadData(mUrlRequestPeer, mUploadContentType,
+                                    mUploadData);
+            } else if (mUploadChannel != null) {
+                nativeSetUploadChannel(mUrlRequestPeer, mUploadContentType,
+                                       mUploadContentLength);
             }
 
             nativeStart(mUrlRequestPeer);
@@ -358,6 +358,36 @@ public class UrlRequest {
         headersMap.get(name).add(value);
     }
 
+    /**
+     * Reads a sequence of bytes from upload channel into the given buffer.
+     * @param dest The buffer into which bytes are to be transferred.
+     * @return Returns number of bytes read (could be 0) or -1 and closes
+     * the channel if error occured.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private int readFromUploadChannel(ByteBuffer dest) {
+        if (mUploadChannel == null || !mUploadChannel.isOpen())
+            return -1;
+        try {
+            int result = mUploadChannel.read(dest);
+            if (result < 0) {
+                mUploadChannel.close();
+                return 0;
+            }
+            return result;
+        } catch (IOException e) {
+            mSinkException = e;
+            try {
+                mUploadChannel.close();
+            } catch (IOException ignored) {
+                // Ignore this exception.
+            }
+            cancel();
+            return -1;
+        }
+    }
+
     private void validateNotRecycled() {
         if (mRecycled) {
             throw new IllegalStateException("Accessing recycled request");
@@ -392,8 +422,7 @@ public class UrlRequest {
             String contentType, byte[] content);
 
     private native void nativeSetUploadChannel(long urlRequestPeer,
-            String contentType, ReadableByteChannel content,
-            long contentLength);
+            String contentType, long contentLength);
 
     private native void nativeStart(long urlRequestPeer);
 

components/cronet/android/org_chromium_net_UrlRequest.cc

@@ -70,9 +70,9 @@ class JniURLRequestPeerDelegate
     int bytes_read = request->bytes_read();
     if (bytes_read != 0) {
       JNIEnv* env = base::android::AttachCurrentThread();
-      jobject bytebuf = env->NewDirectByteBuffer(request->Data(), bytes_read);
-      cronet::Java_UrlRequest_onBytesRead(env, owner_, bytebuf);
-      env->DeleteLocalRef(bytebuf);
+      base::android::ScopedJavaLocalRef<jobject> java_buffer(
+          env, env->NewDirectByteBuffer(request->Data(), bytes_read));
+      cronet::Java_UrlRequest_onBytesRead(env, owner_, java_buffer.obj());
     }
   }
 
@@ -81,6 +81,16 @@ class JniURLRequestPeerDelegate
     cronet::Java_UrlRequest_finish(env, owner_);
   }
 
+  virtual int ReadFromUploadChannel(net::IOBuffer* buf,
+                                    int buf_length) OVERRIDE {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    base::android::ScopedJavaLocalRef<jobject> java_buffer(
+        env, env->NewDirectByteBuffer(buf->data(), buf_length));
+    jint bytes_read = cronet::Java_UrlRequest_readFromUploadChannel(
+        env, owner_, java_buffer.obj());
+    return bytes_read;
+  }
+
  protected:
   virtual ~JniURLRequestPeerDelegate() {
     JNIEnv* env = base::android::AttachCurrentThread();
@@ -175,12 +185,11 @@ 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);
 }
 
 

components/cronet/android/url_request_peer.cc

@@ -51,10 +51,9 @@ void URLRequestPeer::SetUploadContent(const char* bytes, int bytes_len) {
       reader.Pass(), 0));
 }
 
-void URLRequestPeer::SetUploadChannel(
-    JNIEnv* env, jobject channel, int64 content_length) {
+void URLRequestPeer::SetUploadChannel(JNIEnv* env, int64 content_length) {
   scoped_ptr<net::UploadElementReader> reader(
-      new WrappedChannelElementReader(env, channel, content_length));
+      new WrappedChannelElementReader(delegate_, content_length));
   upload_data_stream_.reset(net::UploadDataStream::CreateWithReader(
       reader.Pass(), 0));
 }

components/cronet/android/url_request_peer.h

@@ -37,6 +37,7 @@ class URLRequestPeer : public net::URLRequest::Delegate {
     virtual void OnResponseStarted(URLRequestPeer* request) = 0;
     virtual void OnBytesRead(URLRequestPeer* request) = 0;
     virtual void OnRequestFinished(URLRequestPeer* request) = 0;
+    virtual int ReadFromUploadChannel(net::IOBuffer* buf, int buf_length) = 0;
 
    protected:
     friend class base::RefCountedThreadSafe<URLRequestPeerDelegate>;
@@ -59,7 +60,7 @@ class URLRequestPeer : public net::URLRequest::Delegate {
   void SetUploadContent(const char* bytes, int bytes_len);
 
   // Sets the request to streaming upload.
-  void SetUploadChannel(JNIEnv* env, jobject content, int64 content_length);
+  void SetUploadChannel(JNIEnv* env, int64 content_length);
 
   // Starts the request.
   void Start();

components/cronet/android/wrapped_channel_upload_element_reader.cc

@@ -4,53 +4,20 @@
 
 #include "components/cronet/android/wrapped_channel_upload_element_reader.h"
 
-#include <jni.h>
-
 #include "base/android/jni_android.h"
 #include "base/logging.h"
 #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(
+    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,18 +41,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) {
-    env->CallVoidMethod(channel_, g_method_close);
-    base::android::CheckException(env);
-    return bytes_read;
-  }
+  DCHECK(delegate_);
+  // TODO(mef): Post the read to file thread.
+  int bytes_read = delegate_->ReadFromUploadChannel(buf, buf_length);
+  if (bytes_read < 0)
+    return net::ERR_FAILED;
   offset_ += bytes_read;
   return bytes_read;
 }

components/cronet/android/wrapped_channel_upload_element_reader.h

@@ -5,10 +5,9 @@
 #ifndef COMPONENTS_CRONET_ANDROID_WRAPPED_CHANNEL_UPLOAD_ELEMENT_READER_H_
 #define COMPONENTS_CRONET_ANDROID_WRAPPED_CHANNEL_UPLOAD_ELEMENT_READER_H_
 
-#include <jni.h>
-
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "components/cronet/android/url_request_peer.h"
 #include "net/base/completion_callback.h"
 #include "net/base/upload_element_reader.h"
 
@@ -18,15 +17,15 @@ class IOBuffer;
 
 namespace cronet {
 
-bool WrappedChannelRegisterJni(JNIEnv* env);
-
 // An UploadElementReader implementation for
 // java.nio.channels.ReadableByteChannel.
 // |channel_| should outlive this class because this class does not take the
 // ownership of the data.
 class WrappedChannelElementReader : public net::UploadElementReader {
  public:
-  WrappedChannelElementReader(JNIEnv* env, jobject channel, uint64 length);
+  WrappedChannelElementReader(
+      scoped_refptr<URLRequestPeer::URLRequestPeerDelegate> delegate,
+      uint64 length);
   virtual ~WrappedChannelElementReader();
 
   // UploadElementReader overrides:
@@ -41,7 +40,7 @@ class WrappedChannelElementReader : public net::UploadElementReader {
  private:
   const uint64 length_;
   uint64 offset_;
-  jobject channel_;
+  scoped_refptr<URLRequestPeer::URLRequestPeerDelegate> delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(WrappedChannelElementReader);
 };