[Cronet] Clean up tests

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

Index: components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
index 016040fa2dfcc79ba87157ef7701b8733eb0ecaa..8075e276e662a4b06c548ecfecdc2c0e7cdd76ae 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlRequestContextTest.java
@@ -21,7 +21,6 @@ import org.chromium.base.PathUtils;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.test.util.Feature;
 import org.chromium.net.TestUrlRequestCallback.ResponseStep;
-import org.chromium.net.impl.CronetEngineBase;
 import org.chromium.net.impl.CronetEngineBuilderImpl;
 import org.chromium.net.impl.CronetLibraryLoader;
 import org.chromium.net.impl.CronetUrlRequestContext;
@@ -73,16 +72,13 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         super.tearDown();
     }
 
-    static class RequestThread extends Thread {
+    class RequestThread extends Thread {
         public TestUrlRequestCallback mCallback;
 
-        final CronetTestFramework mTestFramework;
         final String mUrl;
         final ConditionVariable mRunBlocker;
 
-        public RequestThread(
-                CronetTestFramework testFramework, String url, ConditionVariable runBlocker) {
-            mTestFramework = testFramework;
+        public RequestThread(String url, ConditionVariable runBlocker) {
             mUrl = url;
             mRunBlocker = runBlocker;
         }
@@ -90,7 +86,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         @Override
         public void run() {
             mRunBlocker.block();
-            CronetEngine cronetEngine = mTestFramework.initCronetEngine();
+            CronetEngine cronetEngine = new CronetEngine.Builder(getContext()).build();
             mCallback = new TestUrlRequestCallback();
             UrlRequest.Builder urlRequestBuilder =
                     cronetEngine.newUrlRequestBuilder(mUrl, mCallback, mCallback.getExecutor());
@@ -143,13 +139,12 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
             cronetEngineBuilder = createJavaEngineBuilder();
         }
         cronetEngineBuilder.setUserAgent(userAgentValue);
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithUrlAndCronetEngineBuilder(mUrl, cronetEngineBuilder);
+        final CronetEngine cronetEngine = cronetEngineBuilder.build();
         NativeTestServer.shutdownNativeTestServer(); // startNativeTestServer returns false if it's
         // already running
         assertTrue(NativeTestServer.startNativeTestServer(getContext()));
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
-        UrlRequest.Builder urlRequestBuilder = testFramework.mCronetEngine.newUrlRequestBuilder(
+        UrlRequest.Builder urlRequestBuilder = cronetEngine.newUrlRequestBuilder(
                 NativeTestServer.getEchoHeaderURL(userAgentName), callback, callback.getExecutor());
         urlRequestBuilder.build().start();
         callback.blockForDone();
@@ -213,7 +208,6 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @Feature({"Cronet"})
     @OnlyRunNativeCronet
     public void testShutdownDuringInit() throws Exception {
-        final CronetTestFramework testFramework = startCronetTestFrameworkAndSkipLibraryInit();
         final ConditionVariable block = new ConditionVariable(false);
 
         // Post a task to main thread to block until shutdown is called to test
@@ -236,7 +230,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         // Create new request context, but its initialization on the main thread
         // will be stuck behind blockingTask.
         final CronetUrlRequestContext cronetEngine =
-                (CronetUrlRequestContext) testFramework.initCronetEngine();
+                (CronetUrlRequestContext) new CronetEngine.Builder(getContext()).build();
         // Unblock the main thread, so context gets initialized and shutdown on
         // it.
         block.open();
@@ -255,7 +249,6 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @Feature({"Cronet"})
     @OnlyRunNativeCronet
     public void testInitAndShutdownOnMainThread() throws Exception {
-        final CronetTestFramework testFramework = startCronetTestFrameworkAndSkipLibraryInit();
         final ConditionVariable block = new ConditionVariable(false);
 
         // Post a task to main thread to init and shutdown on the main thread.
@@ -264,7 +257,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
             public void run() {
                 // Create new request context, loading the library.
                 final CronetUrlRequestContext cronetEngine =
-                        (CronetUrlRequestContext) testFramework.initCronetEngine();
+                        (CronetUrlRequestContext) new CronetEngine.Builder(getContext()).build();
                 // Shutdown right after init.
                 cronetEngine.shutdown();
                 // Verify that context is shutdown.
@@ -554,11 +547,10 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     // Tests that if CronetEngine is shut down on the network thread, an appropriate exception
     // is thrown.
     public void testShutDownEngineOnNetworkThread() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(CronetEngine.Builder.HTTP_CACHE_DISK);
+        final CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_DISK);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
         // Make a request to a cacheable resource.
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
 
         final AtomicReference<Throwable> thrown = new AtomicReference<>();
         // Shut down the server.
@@ -570,7 +562,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
                 request.cancel();
                 // Shut down CronetEngine immediately after request is destroyed.
                 try {
-                    testFramework.mCronetEngine.shutdown();
+                    cronetEngine.shutdown();
                 } catch (Exception e) {
                     thrown.set(e);
                 }
@@ -597,7 +589,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         CancelUrlRequestCallback callback = new CancelUrlRequestCallback();
         callback.setAllowDirectExecutor(true);
         UrlRequest.Builder urlRequestBuilder =
-                testFramework.mCronetEngine.newUrlRequestBuilder(url, callback, directExecutor);
+                cronetEngine.newUrlRequestBuilder(url, callback, directExecutor);
         urlRequestBuilder.allowDirectExecutor();
         urlRequestBuilder.build().start();
         callback.blockForDone();
@@ -610,11 +602,10 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     // Tests that if CronetEngine is shut down when reading from disk cache,
     // there isn't a crash. See crbug.com/486120.
     public void testShutDownEngineWhenReadingFromDiskCache() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(CronetEngine.Builder.HTTP_CACHE_DISK);
+        final CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_DISK);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
         // Make a request to a cacheable resource.
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
 
         // Shut down the server.
         NativeTestServer.shutdownNativeTestServer();
@@ -624,7 +615,7 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
                 super.onResponseStarted(request, info);
                 request.cancel();
                 // Shut down CronetEngine immediately after request is destroyed.
-                testFramework.mCronetEngine.shutdown();
+                cronetEngine.shutdown();
             }
 
             @Override
@@ -640,8 +631,8 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
             }
         }
         CancelUrlRequestCallback callback = new CancelUrlRequestCallback();
-        UrlRequest.Builder urlRequestBuilder = testFramework.mCronetEngine.newUrlRequestBuilder(
-                url, callback, callback.getExecutor());
+        UrlRequest.Builder urlRequestBuilder =
+                cronetEngine.newUrlRequestBuilder(url, callback, callback.getExecutor());
         urlRequestBuilder.build().start();
         callback.blockForDone();
         assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
@@ -899,21 +890,15 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         assertEquals(expectedStatusCode, callback.mResponseInfo.getHttpStatusCode());
     }
 
-    private CronetTestFramework startCronetTestFrameworkWithCacheEnabled(int cacheType)
-            throws Exception {
-        String cacheTypeString = "";
+    private CronetEngine createCronetEngine(int cacheType) {

[mgersh, 2017/05/23 18:12:13]

Since this is now used in a test earlier in the file too, it would make more
sense near the beginning of the file, and might want a more descriptive name
like createCronetEngineWithCache or something so it's clear why it's used only
in a few tests.

[pauljensen, 2017/05/25 15:15:15]

Done.

+        CronetEngine.Builder builder = new CronetEngine.Builder(getContext());
         if (cacheType == CronetEngine.Builder.HTTP_CACHE_DISK) {
-            cacheTypeString = CronetTestFramework.CACHE_DISK;
-        } else if (cacheType == CronetEngine.Builder.HTTP_CACHE_DISK_NO_HTTP) {
-            cacheTypeString = CronetTestFramework.CACHE_DISK_NO_HTTP;
-        } else if (cacheType == HTTP_CACHE_IN_MEMORY) {
-            cacheTypeString = CronetTestFramework.CACHE_IN_MEMORY;
+            enableDiskCache(builder);

[mgersh, 2017/05/23 18:12:13]

This way of separating things is a little strange. Instead of having an
enableDiskCache() that sets the storage path and then does the same thing as the
else case, I'd prefer doing just the setStoragePath() in the if.

[pauljensen, 2017/05/25 15:15:15]

Done.

+        } else {
+            builder.enableHttpCache(cacheType, 100 * 1024);
         }
-        String[] commandLineArgs = {CronetTestFramework.CACHE_KEY, cacheTypeString};
-        CronetTestFramework testFramework =
-                startCronetTestFrameworkWithUrlAndCommandLineArgs(null, commandLineArgs);
         assertTrue(NativeTestServer.startNativeTestServer(getContext()));
-        return testFramework;
+        return builder.build();
     }
 
     private void checkRequestCaching(CronetEngine engine, String url, boolean expectCached) {
@@ -938,73 +923,69 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @Feature({"Cronet"})
     @OnlyRunNativeCronet
     public void testEnableHttpCacheDisabled() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(CronetEngine.Builder.HTTP_CACHE_DISABLED);
+        CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_DISABLED);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
     }
 
     @SmallTest
     @Feature({"Cronet"})
     public void testEnableHttpCacheInMemory() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(HTTP_CACHE_IN_MEMORY);
+        CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_IN_MEMORY);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, true);
         NativeTestServer.shutdownNativeTestServer();
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, true);
     }
 
     @SmallTest
     @Feature({"Cronet"})
     public void testEnableHttpCacheDisk() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(CronetEngine.Builder.HTTP_CACHE_DISK);
