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..23462316eed5abde1a4a86060dda584c5ff2f007 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
@@ -4,33 +4,131 @@
 
 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.
  */
+@JNINamespace("cronet")
 public class CronetUrlRequest implements UrlRequest {

[Charles, 2014/09/20 00:47:34]

Final?

[mef, 2014/09/22 17:12:13]

Done.

+    /**
+     * Native adapter object, owned by UrlRequest.
+     */
+    private long mUrlRequestAdapter;
+    private final CronetUrlRequestContext mRequestContext;
+    private final String mUrl;
+    private final int mPriority;
+    private final UrlRequestListener mListener;
+    private final Executor mExecutor;
+    private ResponseInfo mResponseInfo;
+    private boolean mCanceled = false;
+
+    class NativeResponseInfo implements ResponseInfo {

[Charles, 2014/09/20 00:47:34]

Final

[mef, 2014/09/22 17:12:13]

Done.

+        @Override
+        public URL getUrl() {
+            return null;
+        }
+
+        @Override
+        public URL[] getUrlChain() {
+            return null;
+        }
+
+        @Override
+        public int getHttpStatusCode() {
+            return 0;
+        }
+
+        @Override
+        public Map<String, List<String>> getAllHeaders() {
+            return null;
+        }
+
+        @Override
+        public boolean wasCached() {
+            return false;
+        }
+
+        @Override
+        public boolean wasFetchedOverSPDY() {

[Charles, 2014/09/20 00:47:34]

I actually prefer getting the negotiated protocol string.

[mef, 2014/09/22 17:12:12]

sgtm

+            return false;
+        }
+
+        @Override
+        public boolean wasFetchedOverQUIC() {
+            return false;
+        }
+    };
+
+    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;
+        mUrl = url;
+        mPriority = convertRequestPriority(priority);
+        mUrlRequestAdapter = nativeCreateRequestAdapter(
+                mRequestContext.getCronetUrlRequestContextAdapter(),

[Charles, 2014/09/20 00:47:34]

These params all fit on one line

[mef, 2014/09/22 17:12:13]

Acknowledged.

[mef, 2014/09/23 18:42:14]

Actually, they don't fit into 80 chars, which is our self-imposed limit due to
rietveld ui.

+                mUrl,
+                mPriority);
+        mListener = listener;
+        mExecutor = executor;
+    }
+
     @Override
     public void setHttpMethod(String method) {
-
+        if (method == null) {
+            throw new NullPointerException("method is required");
+        }
+        nativeSetHttpMethod(mUrlRequestAdapter, method);
     }
 
     @Override
     public void addHeader(String header, String value) {
-
+        if (header == null || value == null) {
+            throw new NullPointerException("Invalid header");
+        }
+        nativeAddHeader(mUrlRequestAdapter, header, value);
     }
 
     @Override
