Initial implementation of Cronet Async API.

components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestTest.java

Index: components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestTest.java b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..5aca2072a2d3418f0f532190187521156e8a6fab
--- /dev/null
+++ b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestTest.java
@@ -0,0 +1,416 @@
+// 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.cronet_test_apk;
+
+import android.os.ConditionVariable;
+
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.chromium.base.test.util.Feature;
+import org.chromium.net.ExtendedResponseInfo;
+import org.chromium.net.ResponseInfo;
+import org.chromium.net.UrlRequest;
+import org.chromium.net.UrlRequestException;
+import org.chromium.net.UrlRequestListener;
+
+import java.net.URL;
+import java.nio.ByteBuffer;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+/**
+ * Test basic functionality of UrlRequest.
+ */
+public class CronetUrlRequestTest extends CronetTestBase {
+    // URL used for base tests.
+    private static final String TEST_URL = "http://127.0.0.1:8000";
+    private static final String MOCK_SUCCESS_PATH = "success.txt";
+
+    private static final String MOCK_CRONET_TEST_SUCCESS_URL =
+            "http://mock.http/success.txt";
+    private static final String MOCK_CRONET_TEST_REDIRECT_URL =
+            "http://mock.http/redirect.html";
+    private static final String MOCK_CRONET_TEST_NOTFOUND_URL =
+            "http://mock.http/notfound.html";
+    private static final String MOCK_CRONET_TEST_FAILED_URL =
+            "http://mock.failed.request/-2";
+
+    // Executor for Cronet callbacks

[mmenke, 2014/10/20 21:07:29]

nit:  +.

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

Done.

+    ExecutorService mExecutor = Executors.newSingleThreadExecutor();
+    CronetTestActivity mActivity;
+
+    class SimpleUrlRequestListener implements UrlRequestListener {
+        ConditionVariable mComplete = new ConditionVariable();

[mmenke, 2014/10/20 21:07:29]

nit:  Suggest a blank line between public and non-public variables.

[mmenke, 2014/10/20 21:07:29]

+private?  Without it, I'm not sure if the outer class can access the field or
not.  For top-level classes, it means anything in the same package has access, I
believe.

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

Done.

+        public ResponseInfo mResponseInfo;
+        public UrlRequestException mError;
+
+        public int mHttpResponseDataLength = 0;
+
+        public boolean mOnRedirectCalled = false;
+        public boolean mOnResponseStartedCalled = false;
+        public boolean mOnDataReceivedCalled = false;
+        public boolean mOnCompleteCalled = false;
+        public boolean mOnErrorCalled = false;
+
+        public SimpleUrlRequestListener() {
+        }
+
+        @Override
+        public void onRedirect(UrlRequest request,
+                               ResponseInfo info,
+                               URL newLocation) {
+            mOnRedirectCalled = true;

[mmenke, 2014/10/20 21:07:29]

Check that mOnResponseStartedCalled is false?  (Since we aren't at the top
level, I suggest just keeping a list of errors we've seen, as strings).  Also
suggest adding a bunch of state checks to all of these, for that reason - fine
to have some inline in your tests, too, but having them here allows for more
complete testing of ordering.

[mmenke, 2014/10/20 21:07:29]

Should keep a count here, or better, a list of the info/newLocation pairs we've
received.  Should also check that we have the right number of them, and that
they're correctly populated.

[mmenke, 2014/10/20 21:07:29]

Should check if request == the request we got when we created a request, on all
of these.

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

Done.

+        }
+
+        @Override
+        public void onResponseStarted(UrlRequest request, ResponseInfo info) {
+            mOnResponseStartedCalled = true;
+            mResponseInfo = info;
+        }
+
+        @Override
+        public void onDataReceived(UrlRequest request,
+                                   ResponseInfo info,
+                                   ByteBuffer byteBuffer) {
+            mOnDataReceivedCalled = true;
+            mHttpResponseDataLength += byteBuffer.capacity();
+        }
+
+        @Override
+        public void onComplete(UrlRequest request, ExtendedResponseInfo info) {
+            mOnCompleteCalled = true;
+            mComplete.open();
+        }
+
+        @Override
+        public void onError(UrlRequest request,
+                            ResponseInfo info,
+                            UrlRequestException error) {
+            mOnErrorCalled = true;
+            mError = error;
+            mComplete.open();
+        }
+
+        public void blockForComplete() {
+            mComplete.block();
+        }
+
+        public void resetComplete() {

[mmenke, 2014/10/20 21:07:29]

Not used.

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

Done.

+            mComplete.close();
+        }
+    }
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        mActivity = launchCronetTestApp();
+        // Make sure the activity was created as expected.
+        waitForActiveShellToBeDoneLoading();
+        // AddUrlInterceptors() after native application context is initialized.
+        MockUrlRequestJobUtil.addUrlInterceptors();
+    }
+
+    public SimpleUrlRequestListener startAndWaitForComplete(String url)
+            throws Exception {
+        SimpleUrlRequestListener listener = new SimpleUrlRequestListener();
+        // Create request.
+        UrlRequest urlRequest = mActivity.mUrlRequestFactory.createRequest(
+                url, listener, mExecutor);
+        urlRequest.start();
+        listener.blockForComplete();
+        return listener;
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testSimpleGet() throws Exception {
+        SimpleUrlRequestListener listener = startAndWaitForComplete(TEST_URL);
+        assertEquals(200, listener.mResponseInfo.getHttpStatusCode());
+        assertTrue(listener.mHttpResponseDataLength != 0);

[mmenke, 2014/10/20 21:07:29]

Should check all the fields of ResponseInfo, and the response body, and that
onComplete was called, and that there was no redirect.  Should do a similar
level of testing for most of these.

+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testMockSuccess() throws Exception {
+        SimpleUrlRequestListener listener = startAndWaitForComplete(
+                MOCK_CRONET_TEST_SUCCESS_URL);
+        assertEquals(200, listener.mResponseInfo.getHttpStatusCode());
+        assertTrue(listener.mHttpResponseDataLength != 0);
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testMockRedirect() throws Exception {
+        SimpleUrlRequestListener listener = startAndWaitForComplete(
+                MOCK_CRONET_TEST_REDIRECT_URL);
+        ResponseInfo mResponseInfo = listener.mResponseInfo;
+        assertTrue(listener.mOnRedirectCalled);
+        assertEquals(200, mResponseInfo.getHttpStatusCode());
+        assertEquals(MOCK_CRONET_TEST_SUCCESS_URL,
+                mResponseInfo.getUrl());
+        assertEquals(2, mResponseInfo.getUrlChain().length);
+        assertEquals(MOCK_CRONET_TEST_REDIRECT_URL,
+                mResponseInfo.getUrlChain()[0]);
+        assertEquals(MOCK_CRONET_TEST_SUCCESS_URL,
+                mResponseInfo.getUrlChain()[1]);
+        assertTrue(listener.mHttpResponseDataLength != 0);
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testMockNotFound() throws Exception {
+        SimpleUrlRequestListener listener = startAndWaitForComplete(
+                MOCK_CRONET_TEST_NOTFOUND_URL);
+        assertEquals(404, listener.mResponseInfo.getHttpStatusCode());
+        assertTrue(listener.mHttpResponseDataLength != 0);
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testMockFailed() throws Exception {

[mmenke, 2014/10/20 21:07:29]

I believe this effectively just tests the same case as testMockStartSyncError,
right?

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

Yes, but using different mock jobs.

+        SimpleUrlRequestListener listener = startAndWaitForComplete(
+                MOCK_CRONET_TEST_FAILED_URL);
+        assertEquals(null, listener.mResponseInfo);
+        assertEquals(0, listener.mHttpResponseDataLength);
+        assertNotNull(listener.mError);
+        assertEquals(-2, listener.mError.netError());
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testMockStartSyncError() throws Exception {

[mmenke, 2014/10/20 21:07:29]

This is actually async.

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

Done.

+        SimpleUrlRequestListener listener = startAndWaitForComplete(
+                MockUrlRequestJobUtil.getMockUrlWithFailure(
+                        MOCK_SUCCESS_PATH,
+                        MockUrlRequestJobUtil.FailurePhase.START,
+                        -3));
+        assertNull(listener.mResponseInfo);
+        assertNotNull(listener.mError);
+        assertEquals(-3, listener.mError.netError());
+        assertFalse(listener.mOnRedirectCalled);
+        assertFalse(listener.mOnResponseStartedCalled);
+        assertFalse(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertTrue(listener.mOnErrorCalled);
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testMockReadDataSyncError() throws Exception {
+        SimpleUrlRequestListener listener = startAndWaitForComplete(
+                MockUrlRequestJobUtil.getMockUrlWithFailure(
+                        MOCK_SUCCESS_PATH,
+                        MockUrlRequestJobUtil.FailurePhase.READ_SYNC,
+                        -5));
+        assertEquals(200, listener.mResponseInfo.getHttpStatusCode());
+        assertNotNull(listener.mError);
+        assertEquals(-5, listener.mError.netError());
+        assertFalse(listener.mOnRedirectCalled);
+        assertTrue(listener.mOnResponseStartedCalled);
+        assertFalse(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertTrue(listener.mOnErrorCalled);
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testMockReadDataAsyncError() throws Exception {
+        SimpleUrlRequestListener listener = startAndWaitForComplete(
+                MockUrlRequestJobUtil.getMockUrlWithFailure(
+                        MOCK_SUCCESS_PATH,
+                        MockUrlRequestJobUtil.FailurePhase.READ_ASYNC,
+                        -5));
+        assertEquals(200, listener.mResponseInfo.getHttpStatusCode());
+        assertNotNull(listener.mError);
+        assertEquals(-5, listener.mError.netError());
+        assertFalse(listener.mOnRedirectCalled);
+        assertTrue(listener.mOnResponseStartedCalled);
+        assertFalse(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertTrue(listener.mOnErrorCalled);
+    }
+
+    class CancelingUrlRequestListener extends SimpleUrlRequestListener {
+        public boolean mCancelAfterRedirect = false;
+        public boolean mCancelAfterResponseStarted = false;
+        public boolean mCancelAfterDataReceived = false;
+
+        public boolean mCancelFromRedirect = false;
+        public boolean mCancelFromResponseStarted = false;
+        public boolean mCancelFromDataReceived = false;

[mmenke, 2014/10/20 21:07:29]

My suggestion:  Use one bool to indicate sync vs async cancellation, and 3 bools
to indicate the phase (Or better, an enum, and take them as constructor args). 
Then each branch would just look like:

super.onRedirect(request, info, newLocation);
if (mCancelFromRedirect) {
  CancelSyncOrAsync();
}

Think that makes the code a bit simpler, and less redundant.

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

Done.

+
+        public CancelingUrlRequestListener() {
+        }
+
+        @Override
+        public void onRedirect(UrlRequest request,
+                               ResponseInfo info,
+                               URL newLocation) {
+            super.onRedirect(request, info, newLocation);
+            if (mCancelFromRedirect) {
+                request.cancel();
+                mComplete.open();
+            }
+            maybePostCancel(request, mCancelAfterRedirect);
+        }
+
+        @Override
+        public void onResponseStarted(UrlRequest request, ResponseInfo info) {
+            super.onResponseStarted(request, info);
+            if (mCancelFromResponseStarted) {
+                request.cancel();
+                mComplete.open();
+            }
+            maybePostCancel(request, mCancelAfterResponseStarted);
+        }
+
+        @Override
+        public void onDataReceived(UrlRequest request,
+                                   ResponseInfo info,
+                                   ByteBuffer byteBuffer) {
+            super.onDataReceived(request, info, byteBuffer);
+            if (mCancelFromDataReceived) {
+                request.cancel();
+                mComplete.open();
+            }
+            maybePostCancel(request, mCancelAfterDataReceived);
+        }
+
+        void maybePostCancel(final UrlRequest request, boolean doPostCancel) {
+            if (doPostCancel) {
+                Runnable task = new Runnable() {
+                    public void run() {
+                        request.cancel();
+                        mComplete.open();
+                    }
+                };
+                mExecutor.execute(task);
+            }
+        }
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testCancelAfterOnRedirect() throws Exception {
+        CancelingUrlRequestListener listener =
+                new CancelingUrlRequestListener();
+        listener.mCancelAfterRedirect = true;
+        UrlRequest urlRequest = mActivity.mUrlRequestFactory.createRequest(
+                MOCK_CRONET_TEST_REDIRECT_URL, listener, mExecutor);
+        urlRequest.start();
+        listener.blockForComplete();
+        assertNull(listener.mResponseInfo);
+        assertNull(listener.mError);
+        assertTrue(listener.mOnRedirectCalled);
+        assertFalse(listener.mOnResponseStartedCalled);
+        assertFalse(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertFalse(listener.mOnErrorCalled);
+        assertTrue(urlRequest.isCanceled());
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testCancelAfterOnResponseStarted() throws Exception {
+        CancelingUrlRequestListener listener =
+                new CancelingUrlRequestListener();
+        listener.mCancelAfterResponseStarted = true;
+        UrlRequest urlRequest = mActivity.mUrlRequestFactory.createRequest(
+                MOCK_CRONET_TEST_REDIRECT_URL, listener, mExecutor);
+        urlRequest.start();
+        listener.blockForComplete();
+        assertNotNull(listener.mResponseInfo);
+        assertNull(listener.mError);
+        assertTrue(listener.mOnRedirectCalled);
+        assertTrue(listener.mOnResponseStartedCalled);
+        assertFalse(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertFalse(listener.mOnErrorCalled);
+        assertTrue(urlRequest.isCanceled());
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testCancelAfterOnDataReceived() throws Exception {
+        CancelingUrlRequestListener listener =
+                new CancelingUrlRequestListener();
+        listener.mCancelAfterDataReceived = true;
+        // Create request.
+        UrlRequest urlRequest = mActivity.mUrlRequestFactory.createRequest(
+                MOCK_CRONET_TEST_REDIRECT_URL, listener, mExecutor);
+        urlRequest.start();
+        listener.blockForComplete();
+        assertNotNull(listener.mResponseInfo);
+        assertNull(listener.mError);
+        assertTrue(listener.mOnRedirectCalled);
+        assertTrue(listener.mOnResponseStartedCalled);
+        assertTrue(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertFalse(listener.mOnErrorCalled);
+        assertTrue(urlRequest.isCanceled());
+    }
+
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testCancelFromOnRedirect() throws Exception {
+        CancelingUrlRequestListener listener =
+                new CancelingUrlRequestListener();
+        listener.mCancelFromRedirect = true;
+        UrlRequest urlRequest = mActivity.mUrlRequestFactory.createRequest(
+                MOCK_CRONET_TEST_REDIRECT_URL, listener, mExecutor);
+        urlRequest.start();
+        listener.blockForComplete();
+        assertNull(listener.mResponseInfo);
+        assertNull(listener.mError);
+        assertTrue(listener.mOnRedirectCalled);
+        assertFalse(listener.mOnResponseStartedCalled);
+        assertFalse(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertFalse(listener.mOnErrorCalled);
+        assertTrue(urlRequest.isCanceled());
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testCancelFromOnResponseStarted() throws Exception {
+        CancelingUrlRequestListener listener =
+                new CancelingUrlRequestListener();
+        listener.mCancelFromResponseStarted = true;
+        UrlRequest urlRequest = mActivity.mUrlRequestFactory.createRequest(
+                MOCK_CRONET_TEST_REDIRECT_URL, listener, mExecutor);
+        urlRequest.start();
+        listener.blockForComplete();
+        assertNotNull(listener.mResponseInfo);
+        assertNull(listener.mError);
+        assertTrue(listener.mOnRedirectCalled);
+        assertTrue(listener.mOnResponseStartedCalled);
+        assertFalse(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertFalse(listener.mOnErrorCalled);
+        assertTrue(urlRequest.isCanceled());
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testCancelFromOnDataReceived() throws Exception {
+        CancelingUrlRequestListener listener =
+                new CancelingUrlRequestListener();
+        listener.mCancelFromDataReceived = true;
+        // Create request.
+        UrlRequest urlRequest = mActivity.mUrlRequestFactory.createRequest(
+                MOCK_CRONET_TEST_REDIRECT_URL, listener, mExecutor);
+        urlRequest.start();
+        listener.blockForComplete();
+        assertNotNull(listener.mResponseInfo);
+        assertNull(listener.mError);
+        assertTrue(listener.mOnRedirectCalled);
+        assertTrue(listener.mOnResponseStartedCalled);
+        assertTrue(listener.mOnDataReceivedCalled);
+        assertFalse(listener.mOnCompleteCalled);
+        assertFalse(listener.mOnErrorCalled);
+        assertTrue(urlRequest.isCanceled());

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

Hrm...Is there some way to validate the request was really cancelled?  May not
be a simple way to do it, just want to make sure that's the case.

+    }

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

Since we're handling it, should have yet another set of tests where the listener
throws exceptions, and make sure we get the same exceptions them back on the
listener.

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

Done.

+}