+        CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_DISK);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, true);
         NativeTestServer.shutdownNativeTestServer();
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, true);
     }
 
     @SmallTest
     @Feature({"Cronet"})
     @OnlyRunNativeCronet
     public void testEnableHttpCacheDiskNoHttp() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(CronetEngine.Builder.HTTP_CACHE_DISABLED);
+        // TODO(pauljensen): This should be testing HTTP_CACHE_DISK_NO_HTTP.
+        CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_DISABLED);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false);
     }
 
     @SmallTest
     @Feature({"Cronet"})
     public void testDisableCache() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(CronetEngine.Builder.HTTP_CACHE_DISK);
+        CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_DISK);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
 
         // When cache is disabled, making a request does not write to the cache.
-        checkRequestCaching(testFramework.mCronetEngine, url, false, true /** disable cache */);
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, false, true /** disable cache */);
+        checkRequestCaching(cronetEngine, url, false);
 
         // When cache is enabled, the second request is cached.
-        checkRequestCaching(testFramework.mCronetEngine, url, false, true /** disable cache */);
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, false, true /** disable cache */);
+        checkRequestCaching(cronetEngine, url, true);
 
         // Shut down the server, next request should have a cached response.
         NativeTestServer.shutdownNativeTestServer();
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, true);
 
         // Cache is disabled after server is shut down, request should fail.
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
-        UrlRequest.Builder urlRequestBuilder = testFramework.mCronetEngine.newUrlRequestBuilder(
-                url, callback, callback.getExecutor());
+        UrlRequest.Builder urlRequestBuilder =
+                cronetEngine.newUrlRequestBuilder(url, callback, callback.getExecutor());
         urlRequestBuilder.disableCache();
         urlRequestBuilder.build().start();
         callback.blockForDone();
