Change RequestFinishedListener to provide executor

components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedListenerTest.java

Index: components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedListenerTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedListenerTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedListenerTest.java
new file mode 100644
index 0000000000000000000000000000000000000000..09dfbb347b3d2b0a214dc60755e05ecde02818be
--- /dev/null
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/RequestFinishedListenerTest.java
@@ -0,0 +1,158 @@
+// Copyright 2016 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 static org.chromium.base.CollectionUtil.newHashSet;
+
+import android.test.suitebuilder.annotation.SmallTest;
+
+import org.chromium.base.test.util.Feature;
+import org.chromium.net.CronetEngine.UrlRequestInfo;
+import org.chromium.net.test.EmbeddedTestServer;
+
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.NoSuchElementException;
+import java.util.concurrent.Executor;
+
+/**
+ * Test RequestFinishedListener and the metrics information it provides

[tbansal1, 2016/07/26 20:37:27]

Missing period at the end.

[mgersh, 2016/07/27 20:54:12]

Done.

+ */
+public class RequestFinishedListenerTest extends CronetTestBase {
+    CronetTestFramework mTestFramework;
+    private EmbeddedTestServer mTestServer;
+    private String mUrl;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        mTestServer = EmbeddedTestServer.createAndStartDefaultServer(getContext());
+        mUrl = mTestServer.getURL("/echo?status=200");
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        mTestServer.stopAndDestroyServer();
+        super.tearDown();
+    }
+
+    static class TestExecutor implements Executor {
+        private final LinkedList<Runnable> mTaskQueue = new LinkedList<Runnable>();
+
+        @Override
+        public void execute(Runnable task) {
+            mTaskQueue.add(task);
+        }
+
+        public void runAllTasks() {
+            try {
+                while (mTaskQueue.size() > 0) {
+                    mTaskQueue.remove().run();
+                }
+            } catch (NoSuchElementException e) {

[xunjieli, 2016/07/26 21:11:11]

Maybe rethrow this exception as a runtime exception. 

[mgersh, 2016/07/27 20:54:13]

Done.

+            }
+        }
+    }
+
+    private static class TestRequestFinishedListener extends RequestFinishedListener {
+        private UrlRequestInfo mRequestInfo = null;

[xunjieli, 2016/07/26 21:11:09]

nit: omit initialization to null and rely on default initialization.

[mgersh, 2016/07/27 20:54:13]

Done.

+
+        public TestRequestFinishedListener(Executor executor) {
+            super(executor);
+        }
+
+        @Override
+        public void onRequestFinished(UrlRequestInfo requestInfo) {
+            assertNull("onRequestFinished called repeatedly", mRequestInfo);
+            assertNotNull(requestInfo);
+            mRequestInfo = requestInfo;
+        }
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    @SuppressWarnings("deprecation")
+    public void testRequestFinishedListener() throws Exception {
+        mTestFramework = startCronetTestFramework();
+        TestExecutor testExecutor = new TestExecutor();
+        TestRequestFinishedListener requestFinishedListener =
+                new TestRequestFinishedListener(testExecutor);
+        mTestFramework.mCronetEngine.addRequestFinishedListener(requestFinishedListener);
+        TestUrlRequestCallback callback = new TestUrlRequestCallback();
+        UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(
+                mUrl, callback, callback.getExecutor(), mTestFramework.mCronetEngine);
+        urlRequestBuilder.addRequestAnnotation("request annotation")
+                .addRequestAnnotation(this)
+                .build()
+                .start();
+        callback.blockForDone();
+        testExecutor.runAllTasks();

[xunjieli, 2016/07/26 21:11:10]

Maybe add a test where the executor runs the task directly, and one test where
the executor runs the task on a different thread than the test thread.

[mgersh, 2016/07/27 20:54:13]

Done.

+
+        CronetEngine.UrlRequestInfo requestInfo = requestFinishedListener.mRequestInfo;
+        assertNotNull("RequestFinishedListener must be called", requestInfo);
+        assertEquals(mUrl, requestInfo.getUrl());
+        assertNotNull(requestInfo.getResponseInfo());
+        assertEquals(newHashSet("request annotation", this), // Use sets for unordered comparison.
+                new HashSet<Object>(requestInfo.getAnnotations()));
+        CronetEngine.UrlRequestMetrics metrics = requestInfo.getMetrics();
+        assertNotNull("UrlRequestInfo.getMetrics() must not be null", metrics);
+        assertTrue(metrics.getTotalTimeMs() > 0);
+        assertTrue(metrics.getTotalTimeMs() >= metrics.getTtfbMs());
+        assertTrue(metrics.getReceivedBytesCount() > 0);
+        mTestFramework.mCronetEngine.shutdown();
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    @SuppressWarnings("deprecation")
+    public void testRequestFinishedListenerFailedRequest() throws Exception {
+        String connectionRefusedUrl = "http://127.0.0.1:3";
+        mTestFramework = startCronetTestFramework();
+        TestExecutor testExecutor = new TestExecutor();
+        TestRequestFinishedListener requestFinishedListener =
+                new TestRequestFinishedListener(testExecutor);
+        mTestFramework.mCronetEngine.addRequestFinishedListener(requestFinishedListener);
+        TestUrlRequestCallback callback = new TestUrlRequestCallback();
+        UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(connectionRefusedUrl,
+                callback, callback.getExecutor(), mTestFramework.mCronetEngine);
+        urlRequestBuilder.build().start();
+        callback.blockForDone();
+        assertTrue(callback.mOnErrorCalled);
+        testExecutor.runAllTasks();
+
+        CronetEngine.UrlRequestInfo requestInfo = requestFinishedListener.mRequestInfo;
+        assertNotNull("RequestFinishedListener must be called", requestInfo);
+        assertEquals(connectionRefusedUrl, requestInfo.getUrl());
+        assertTrue(requestInfo.getAnnotations().isEmpty());
+        CronetEngine.UrlRequestMetrics metrics = requestInfo.getMetrics();
+        assertNotNull("UrlRequestInfo.getMetrics() must not be null", metrics);
+        assertTrue(metrics.getTotalTimeMs() > 0);
+        assertNull(metrics.getTtfbMs());
+        assertTrue(metrics.getReceivedBytesCount() == null || metrics.getReceivedBytesCount() == 0);
+        mTestFramework.mCronetEngine.shutdown();
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    @SuppressWarnings("deprecation")
+    public void testRequestFinishedListenerRemoved() throws Exception {
+        mTestFramework = startCronetTestFramework();
+        TestExecutor testExecutor = new TestExecutor();
+        TestRequestFinishedListener requestFinishedListener =
+                new TestRequestFinishedListener(testExecutor);
+        mTestFramework.mCronetEngine.addRequestFinishedListener(requestFinishedListener);
+        mTestFramework.mCronetEngine.removeRequestFinishedListener(requestFinishedListener);
+        TestUrlRequestCallback callback = new TestUrlRequestCallback();
+        UrlRequest.Builder urlRequestBuilder = new UrlRequest.Builder(
+                mUrl, callback, callback.getExecutor(), mTestFramework.mCronetEngine);
+        urlRequestBuilder.build().start();
+        callback.blockForDone();
+        testExecutor.runAllTasks();
+
+        assertNull(
+                "RequestFinishedListener must not be called", requestFinishedListener.mRequestInfo);
+        mTestFramework.mCronetEngine.shutdown();
+    }
+}