Cronet modifications to support AGSA.

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

 // 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.
 
 package org.chromium.net;
 
+import android.os.ConditionVariable;
+
 import org.apache.http.conn.ConnectTimeoutException;
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.net.UnknownHostException;
 import java.nio.ByteBuffer;
 import java.nio.channels.ReadableByteChannel;
@@ skipping 15 matching lines @@
     private final UrlRequestContext mRequestContext;
     private final String mUrl;
     private final int mPriority;
     private final Map<String, String> mHeaders;
     private final WritableByteChannel mSink;
     private Map<String, String> mAdditionalHeaders;
     private String mUploadContentType;
     private String mMethod;
     private byte[] mUploadData;
     private ReadableByteChannel mUploadChannel;
-    private WritableByteChannel mOutputChannel;
     private IOException mSinkException;
     private volatile boolean mStarted;
     private volatile boolean mCanceled;
     private volatile boolean mRecycled;
     private volatile boolean mFinished;
     private boolean mHeadersAvailable;
     private String mContentType;
     private long mContentLength;
     private long mUploadContentLength;
+    private ConditionVariable mAppendChunkCondition;
     private final ContextLock mLock;
 
     /**
      * Native adapter object, owned by UrlRequest.
      */
     private long mUrlRequestAdapter;
 
     /**
      * Constructor.
      *
@@ skipping 41 matching lines @@
      *            POST.
      * @param data The content that needs to be uploaded if this is a POST
      *            request.
      */
     public void setUploadData(String contentType, byte[] data) {
         synchronized (mLock) {
             validateNotStarted();
             mUploadContentType = contentType;
             mUploadData = data;
             mUploadChannel = null;
+            mAppendChunkCondition = null;
         }
     }
 
     /**
      * Sets a readable byte channel to upload as part of a POST request.
      *
      * @param contentType MIME type of the post content or null if this is not a
      *            POST request.
      * @param channel The channel to read to read upload data from if this is a
      *            POST request.
      * @param contentLength The length of data to upload.
      */
     public void setUploadChannel(String contentType,
             ReadableByteChannel channel, long contentLength) {
         synchronized (mLock) {
             validateNotStarted();
             mUploadContentType = contentType;
             mUploadChannel = channel;
             mUploadContentLength = contentLength;
             mUploadData = null;
+            mAppendChunkCondition = null;
         }
     }
 