@@ -1016,28 +997,24 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     public void testEnableHttpCacheDiskNewEngine() throws Exception {
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithCacheEnabled(CronetEngine.Builder.HTTP_CACHE_DISK);
+        CronetEngine cronetEngine = createCronetEngine(CronetEngine.Builder.HTTP_CACHE_DISK);
         String url = NativeTestServer.getFileURL("/cacheable.txt");
-        checkRequestCaching(testFramework.mCronetEngine, url, false);
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, false);
+        checkRequestCaching(cronetEngine, url, true);
         NativeTestServer.shutdownNativeTestServer();
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        checkRequestCaching(cronetEngine, url, true);
 
         // Shutdown original context and create another that uses the same cache.
-        testFramework.mCronetEngine.shutdown();
-        testFramework.mCronetEngine =
-                (CronetEngineBase) testFramework.getCronetEngineBuilder().build();
-        checkRequestCaching(testFramework.mCronetEngine, url, true);
+        cronetEngine.shutdown();
+        cronetEngine = enableDiskCache(new CronetEngine.Builder(getContext())).build();
+        checkRequestCaching(cronetEngine, url, true);
     }
 
     @SmallTest
     @Feature({"Cronet"})
     public void testInitEngineAndStartRequest() {
-        CronetTestFramework testFramework = startCronetTestFrameworkAndSkipLibraryInit();
-
         // Immediately make a request after initializing the engine.
-        CronetEngine cronetEngine = testFramework.initCronetEngine();
+        CronetEngine cronetEngine = new CronetEngine.Builder(getContext()).build();
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest.Builder urlRequestBuilder =
                 cronetEngine.newUrlRequestBuilder(mUrl, callback, callback.getExecutor());
@@ -1049,10 +1026,9 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     public void testEmptyGetCertVerifierData() {
-        CronetTestFramework testFramework = startCronetTestFrameworkAndSkipLibraryInit();
-
         // Immediately make a request after initializing the engine.
-        ExperimentalCronetEngine cronetEngine = testFramework.initCronetEngine();
+        ExperimentalCronetEngine cronetEngine =
+                new ExperimentalCronetEngine.Builder(getContext()).build();
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest.Builder urlRequestBuilder =
                 cronetEngine.newUrlRequestBuilder(mUrl, callback, callback.getExecutor());
@@ -1074,10 +1050,8 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     public void testInitEngineStartTwoRequests() throws Exception {
-        CronetTestFramework testFramework = startCronetTestFrameworkAndSkipLibraryInit();
-
         // Make two requests after initializing the context.
-        CronetEngine cronetEngine = testFramework.initCronetEngine();
+        CronetEngine cronetEngine = new CronetEngine.Builder(getContext()).build();
         int[] statusCodes = {0, 0};
         String[] urls = {mUrl, mUrl404};
         for (int i = 0; i < 2; i++) {
@@ -1095,12 +1069,10 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     public void testInitTwoEnginesSimultaneously() throws Exception {
-        final CronetTestFramework testFramework = startCronetTestFrameworkAndSkipLibraryInit();
-
         // Threads will block on runBlocker to ensure simultaneous execution.
         ConditionVariable runBlocker = new ConditionVariable(false);
-        RequestThread thread1 = new RequestThread(testFramework, mUrl, runBlocker);
-        RequestThread thread2 = new RequestThread(testFramework, mUrl404, runBlocker);
+        RequestThread thread1 = new RequestThread(mUrl, runBlocker);
+        RequestThread thread2 = new RequestThread(mUrl404, runBlocker);
 
         thread1.start();
         thread2.start();
@@ -1114,11 +1086,9 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     public void testInitTwoEnginesInSequence() throws Exception {
-        final CronetTestFramework testFramework = startCronetTestFrameworkAndSkipLibraryInit();
-
         ConditionVariable runBlocker = new ConditionVariable(true);
-        RequestThread thread1 = new RequestThread(testFramework, mUrl, runBlocker);
-        RequestThread thread2 = new RequestThread(testFramework, mUrl404, runBlocker);
+        RequestThread thread1 = new RequestThread(mUrl, runBlocker);
+        RequestThread thread2 = new RequestThread(mUrl404, runBlocker);
 
         thread1.start();
         thread1.join();
@@ -1267,12 +1237,11 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
                 new JSONObject().put("HostResolverRules", hostResolverRules);
         cronetEngineBuilder.setExperimentalOptions(experimentalOptions.toString());
 
-        final CronetTestFramework testFramework =
-                startCronetTestFrameworkWithUrlAndCronetEngineBuilder(null, cronetEngineBuilder);
+        final CronetEngine cronetEngine = cronetEngineBuilder.build();
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         URL requestUrl =
                 new URL("http", resolverTestHostname, testUrl.getPort(), testUrl.getFile());
-        UrlRequest.Builder urlRequestBuilder = testFramework.mCronetEngine.newUrlRequestBuilder(
+        UrlRequest.Builder urlRequestBuilder = cronetEngine.newUrlRequestBuilder(
                 requestUrl.toString(), callback, callback.getExecutor());
         urlRequestBuilder.build().start();
         callback.blockForDone();