Remove explicit JNI references by adding UrlRequest.readFromUploadChannel callback.

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

Index: components/cronet/android/java/src/org/chromium/net/UrlRequest.java
diff --git a/components/cronet/android/java/src/org/chromium/net/UrlRequest.java b/components/cronet/android/java/src/org/chromium/net/UrlRequest.java
index 0d978a4781034cfe60574ef13f79db9f647a47b9..1a3bf2d82bd9dbaed21268c55af6f15f82ef49cc 100644
--- a/components/cronet/android/java/src/org/chromium/net/UrlRequest.java
+++ b/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,34 @@ public class UrlRequest {
         headersMap.get(name).add(value);
     }
 
+    /**
+     * Reads data from |mUploadChannel| into |dst|. Returns -1 and closes
+     * the channel on error.

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

Should we be using javadoc comments?  @param, @returns, etc?

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

Done.

+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private int readFromUploadChannel(ByteBuffer dst) {

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

nit:  Suggest dest instead of dst.  It's a bit more common.

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

Done.

+        if (mUploadChannel == null || !mUploadChannel.isOpen())
+            return -1;
+        try {
+            int result = mUploadChannel.read(dst);
+            if (result < 0) {
+                mUploadChannel.close();
+                return 0;

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

Hrm...think this is actually a fail case, too.  We only read when we haven't
read up to the reported size, so if we read and get 0, we'll read again...and
then return -1 in the isOpen check.  Seems like it should be fine, though.

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

Yeah, in my testing current implementation doesn't attempt to read beyond the
|GetContentLength()| of UploadElementReader, so in normal file upload scenario
this is not triggered, which probably is not good?

[mmenke, 2014/08/06 14:33:27]

Yea, the issue is just if we get the wrong length.  Anyways, seems fine.

+            }
+            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 +420,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);