+    /**
+     * Sets this request up for chunked uploading. To upload data call
+     * {@link #appendChunkBlocking(ByteBuffer, boolean)} after {@link #start()}.
+     *
+     * @param contentType MIME type of the post content or null if this is not a
+     *            POST request.
+     */
+    public void setChunkedUpload(String contentType) {

[mmenke, 2014/08/15 15:30:21]

setUploadChannel uses chunked uploads, too, so should probably rename this.

[mef, 2014/08/15 15:42:52]

Actually, I think (correct me if I'm wrong) setUploadChannel does not use
chunked uploads any more. That was a change upstreamed back in July, which has
removed chunked uploads.

[mmenke, 2014/08/15 15:49:01]

You're right, was forgetting it took a length.

+        synchronized (mLock) {
+            validateNotStarted();
+            mUploadContentType = contentType;
+            mAppendChunkCondition = new ConditionVariable();
+            mUploadData = null;
+            mUploadChannel = null;
+        }
+    }
+
+    /**
+     * Uploads a new chunk. Must have called {@link #setChunkedUpload(String)}
+     * and {@link #start()}.
+     *
+     * @param chunk The data, which will not be modified. It must not be empty
+     *            and its current position must be zero.
+     * @param isLastChunk Whether this chunk is the last one.
+     */
+    // Invokes {@link #nativeAppendChunk(long, ByteBuffer, int, boolean)} and
+    // waits for it
+    // to notify completion.

[mmenke, 2014/08/15 15:30:21]

This should be merged with the above comment, I think...At least I've never see
two comment styles mixed like this.

[mdumitrescu, 2014/08/15 15:50:20]

Well, the implementation details shouldn't be in the javadoc (unless relevant).
Removed since it's pretty clear from the doc.

+    public void appendChunkBlocking(ByteBuffer chunk, boolean isLastChunk)
+            throws IOException {
+        if (!chunk.hasRemaining()) {
+            throw new IllegalArgumentException(
+                    "Attempted to write empty buffer.");
+        }
+        if (chunk.position() != 0) {
+            throw new IllegalArgumentException("The position must be zero.");
+        }
+        synchronized (mLock) {
+            if (!mStarted) {
+                throw new IllegalStateException("Request not yet started.");
+            }
+            if (mAppendChunkCondition == null) {
+                throw new IllegalStateException(
+                        "Request not set for chunked uploadind.");
+            }
+            if (mUrlRequestAdapter == 0) {
+                throw new IOException("Native peer destroyed.");
+            }
+            mAppendChunkCondition.close();
+            nativeAppendChunk(mUrlRequestAdapter, chunk, chunk.limit(),
+                    isLastChunk);
+        }
+        // Wait for the data to be actually consumed. Outside mLock to avoid
+        // deadlock.
+        mAppendChunkCondition.block();

[mmenke, 2014/08/15 15:30:21]

We're blocking on another thread to avoid a string copy?  Does this really make
sense?  I suppose these are rather large chunks?

[mef, 2014/08/15 15:42:52]

I think the idea is that one thread is writing into buffer and another (network
thread) is uploading from buffer, hence the lock. nativeAppendChunk posts to
network thread.

[mmenke, 2014/08/15 15:49:01]

Right...but the question is if two copies is worse than blocking on whatever
else the network thread is doing and a copy.  In general, I am very much against
blocking on another thread.

[mdumitrescu, 2014/08/15 15:50:20]

I'm very open to suggestions regarding this.

A copy wouldn't necessarily be a problem (well, unless we'd have to
allocateDirect). In my mind, the problem comes from url_request_adapter.cc.
Specifically:

void URLRequestAdapter::AppendChunk(const char* bytes, int bytes_len,
                                    bool is_last_chunk) {
  VLOG(1) << "AppendChunk, len: " << bytes_len << ", last: " << is_last_chunk;
  context_->GetNetworkTaskRunner()->PostTask(
      FROM_HERE,
      base::Bind(&URLRequestAdapter::OnAppendChunk,
                 base::Unretained(this),
                 bytes,
                 bytes_len,
                 is_last_chunk));
}

Since this sends the data to another thread what happens if instead we return
immediately, then call another appendChunk? Could the data become reordered? Or
what happens if the cancel() or getHttpStatusCode() is called? Are these cases
safe? If so, awesome, I'm all for making a copy right before
context_->GetNetworkTaskRunner()->PostTask() and returning immediately.

[mmenke, 2014/08/15 16:09:24]

Tasks are executed in the order they're posted, since the NetworkTaskRunner is a
SingleThreadedTaskRunner - a lot of things would break if that changed.

cancel() is also safe.  The upload data stream already has to handle both the
case where it receives data and the network request isn't ready to read it, and
where it's being used on cancellation.

getHttpStatusCode is also safe.

The one issue is that we need to make sure the buffer we're posting over to
another thread isn't destroyed in the meantime.  Simplest way to do that is to
copy it.

[mdumitrescu, 2014/08/15 18:20:15]

Done.

+    }
+
     public void setHttpMethod(String method) {
         validateNotStarted();
         if (!("PUT".equals(method) || "POST".equals(method))) {
             throw new IllegalArgumentException("Only PUT or POST are allowed.");
         }
         mMethod = method;
     }
 
     public WritableByteChannel getSink() {
         return mSink;
     }
 
     public void start() {
         synchronized (mLock) {
             if (mCanceled) {
                 return;
             }
 
             validateNotStarted();
             validateNotRecycled();
 
             mStarted = true;
 
             String method = mMethod;
             if (method == null &&
                     ((mUploadData != null && mUploadData.length > 0) ||
-                      mUploadChannel != null)) {
+                      mUploadChannel != null || mAppendChunkCondition != null)) {
                 // Default to POST if there is data to upload but no method was
                 // specified.
                 method = "POST";
             }
 
             if (method != null) {
                 nativeSetMethod(mUrlRequestAdapter, method);
             }
 
             if (mHeaders != null && !mHeaders.isEmpty()) {
@@ skipping 10 matching lines @@
                                     entry.getValue());
               }
             }
 
             if (mUploadData != null && mUploadData.length > 0) {
                 nativeSetUploadData(mUrlRequestAdapter, mUploadContentType,
                                     mUploadData);
             } else if (mUploadChannel != null) {
                 nativeSetUploadChannel(mUrlRequestAdapter, mUploadContentType,
                                        mUploadContentLength);
+            } else if (mAppendChunkCondition != null) {
+                nativeEnableChunkedUpload(mUrlRequestAdapter,
+                                          mUploadContentType);
             }
 
             nativeStart(mUrlRequestAdapter);
           }
     }
 
     public void cancel() {
         synchronized (mLock) {
             if (mCanceled) {
                 return;
@@ skipping 78 matching lines @@
 
     // All response headers.
     public Map<String, List<String>> getAllHeaders() {
         validateHeadersAvailable();
         ResponseHeadersMap result = new ResponseHeadersMap();
         nativeGetAllHeaders(mUrlRequestAdapter, result);
         return result;
     }
 
     /**
+     * A callback invoked when appending a chunk to the request has completed.
+     */
+    @CalledByNative
+    protected void onAppendChunkCompleted() {
+        mAppendChunkCondition.open();
+    }
+
+    /**
      * A callback invoked when the first chunk of the response has arrived.
      */
     @CalledByNative
     protected void onResponseStarted() {
         mContentType = nativeGetContentType(mUrlRequestAdapter);
         mContentLength = nativeGetContentLength(mUrlRequestAdapter);
         mHeadersAvailable = true;
     }
 
     /**
@@ skipping 22 matching lines @@
     }
 
     /**
      * Notifies the listener, releases native data structures.
      */
     @SuppressWarnings("unused")
     @CalledByNative
     private void finish() {
         synchronized (mLock) {
             mFinished = true;
+            if (mAppendChunkCondition != null) {
+                mAppendChunkCondition.open();
+            }
 
             if (mRecycled) {
                 return;
             }
             try {
                 mSink.close();
             } catch (IOException e) {
                 // Ignore
             }
             onRequestComplete();
@@ skipping 75 matching lines @@
             String value);
 
     private native void nativeSetMethod(long urlRequestAdapter, String method);
 
     private native void nativeSetUploadData(long urlRequestAdapter,
             String contentType, byte[] content);
 
     private native void nativeSetUploadChannel(long urlRequestAdapter,
             String contentType, long contentLength);
 
+    private native void nativeEnableChunkedUpload(long urlRequestAdapter,
+            String contentType);
+
+    private native void nativeAppendChunk(long urlRequestAdapter,
+            ByteBuffer chunk, int chunkSize, boolean isLastChunk);
+
     private native void nativeStart(long urlRequestAdapter);
 
     private native void nativeCancel(long urlRequestAdapter);
 
     private native void nativeDestroyRequestAdapter(long urlRequestAdapter);
 
     private native int nativeGetErrorCode(long urlRequestAdapter);
 
     private native int nativeGetHttpStatusCode(long urlRequestAdapter);
 
     private native String nativeGetErrorString(long urlRequestAdapter);
 
     private native String nativeGetContentType(long urlRequestAdapter);
 
     private native long nativeGetContentLength(long urlRequestAdapter);
 
     private native String nativeGetHeader(long urlRequestAdapter, String name);
 
     private native void nativeGetAllHeaders(long urlRequestAdapter,
             ResponseHeadersMap headers);
 
     // Explicit class to work around JNI-generator generics confusion.
     private class ResponseHeadersMap extends HashMap<String, List<String>> {
     }
 }