-    public void start(UrlRequestListener listener) {
-
+    public void start() {

[Charles, 2014/09/20 00:47:33]

These methods should have threading policy attached to them, so that they throw
if they're called on the wrong thread.

[mef, 2014/09/22 17:12:12]

Is there some way to formally declare threading policy via annotations or
something?
Also, I was thinking that "start" could get called from arbitrary thread and
posts to network thread internally.

[Charles, 2014/09/22 21:22:54]

There is, but it's not standard and the compiler doesn't enforce it. Checking
currentThread() and throwing exceptions is the way to go. Also, while start()
should be able to be called from other threads, it should be called from the
same thread as the setup of the request.

+        nativeStart(mUrlRequestAdapter);
     }
 
     @Override
     public void cancel() {
-
+        mCanceled = true;
+        nativeDestroyRequestAdapter(mUrlRequestAdapter);
     }
 
     @Override
     public boolean isCanceled() {
-        return false;
+        return mCanceled;
     }
 
     @Override
@@ -47,4 +145,227 @@ public class CronetUrlRequest implements UrlRequest {
     public void resume() {
 
     }
+
+    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 void prepareResponseInfo() {
+
+    }
+
+
+    // Private methods called by native library.
+
+    /**
+     * If @CalledByNative method throws an exception, request gets cancelled
+     * and exception could be retrieved from request using getException().
+     */
+    private void onCalledByNativeException(Exception e) {

[Charles, 2014/09/20 00:47:34]

Needs @CalledByNative annotation

[mef, 2014/09/22 17:12:12]

It is not called by native, it merely handles exceptions caught in
@CalledByNative methods.

+        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, requestError);
+            cancel();
+        } catch (Exception cancel_exception) {

[Charles, 2014/09/20 00:47:33]

Use java style naming of exception

[mef, 2014/09/22 17:12:13]

Done.

+            Log.e(CronetUrlRequestContext.LOG_TAG,
+                    "Exception trying to cancel request", cancel_exception);
+        }
+    }
+
+    /**
+     * 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 info Response information.
+     * @param newLocation Location where request is redirected.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onRedirect(final String newLocation) {
+        prepareResponseInfo();
+        Runnable task = new Runnable() {
+            public void run() {
+                try {
+                    mListener.onRedirect(CronetUrlRequest.this, mResponseInfo,
+                        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.
+     *
+     * @param info Response information.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onResponseStarted() {
+        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 for the duration of the 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) {
+        Runnable task = new Runnable() {

[Charles, 2014/09/20 00:47:34]

Is there any way to avoid allocating this every time onDataRecieved is called?

[mef, 2014/09/22 17:12:12]

I guess I can keep onDataReceivedRunnable as a member of CronetUrlRequest and
just pass new byte buffer to it. Is this what you mean?

[Charles, 2014/09/22 21:22:53]

That would work.

On 2014/09/22 17:12:12, mef wrote:

[mef, 2014/09/23 18:42:14]

Done.

+            public void run() {
+                try {
+                    mListener.onDataReceived(CronetUrlRequest.this, byteBuffer);
+                    if (!isCanceled())
+                        nativeReceiveData(mUrlRequestAdapter);
+                } catch (Exception e) {
+                    onCalledByNativeException(e);
+                }
+            }
+        };
+        mExecutor.execute(task);
+    }
+
+    /**
+     * Called when request is complete, no callbacks will be called afterwards.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onComplete() {
+        Runnable task = new Runnable() {
+            public void run() {
+                try {
+                    mListener.onComplete(CronetUrlRequest.this);
+                    nativeDestroyRequestAdapter(mUrlRequestAdapter);
+                } catch (Exception e) {
+                    onCalledByNativeException(e);
+                }
+            }
+        };
+        mExecutor.execute(task);
+    }
+
+    /**
+     * Called when error has occured, no callbacks will be called afterwards.
+     */
+    @SuppressWarnings("unused")
+    @CalledByNative
+    private void onError(int error) {
+        Runnable task = new Runnable() {
+            public void run() {
+                try {
+                    UrlRequestException requestError = new UrlRequestException(
+                            "Exception in CronetUrlRequest ", null);
+                        mListener.onError(CronetUrlRequest.this, requestError);
+                    nativeDestroyRequestAdapter(mUrlRequestAdapter);
+                } catch (Exception e) {
+                    onCalledByNativeException(e);

[Charles, 2014/09/20 00:47:34]

Given that this is happening on the other thread, do we need the Exception
catching?

[mef, 2014/09/22 17:12:12]

Good question. What should happen if listener throws an exception?

[Charles, 2014/09/22 21:22:53]

I think the current behavior is correct, except that if cancel() is called from
the executor thread then you'll be racing the state of mCancelled and the native
destruction of the request. The cancel() method should cause a post to the
network thread.

On 2014/09/22 17:12:12, mef wrote:

[mef, 2014/09/23 18:42:14]

Done.

+                }
+            }
+        };
+        mExecutor.execute(task);

[mef, 2014/09/22 17:12:13]

Should we execute it inside of try...catch to handle bad executors?

+    }
+
+    /**
+     * 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);
+
+    // TODO(mef): Remove unused methods (if any).
+
+    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);
+
+    private native String nativeGetNegotiatedProtocol(long urlRequestAdapter);
+
+    // Explicit class to work around JNI-generator generics confusion.
+    private class ResponseHeadersMap extends HashMap<String, List<String>> {
+    }
+
+
 }