[Cronet] Added a unit test in QuicTest to test async API.

This CL removed the incorrect comment in QuicTest, and
added a second unit test to make sure addQuicHints
works for the new async API.

It made QuicTestServer install test files during start up
to match the new behavior of NativeTestServer, and adds an
assertion to make sure the server has test data.

The CL caches the result returned by
CronetTestActivity#getContextConfig, so we don't
need to go through that path for initializing
mUrlRequestContext, mHttpUrlRequestFactory and
mStreamHandlerFactory, and cleaned up log statements.

Review URL: https://codereview.chromium.org/1138493002

Cr-Commit-Position: refs/heads/master@{#329678}

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

@@ -20,8 +20,18 @@ public class QuicTest extends CronetTestBase {
     @Override
     protected void setUp() throws Exception {
         super.setUp();
-        mActivity = launchCronetTestApp();
-        QuicTestServer.startQuicTestServer(mActivity.getApplicationContext());
+        // Load library first, since we need the Quic test server's URL.
+        System.loadLibrary("cronet_tests");
+        QuicTestServer.startQuicTestServer(getInstrumentation().getTargetContext());
+        UrlRequestContextConfig config = new UrlRequestContextConfig();
+        config.enableQUIC(true);
+        config.addQuicHint(QuicTestServer.getServerHost(), QuicTestServer.getServerPort(),
+                QuicTestServer.getServerPort());
+        config.setExperimentalQuicConnectionOptions("PACE,IW10,FOO,DEADBEEF");
+
+        String[] commandLineArgs = {CronetTestActivity.CONFIG_KEY, config.toString(),
+                CronetTestActivity.CACHE_KEY, CronetTestActivity.CACHE_DISK_NO_HTTP};
+        mActivity = launchCronetTestAppWithUrlAndCommandLineArgs(null, commandLineArgs);
     }
 
     @Override
@@ -32,38 +42,44 @@ public class QuicTest extends CronetTestBase {
 
     @SmallTest
     @Feature({"Cronet"})
-    public void testQuicLoadUrl() throws Exception {
-        HttpUrlRequestFactoryConfig config = new HttpUrlRequestFactoryConfig();
+    public void testQuicLoadUrl_LegacyAPI() throws Exception {
         String quicURL = QuicTestServer.getServerURL() + "/simple.txt";
-        config.enableQUIC(true);
-        config.setLibraryName("cronet_tests");
-        config.addQuicHint(QuicTestServer.getServerHost(),
-                QuicTestServer.getServerPort(), QuicTestServer.getServerPort());
-        config.setExperimentalQuicConnectionOptions("PACE,IW10,FOO,DEADBEEF");
-
-        HttpUrlRequestFactory factory = HttpUrlRequestFactory.createFactory(
-                mActivity.getApplicationContext(), config);
 
         HashMap<String, String> headers = new HashMap<String, String>();
         TestHttpUrlRequestListener listener = new TestHttpUrlRequestListener();
 
-        // Quic always races the first request with SPDY, but the second request
-        // should go through Quic.
-        for (int i = 0; i < 2; i++) {
-            HttpUrlRequest request =
-                    factory.createRequest(
-                            quicURL,
-                            HttpUrlRequest.REQUEST_PRIORITY_MEDIUM,
-                            headers,
-                            listener);
+        // Although the native stack races QUIC and SPDY for the first request,
+        // since there is no http server running on the corresponding TCP port,
+        // QUIC will always succeed with a 200 (see
+        // net::HttpStreamFactoryImpl::Request::OnStreamFailed).
+        HttpUrlRequest request = mActivity.mRequestFactory.createRequest(
+                quicURL, HttpUrlRequest.REQUEST_PRIORITY_MEDIUM, headers, listener);
         request.start();
         listener.blockForComplete();
-            if (listener.mHttpStatusCode == 200) break;
-        }
         assertEquals(200, listener.mHttpStatusCode);
         assertEquals(
                 "This is a simple text file served by QUIC.\n",
                 listener.mResponseAsString);
         assertEquals("quic/1+spdy/3", listener.mNegotiatedProtocol);
     }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testQuicLoadUrl() throws Exception {
+        String quicURL = QuicTestServer.getServerURL() + "/simple.txt";
+        TestUrlRequestListener listener = new TestUrlRequestListener();
+
+        // Although the native stack races QUIC and SPDY for the first request,
+        // since there is no http server running on the corresponding TCP port,
+        // QUIC will always succeed with a 200 (see
+        // net::HttpStreamFactoryImpl::Request::OnStreamFailed).
+        UrlRequest request = mActivity.mUrlRequestContext.createRequest(
+                quicURL, listener, listener.getExecutor());
+        request.start();
+        listener.blockForDone();
+
+        assertEquals(200, listener.mResponseInfo.getHttpStatusCode());
+        assertEquals("This is a simple text file served by QUIC.\n", listener.mResponseAsString);
+        assertEquals("quic/1+spdy/3", listener.mResponseInfo.getNegotiatedProtocol());
+    }
 }

components/cronet/android/test/quic_test_server.cc

@@ -8,6 +8,7 @@
 #include "base/android/jni_string.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
+#include "base/files/file_util.h"
 #include "base/threading/thread.h"
 #include "jni/QuicTestServer_jni.h"
 #include "net/base/ip_endpoint.h"
@@ -30,6 +31,7 @@ void ServeFilesFromDirectory(
   DCHECK(g_quic_server_thread->task_runner()->BelongsToCurrentThread());
   DCHECK(!g_quic_server);
   base::FilePath file_dir = directory.Append("quic_data");
+  CHECK(base::PathExists(file_dir)) << "Quic data does not exist";
   net::tools::QuicInMemoryCache::GetInstance()->InitializeFromDirectory(
       file_dir.value());
   net::IPAddressNumber ip;

components/cronet/android/test/src/org/chromium/net/CronetTestActivity.java

@@ -8,10 +8,11 @@ import android.app.Activity;
 import android.content.Intent;
 import android.os.Bundle;
 import android.os.Environment;
-import android.util.Log;
 
+import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertTrue;
 
+import org.chromium.base.Log;
 import org.chromium.base.PathUtils;
 import org.chromium.base.annotations.SuppressFBWarnings;
 import org.chromium.net.urlconnection.CronetURLStreamHandlerFactory;
@@ -72,6 +73,9 @@ public class CronetTestActivity extends Activity {
 
     int mHttpStatusCode = 0;
 
+    // UrlRequestContextConfig used for this activity.
+    private UrlRequestContextConfig mConfig;
+
     class TestHttpUrlRequestListener implements HttpUrlRequestListener {
         public TestHttpUrlRequestListener() {
         }
@@ -92,6 +96,25 @@ public class CronetTestActivity extends Activity {
         super.onCreate(savedInstanceState);
         prepareTestStorage();
 
+        // Print out extra arguments passed in starting this activity.
+        Intent intent = getIntent();
+        Bundle extras = intent.getExtras();
+        Log.i(TAG, "Cronet extras: " + extras);
+        if (extras != null) {
+            String[] commandLine = extras.getStringArray(COMMAND_LINE_ARGS_KEY);
+            if (commandLine != null) {
+                assertEquals(0, commandLine.length % 2);
+                for (int i = 0; i < commandLine.length / 2; i++) {
+                    Log.i(TAG, "Cronet commandLine %s = %s", commandLine[i * 2],
+                            commandLine[i * 2 + 1]);
+                }
+            }
+        }
+
+        // Initializes UrlRequestContextConfig from commandLine args.
+        mConfig = initializeContextConfig();
+        Log.i(TAG, "Using Config: " + mConfig.toString());
+
         String initString = getCommandLineArg(LIBRARY_INIT_KEY);
         if (LIBRARY_INIT_SKIP.equals(initString)) {
             return;
@@ -99,7 +122,7 @@ public class CronetTestActivity extends Activity {
 
         if (LIBRARY_INIT_WRAPPER.equals(initString)) {
             mStreamHandlerFactory =
-                new CronetURLStreamHandlerFactory(this, getContextConfig());
+                new CronetURLStreamHandlerFactory(this, mConfig);
         }
 
         mUrlRequestContext = initRequestContext();
@@ -143,25 +166,17 @@ public class CronetTestActivity extends Activity {
     }
 
     UrlRequestContextConfig getContextConfig() {
-        UrlRequestContextConfig config = new UrlRequestContextConfig();
-
-        String cacheString = getCommandLineArg(CACHE_KEY);
-        if (CACHE_DISK.equals(cacheString)) {
-            config.setStoragePath(getTestStorage());
-            config.enableHttpCache(UrlRequestContextConfig.HttpCache.DISK, 1000 * 1024);
-        } else if (CACHE_DISK_NO_HTTP.equals(cacheString)) {
-            config.setStoragePath(getTestStorage());
-            config.enableHttpCache(UrlRequestContextConfig.HttpCache.DISK_NO_HTTP, 1000 * 1024);
-        } else if (CACHE_IN_MEMORY.equals(cacheString)) {
-            config.enableHttpCache(UrlRequestContextConfig.HttpCache.IN_MEMORY, 100 * 1024);
+        return mConfig;
     }
+
+    private UrlRequestContextConfig initializeContextConfig() {
+        UrlRequestContextConfig config = new UrlRequestContextConfig();
         config.enableSPDY(true).enableQUIC(true);
 
         // Override config if it is passed from the launcher.
         String configString = getCommandLineArg(CONFIG_KEY);
         if (configString != null) {
             try {
-                Log.i(TAG, "Using Config: " + configString);
                 config = new UrlRequestContextConfig(configString);
             } catch (org.json.JSONException e) {
                 Log.e(TAG, "Invalid Config.", e);
@@ -170,6 +185,17 @@ public class CronetTestActivity extends Activity {
             }
         }
 
+        String cacheString = getCommandLineArg(CACHE_KEY);
+        if (CACHE_DISK.equals(cacheString)) {
+            config.setStoragePath(getTestStorage());
+            config.enableHttpCache(UrlRequestContextConfig.HttpCache.DISK, 1000 * 1024);
+        } else if (CACHE_DISK_NO_HTTP.equals(cacheString)) {
+            config.setStoragePath(getTestStorage());
+            config.enableHttpCache(UrlRequestContextConfig.HttpCache.DISK_NO_HTTP, 1000 * 1024);
+        } else if (CACHE_IN_MEMORY.equals(cacheString)) {
+            config.enableHttpCache(UrlRequestContextConfig.HttpCache.IN_MEMORY, 100 * 1024);
+        }
+
         // Setting this here so it isn't overridden on the command line
         config.setLibraryName("cronet_tests");
         return config;
@@ -177,12 +203,12 @@ public class CronetTestActivity extends Activity {
 
     // Helper function to initialize request context. Also used in testing.
     public UrlRequestContext initRequestContext() {
-        return UrlRequestContext.createContext(this, getContextConfig());
+        return UrlRequestContext.createContext(this, mConfig);
     }
 
     // Helper function to initialize request factory. Also used in testing.
     public HttpUrlRequestFactory initRequestFactory() {
-        return HttpUrlRequestFactory.createFactory(this, getContextConfig());
+        return HttpUrlRequestFactory.createFactory(this, mConfig);
     }
 
     private static String getUrlFromIntent(Intent intent) {
@@ -192,13 +218,10 @@ public class CronetTestActivity extends Activity {
     private String getCommandLineArg(String key) {
         Intent intent = getIntent();
         Bundle extras = intent.getExtras();
-        Log.i(TAG, "Cronet extras: " + extras);
         if (extras != null) {
             String[] commandLine = extras.getStringArray(COMMAND_LINE_ARGS_KEY);
             if (commandLine != null) {
                 for (int i = 0; i < commandLine.length; ++i) {
-                    Log.i(TAG,
-                            "Cronet commandLine[" + i + "]=" + commandLine[i]);
                     if (commandLine[i].equals(key)) {
                         return commandLine[++i];
                     }

components/cronet/android/test/src/org/chromium/net/QuicTestServer.java

@@ -9,6 +9,7 @@ import android.os.ConditionVariable;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
+import org.chromium.base.Log;
 
 /**
  * Wrapper class to start a Quic test server.
@@ -16,8 +17,10 @@ import org.chromium.base.JNINamespace;
 @JNINamespace("cronet")
 public final class QuicTestServer {
     private static final ConditionVariable sBlock = new ConditionVariable();
+    private static final String TAG = Log.makeTag("QuicTestServer");
 
     public static void startQuicTestServer(Context context) {
+        TestFilesInstaller.installIfNeeded(context);
         nativeStartQuicTestServer(TestFilesInstaller.getInstalledPath(context));
         sBlock.block();
     }
@@ -41,6 +44,7 @@ public final class QuicTestServer {
 
     @CalledByNative
     private static void onServerStarted() {
+        Log.i(TAG, "Quic server started.");
         sBlock.open();
     }