Initial implementation of Cronet Async API.

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

Index: components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
diff --git a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
index 3a7776dc8520cbabc219fde0c6515e7d1499e808..3f6a6d37ae37b6a58986f8694789c2fa48c3d04f 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
@@ -4,38 +4,174 @@
 
 package org.chromium.net;
 
+import android.util.Log;
+
+import org.chromium.base.CalledByNative;
+import org.chromium.base.JNINamespace;
+
+import java.net.URL;
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+
 /**
  * UrlRequest using Chromium HTTP stack implementation.
  */
-public class CronetUrlRequest implements UrlRequest {
+@JNINamespace("cronet")
+final class CronetUrlRequest implements UrlRequest {
+    /** Native adapter object, owned by UrlRequest. */
+    private long mUrlRequestAdapter;
+    private final CronetUrlRequestContext mRequestContext;
+    private final List<String> mUrlChain = new ArrayList<String>();
+    private final int mPriority;
+    private final UrlRequestListener mListener;
+    private final Executor mExecutor;
+    private NativeResponseInfo mResponseInfo;
+    private boolean mStarted = false;
+    private boolean mCanceled = false;
+    private OnDataReceivedRunnable mOnDataReceivedTask;
+
+    final class OnDataReceivedRunnable implements Runnable {
+        ByteBuffer mByteBuffer;
+        public void run() {
+            if (isCanceled()) {
+                return;
+            }
+            try {
+                mListener.onDataReceived(CronetUrlRequest.this,
+                        mResponseInfo, mByteBuffer);
+                mByteBuffer = null;
+                if (!isCanceled()) {
+                    nativeReceiveData(mUrlRequestAdapter);
+                }
+            } catch (Exception e) {
+                onCalledByNativeException(e);
+            }
+        }
+    }
+
+    final class NativeResponseInfo implements ExtendedResponseInfo {
+        private final String[] mUrlChain;
+        private final int mHttpStatusCode;
+        private final ResponseHeadersMap mAllHeaders = new ResponseHeadersMap();
+        private final boolean mWasCached;
+        private final String mNegotiatedProtocol;
+        private long mTotalReceivedBytes = 0;

[mmenke, 2014/10/20 21:23:04]

I'm still nervous about changing this out from under the embedder - I could see
reasons they might want to cache this structure, or fool with it on a thread
other than the executor thread, and in either case, modifying this can get us
into trouble.

[mef, 2014/10/21 21:27:47]

Done.

+
+        NativeResponseInfo(String[] urlChain, int httpStatusCode,
+                boolean wasCached, String negotiatedProtocol) {
+            mUrlChain = urlChain;
+            mHttpStatusCode = httpStatusCode;
+            mWasCached = wasCached;
+            mNegotiatedProtocol = negotiatedProtocol;
+        }
+
+        @Override
+        public String getUrl() {
+            return mUrlChain[mUrlChain.length - 1];
+        }
+
+        @Override
+        public String[] getUrlChain() {
+            return mUrlChain;
+        }
+
+        @Override
+        public int getHttpStatusCode() {
+            return mHttpStatusCode;
+        }
+
+        @Override
+        public Map<String, List<String>> getAllHeaders() {
+            return mAllHeaders;
+        }
+
+        @Override
+        public boolean wasCached() {
+            return mWasCached;
+        }
+
+        @Override
+        public String getNegotiatedProtocol() {
+            return mNegotiatedProtocol;
+        }
+
+        @Override
+        public long getTotalReceivedBytes() {
+            return mTotalReceivedBytes;
+        }
+    };
+
+    CronetUrlRequest(CronetUrlRequestContext requestContext,
+            String url, int priority,
+            UrlRequestListener listener,
+            Executor executor) {
+        if (requestContext == null) {
+            throw new NullPointerException("Context is required");
+        }
+        if (url == null) {
+            throw new NullPointerException("URL is required");
+        }
+        if (listener == null) {
+            throw new NullPointerException("Listener is required");
+        }
+        if (executor == null) {
+            throw new NullPointerException("Executor is required");
+        }
+
+        mRequestContext = requestContext;
+        mUrlChain.add(url);
+        mPriority = convertRequestPriority(priority);
+        mUrlRequestAdapter = nativeCreateRequestAdapter(
+                mRequestContext.getCronetUrlRequestContextAdapter(),
+                url,
+                mPriority);
+        mListener = listener;
+        mExecutor = executor;
+    }
+
     @Override
     public void setHttpMethod(String method) {

[mmenke, 2014/10/20 21:23:04]

Should test this, and add header, now that we have a way to do so.  Should also
check the default method.

[mef, 2014/10/22 20:50:05]

Done.

-
+        checkNotStarted();
+        if (method == null) {
+            throw new NullPointerException("method is required");
+        }
+        nativeSetHttpMethod(mUrlRequestAdapter, method);
     }
 
     @Override
     public void addHeader(String header, String value) {
-
+        checkNotStarted();
+        if (header == null || value == null) {
+            throw new NullPointerException("Invalid header");
+        }
+        nativeAddHeader(mUrlRequestAdapter, header, value);
     }
 
     @Override
-    public void start(UrlRequestListener listener) {
-
+    public void start() {
+        checkNotStarted();
+        mStarted = true;
+        nativeStart(mUrlRequestAdapter);
     }
 
     @Override
     public void cancel() {

[mmenke, 2014/10/20 21:23:04]

Should assert that we're called on the executor thread, here and in isCanceled
and pause.  And maybe a test to make sure that fails correctly?

[mef, 2014/10/20 22:30:09]

It seems that Executors unlike loopers don't necessarily attach to a particular
thread. For example one can create thread pool executor.

[mmenke, 2014/10/21 14:28:01]

This is a problem - the code doesn't currently support that case.  We should
either require this not be the case with runtime checks and exceptions, or allow
it, and add locks around all access to mCanceled (And mPaused once we add it).

[mef, 2014/10/21 14:49:11]

Either that or switch to loopers. In my opinion adding locks around access to
mLocked is not that bad as I don't expect any real contention there. WDYT?

[mmenke, 2014/10/21 15:30:45]

Per offline discussion, I'm not concerned about performance.  my main concern
with exector is correctness and giving strong guarantees that we won't call into
the embedder after cancelation, without addind the potential for deadlock.

I think Looper is the way to go, though there are tradeoffs.

-
+        mCanceled = true;
+        nativeCancel(mUrlRequestAdapter);
     }
 
     @Override
     public boolean isCanceled() {
-        return false;
+        return mCanceled;
     }
 
     @Override
     public void pause() {
-
+        throw new UnsupportedOperationException("Not implemented yet");
     }
 
     @Override
@@ -45,6 +181,260 @@ public class CronetUrlRequest implements UrlRequest {
 
     @Override
     public void resume() {
+        throw new UnsupportedOperationException("Not implemented yet");
+    }
+
+    private static int convertRequestPriority(int priority) {
+        switch (priority) {
+            case REQUEST_PRIORITY_IDLE:
+                return ChromiumUrlRequestPriority.IDLE;
+            case REQUEST_PRIORITY_LOWEST:
+                return ChromiumUrlRequestPriority.LOWEST;
+            case REQUEST_PRIORITY_LOW:
+                return ChromiumUrlRequestPriority.LOW;
+            case REQUEST_PRIORITY_MEDIUM:
+                return ChromiumUrlRequestPriority.MEDIUM;
+            case REQUEST_PRIORITY_HIGHEST:
+                return ChromiumUrlRequestPriority.HIGHEST;
+            default:
+                return ChromiumUrlRequestPriority.MEDIUM;
+        }
+    }
+
+    private NativeResponseInfo prepareResponseInfo() {
+        NativeResponseInfo responseInfo = new NativeResponseInfo(
+                mUrlChain.toArray(new String[mUrlChain.size()]),
+                nativeGetHttpStatusCode(mUrlRequestAdapter),
+                nativeGetWasCached(mUrlRequestAdapter),
+                nativeGetNegotiatedProtocol(mUrlRequestAdapter));
+        nativeGetAllHeaders(mUrlRequestAdapter, responseInfo.mAllHeaders);
+        return responseInfo;
+    }
+
+    private void checkNotStarted() {

[mmenke, 2014/10/20 21:23:04]

Should have a test where this fails, and make sure an exception is thrown.

[mef, 2014/10/22 20:50:05]

Done.

+        if (mStarted) {
+            throw new IllegalStateException("Request is already started.");
+        }
+    }
+
+    private void destroyRequestAdapter() {
+        if (mUrlRequestAdapter != 0) {
+            nativeDestroyRequestAdapter(mUrlRequestAdapter);
+            mUrlRequestAdapter = 0;
+        } else {
+            throw new IllegalStateException("Request is already destroyed.");
+        }
+    }
+
+    /**
+     * If @CalledByNative method throws an exception, request gets cancelled
+     * and exception could be retrieved from request using getException().
+     */
+    private void onCalledByNativeException(Exception e) {
+        UrlRequestException requestError = new UrlRequestException(
+                "CalledByNative method has thrown an exception", e);
+        Log.e(CronetUrlRequestContext.LOG_TAG,
+                "Exception in CalledByNative method", e);
+        try {
+            mListener.onError(this, mResponseInfo, requestError);
+            cancel();
+        } catch (Exception cancelException) {
+            Log.e(CronetUrlRequestContext.LOG_TAG,
+                    "Exception trying to cancel request", cancelException);
+        }
+    }
+
+    // Private methods called by the native code.

[mmenke, 2014/10/20 21:23:04]

Suggest making this more noticeable, like:

////////////////////////////////////////////////
//
//  Private methods called by native code.
//
////////////////////////////////////////////////

(Using "//" because I think "/*" may make Javadoc sad)

[mef, 2014/10/21 21:27:47]

Done.

+
+    /**
+     * Called before following redirects.  The redirect will automatically be
+     * followed, unless the request is paused or cancelled during this
+     * callback.  If the redirect response has a body, it will be ignored.
+     * This will only be called between start and onResponseStarted.
+     *
+     * @param newLocation Location where request is redirected.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onRedirect(final String newLocation) {
+        mUrlChain.add(newLocation);
+        final NativeResponseInfo responseInfo = prepareResponseInfo();
+        Runnable task = new Runnable() {
+            public void run() {
+                try {
+                    mListener.onRedirect(CronetUrlRequest.this, responseInfo,
+                        new URL(newLocation));
+                    if (!isCanceled()) {
+                        nativeFollowDeferredRedirect(mUrlRequestAdapter);
+                    }
+                } catch (Exception e) {
+                    onCalledByNativeException(e);
+                }
+            }
+        };
+        mExecutor.execute(task);
+    }
+
+    /**
+     * Called when the final set of headers, after all redirects,
+     * is received. Can only be called once for each request.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onResponseStarted() {
+        mResponseInfo = prepareResponseInfo();
+        Runnable task = new Runnable() {
+            public void run() {
+                try {
+                    mListener.onResponseStarted(CronetUrlRequest.this,
+                                                mResponseInfo);
+                    if (!isCanceled()) {
+                        nativeReceiveData(mUrlRequestAdapter);
+                    }
+                } catch (Exception e) {
+                    onCalledByNativeException(e);
+                }
+            }
+        };
+        mExecutor.execute(task);
+    }
+
+    /**
+     * Called whenever data is received.  The ByteBuffer remains
+     * valid only until listener callback.  Or if the callback
+     * pauses the request, it remains valid until the request is resumed.
+     * Cancelling the request also invalidates the buffer.
+     *
+     * @param byteBuffer Received data.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onDataReceived(final ByteBuffer byteBuffer) {
+        if (mOnDataReceivedTask == null) {
+            mOnDataReceivedTask = new OnDataReceivedRunnable();
+        }
+        mOnDataReceivedTask.mByteBuffer = byteBuffer;
+        mExecutor.execute(mOnDataReceivedTask);
+    }
+
+    /**
+     * Called when request is canceled, no callbacks are called.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onCanceled() {
+        Runnable task = new Runnable() {
+            public void run() {
+                destroyRequestAdapter();
+            }
+        };
+        mExecutor.execute(task);
+    }
+
+    /**
+     * Called when request is complete, no callbacks will be called afterwards.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onComplete() {
+        mResponseInfo.mTotalReceivedBytes =
+                nativeGetTotalReceivedBytes(mUrlRequestAdapter);
+        Runnable task = new Runnable() {
+            public void run() {
+                if (isCanceled()) {
+                    return;
+                }
+                try {
+                    mListener.onComplete(CronetUrlRequest.this, mResponseInfo);
+                    destroyRequestAdapter();
+                } catch (Exception e) {
+                    onCalledByNativeException(e);
+                }
+            }
+        };
+        mExecutor.execute(task);
+    }
+
+    /**
+     * Called when error has occured, no callbacks will be called afterwards.
+     *
+     * @param nativeError native net error code.
+     * @param errorString textual representation of the error code.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onError(final int nativeError, final String errorString) {
+        Runnable task = new Runnable() {
+            public void run() {
+                if (isCanceled()) {
+                    return;
+                }
+                try {
+                    UrlRequestException requestError = new UrlRequestException(
+                            "Exception in CronetUrlRequest: " + errorString,
+                            nativeError);
+                    mListener.onError(CronetUrlRequest.this,
+                                      mResponseInfo,
+                                      requestError);
+                    destroyRequestAdapter();
+                } catch (Exception e) {
+                    onCalledByNativeException(e);
+                }
+            }
+        };
+        mExecutor.execute(task);
+    }
+
+    /**
+     * Appends header |name| with value |value| to |headersMap|.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onAppendResponseHeader(ResponseHeadersMap headersMap,
+            String name, String value) {
+        try {
+            if (!headersMap.containsKey(name)) {
+                headersMap.put(name, new ArrayList<String>());
+            }
+            headersMap.get(name).add(value);
+        } catch (Exception e) {
+            onCalledByNativeException(e);
+        }
+    }
+
+    // Native methods are implemented in cronet_url_request.cc.
+
+    private native long nativeCreateRequestAdapter(
+            long urlRequestContextAdapter, String url, int priority);
+
+    private native void nativeAddHeader(long urlRequestAdapter, String name,
+            String value);
+
+    private native void nativeSetHttpMethod(long urlRequestAdapter,
+            String method);
+
+    private native void nativeStart(long urlRequestAdapter);
+
+    private native void nativeCancel(long urlRequestAdapter);
+
+    private native void nativeDestroyRequestAdapter(long urlRequestAdapter);
+
+    private native void nativeFollowDeferredRedirect(long urlRequestAdapter);
+
+    private native void nativeReceiveData(long urlRequestAdapter);
+
+    private native int nativeGetHttpStatusCode(long urlRequestAdapter);
+
+    private native void nativeGetAllHeaders(long urlRequestAdapter,
+            ResponseHeadersMap headers);
+
+    private native String nativeGetNegotiatedProtocol(long urlRequestAdapter);
+
+    private native boolean nativeGetWasCached(long urlRequestAdapter);
+
+    private native long nativeGetTotalReceivedBytes(long urlRequestAdapter);
 
+    // Explicit class to work around JNI-generator generics confusion.
+    private class ResponseHeadersMap extends HashMap<String, List<String>> {
     }
 }