[Cronet] Remove CronetEngine.Builder JSON serial/deserialization APIs

This CL removes test dependencies on the serial/deserialization of
CronetEngine.Builders, and removes the related public APIs.
Internally CronetEngine.Builders are still serial/deserialized during
CronetUrlRequestContext construction but at least this doesn't
complicate the public APIs.
Perhaps in the future the internal serial/deserialization can be removed
in exchange for directly passing configuration options; this is a
large change however as the options are passed across several functions
and a thread hop.
Perhaps in the future the use of command line arguments to
CronetTestFramework can also be removed; for now I'm removing the
CONFIG_KEY argument.

BUG=531538

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

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

13 files changed, 64 insertions, 103 deletions

diff --git a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
index 9316a7c..4307b35 100644
--- a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
+++ b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
@@ -53,19 +53,6 @@ public abstract class CronetEngine {
         }
 
         /**
-         * Creates a config from a JSON string, which was serialized using
-         * {@link #toString}.
-         *
-         * @param context Android {@link Context} for engine to use.
-         * @param json JSON string of configuration parameters, which was
-         *         serialized using {@link #toString}.
-         */
-        public Builder(Context context, String json) throws JSONException {
-            mConfig = new JSONObject(json);
-            mContext = context;
-        }
-
-        /**
          * Constructs a User-Agent string including Cronet version, and
          * application name and version.
          *
@@ -334,10 +321,9 @@ public abstract class CronetEngine {
         }
 
         /**
-         * Get JSON string representation of the builder.
+         * Gets a JSON string representation of the builder.
          */
-        @Override
-        public String toString() {
+        String toJSONString() {
             return mConfig.toString();
         }
 
diff --git a/components/cronet/android/api/src/org/chromium/net/HttpUrlRequestFactoryConfig.java b/components/cronet/android/api/src/org/chromium/net/HttpUrlRequestFactoryConfig.java
index 65434e4..6f7c158 100644
--- a/components/cronet/android/api/src/org/chromium/net/HttpUrlRequestFactoryConfig.java
+++ b/components/cronet/android/api/src/org/chromium/net/HttpUrlRequestFactoryConfig.java
@@ -4,8 +4,6 @@
 
 package org.chromium.net;
 
-import org.json.JSONException;
-
 /**
  * A config for HttpUrlRequestFactory, which allows runtime configuration of
  * HttpUrlRequestFactory.
@@ -20,11 +18,4 @@ public class HttpUrlRequestFactoryConfig extends UrlRequestContextConfig {
     public HttpUrlRequestFactoryConfig() {
         super();
     }
-
-    /**
-     * Create config from json serialized using @toString.
-     */
-    public HttpUrlRequestFactoryConfig(String json) throws JSONException {
-        super(json);
-    }
 }
diff --git a/components/cronet/android/api/src/org/chromium/net/UrlRequestContextConfig.java b/components/cronet/android/api/src/org/chromium/net/UrlRequestContextConfig.java
index ff0c777..0e55fc6 100644
--- a/components/cronet/android/api/src/org/chromium/net/UrlRequestContextConfig.java
+++ b/components/cronet/android/api/src/org/chromium/net/UrlRequestContextConfig.java
@@ -4,8 +4,6 @@
 
 package org.chromium.net;
 
-import org.json.JSONException;
-
 /**
  * A config for CronetEngine, which allows runtime configuration of
  * CronetEngine.
@@ -17,10 +15,4 @@ public class UrlRequestContextConfig extends CronetEngine.Builder {
         // or ChromiumUrlRequestContext is created.
         super(null);
     }
-
-    public UrlRequestContextConfig(String json) throws JSONException {
-        // Context will be passed in later when the ChromiumUrlRequestFactory
-        // or ChromiumUrlRequestContext is created.
-        super(null, json);
-    }
 }
diff --git a/components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java b/components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java
index 6c875e1..40d6642 100644
--- a/components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java
+++ b/components/cronet/android/java/src/org/chromium/net/ChromiumUrlRequestContext.java
@@ -36,8 +36,8 @@ public class ChromiumUrlRequestContext {
     protected ChromiumUrlRequestContext(
             final Context context, String userAgent, CronetEngine.Builder config) {
         CronetLibraryLoader.ensureInitialized(context, config);
-        mChromiumUrlRequestContextAdapter =
-                nativeCreateRequestContextAdapter(userAgent, getLoggingLevel(), config.toString());
+        mChromiumUrlRequestContextAdapter = nativeCreateRequestContextAdapter(
+                userAgent, getLoggingLevel(), config.toJSONString());
         if (mChromiumUrlRequestContextAdapter == 0) {
             throw new NullPointerException("Context Adapter creation failed");
         }
diff --git a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
index 84ac0a8..1faf35f 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
@@ -73,7 +73,7 @@ class CronetUrlRequestContext extends CronetEngine {
     public CronetUrlRequestContext(CronetEngine.Builder builder) {
         CronetLibraryLoader.ensureInitialized(builder.getContext(), builder);
         nativeSetMinLogLevel(getLoggingLevel());
-        mUrlRequestContextAdapter = nativeCreateRequestContextAdapter(builder.toString());
+        mUrlRequestContextAdapter = nativeCreateRequestContextAdapter(builder.toJSONString());
         if (mUrlRequestContextAdapter == 0) {
             throw new NullPointerException("Context Adapter creation failed.");
         }
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java
index a7c5856..ed8f586 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetTestBase.java
@@ -6,6 +6,8 @@ package org.chromium.net;
 
 import android.test.AndroidTestCase;
 
+import org.chromium.base.PathUtils;
+
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
@@ -17,13 +19,21 @@ import java.net.URL;
  * Base test class for all CronetTest based tests.
  */
 public class CronetTestBase extends AndroidTestCase {
+    private static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "cronet_test";
+
     private CronetTestFramework mCronetTestFramework;
 
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        PathUtils.setPrivateDataDirectorySuffix(PRIVATE_DATA_DIRECTORY_SUFFIX, getContext());
+    }
+
     /**
      * Starts the CronetTest framework.
      */
     protected CronetTestFramework startCronetTestFramework() {
-        return startCronetTestFrameworkWithUrlAndCommandLineArgs(null, null);
+        return startCronetTestFrameworkWithUrlAndCronetEngineBuilder(null, null);
     }
 
     /**
@@ -31,7 +41,17 @@ public class CronetTestBase extends AndroidTestCase {
      * null.
      */
     protected CronetTestFramework startCronetTestFrameworkWithUrl(String url) {
-        return startCronetTestFrameworkWithUrlAndCommandLineArgs(url, null);
+        return startCronetTestFrameworkWithUrlAndCronetEngineBuilder(url, null);
+    }
+
+    /**
+     * Starts the CronetTest framework using the provided CronetEngine.Builder
+     * and loads the given URL. The URL can be null.
+     */
+    protected CronetTestFramework startCronetTestFrameworkWithUrlAndCronetEngineBuilder(
+            String url, CronetEngine.Builder builder) {
+        mCronetTestFramework = new CronetTestFramework(url, null, getContext(), builder);
+        return mCronetTestFramework;
     }
 
     /**
@@ -40,7 +60,7 @@ public class CronetTestBase extends AndroidTestCase {
      */
     protected CronetTestFramework startCronetTestFrameworkWithUrlAndCommandLineArgs(
             String url, String[] commandLineArgs) {
-        mCronetTestFramework = new CronetTestFramework(url, commandLineArgs, getContext());
+        mCronetTestFramework = new CronetTestFramework(url, commandLineArgs, getContext(), null);
         return mCronetTestFramework;
     }
 
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 6603afc..9b023ac 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
@@ -128,9 +128,8 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         CronetEngine.Builder cronetEngineBuilder = new CronetEngine.Builder(getContext());
         cronetEngineBuilder.setUserAgent(userAgentValue);
         cronetEngineBuilder.setLibraryName("cronet_tests");
-        String[] commandLineArgs = {CronetTestFramework.CONFIG_KEY, cronetEngineBuilder.toString()};
-        mTestFramework =
-                startCronetTestFrameworkWithUrlAndCommandLineArgs(TEST_URL, commandLineArgs);
+        mTestFramework = startCronetTestFrameworkWithUrlAndCronetEngineBuilder(
+                TEST_URL, cronetEngineBuilder);
         assertTrue(NativeTestServer.startNativeTestServer(getContext()));
         TestUrlRequestCallback callback = new TestUrlRequestCallback();
         UrlRequest.Builder urlRequestBuilder =
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlTest.java
index c9ea589..86ac30a 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/CronetUrlTest.java
@@ -133,14 +133,12 @@ public class CronetUrlTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     public void testLegacyLoadUrl() throws Exception {
-        HttpUrlRequestFactoryConfig config = new HttpUrlRequestFactoryConfig();
-        config.enableLegacyMode(true);
+        CronetEngine.Builder builder = new CronetEngine.Builder(getContext());
+        builder.enableLegacyMode(true);
         // TODO(mef) fix tests so that library isn't loaded for legacy stack
-        config.setLibraryName("cronet_tests");
 
-        String[] commandLineArgs = {CronetTestFramework.CONFIG_KEY, config.toString()};
         CronetTestFramework testFramework =
-                startCronetTestFrameworkWithUrlAndCommandLineArgs(URL, commandLineArgs);
+                startCronetTestFrameworkWithUrlAndCronetEngineBuilder(URL, builder);
 
         // Make sure that the URL is set as expected.
         assertEquals(URL, testFramework.getUrl());
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/HttpUrlRequestFactoryTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/HttpUrlRequestFactoryTest.java
index d7791be..0e758e7 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/HttpUrlRequestFactoryTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/HttpUrlRequestFactoryTest.java
@@ -24,14 +24,13 @@ public class HttpUrlRequestFactoryTest extends CronetTestBase {
     @SmallTest
     @Feature({"Cronet"})
     public void testCreateFactory() throws Throwable {
-        HttpUrlRequestFactoryConfig config = new HttpUrlRequestFactoryConfig();
-        config.enableQUIC(true);
-        config.addQuicHint("www.google.com", 443, 443);
-        config.addQuicHint("www.youtube.com", 443, 443);
-        config.setLibraryName("cronet_tests");
-        String[] commandLineArgs = {CronetTestFramework.CONFIG_KEY, config.toString()};
+        CronetEngine.Builder builder = new CronetEngine.Builder(getContext());
+        builder.enableQUIC(true);
+        builder.addQuicHint("www.google.com", 443, 443);
+        builder.addQuicHint("www.youtube.com", 443, 443);
+        builder.setLibraryName("cronet_tests");
         CronetTestFramework testFramework =
-                startCronetTestFrameworkWithUrlAndCommandLineArgs(URL, commandLineArgs);
+                startCronetTestFrameworkWithUrlAndCronetEngineBuilder(URL, builder);
         HttpUrlRequestFactory factory = testFramework.mRequestFactory;
         assertNotNull("Factory should be created", factory);
         assertTrue("Factory should be Chromium/n.n.n.n@r but is "
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/QuicTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/QuicTest.java
index 458554b..f18eb36 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/QuicTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/QuicTest.java
@@ -38,10 +38,10 @@ public class QuicTest extends CronetTestBase {
                 QuicTestServer.getServerPort());
         builder.setExperimentalQuicConnectionOptions("PACE,IW10,FOO,DEADBEEF");
         builder.setMockCertVerifierForTesting(MockCertVerifier.createMockCertVerifier(CERTS_USED));
+        builder.setStoragePath(CronetTestFramework.getTestStorage(getContext()));
+        builder.enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISK_NO_HTTP, 1000 * 1024);
 
-        String[] commandLineArgs = {CronetTestFramework.CONFIG_KEY, builder.toString(),
-                CronetTestFramework.CACHE_KEY, CronetTestFramework.CACHE_DISK_NO_HTTP};
-        mTestFramework = startCronetTestFrameworkWithUrlAndCommandLineArgs(null, commandLineArgs);
+        mTestFramework = startCronetTestFrameworkWithUrlAndCronetEngineBuilder(null, builder);
     }
 
     @Override
@@ -124,7 +124,7 @@ public class QuicTest extends CronetTestBase {
 
         // Make another request using a new context but with no QUIC hints.
         CronetEngine.Builder builder = new CronetEngine.Builder(getContext());
-        builder.setStoragePath(mTestFramework.getTestStorage());
+        builder.setStoragePath(CronetTestFramework.getTestStorage(getContext()));
         builder.enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISK, 1000 * 1024);
         builder.enableQUIC(true);
         builder.setMockCertVerifierForTesting(MockCertVerifier.createMockCertVerifier(CERTS_USED));
@@ -148,7 +148,7 @@ public class QuicTest extends CronetTestBase {
     // Returns whether a file contains a particular string.
     @SuppressFBWarnings("OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE")
     private boolean fileContainsString(String filename, String content) throws IOException {
-        File file = new File(mTestFramework.getTestStorage() + "/" + filename);
+        File file = new File(CronetTestFramework.getTestStorage(getContext()) + "/" + filename);
         FileInputStream fileInputStream = new FileInputStream(file);
         byte[] data = new byte[(int) file.length()];
         fileInputStream.read(data);
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/SdchTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/SdchTest.java
index ee6d898..e5517ba 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/SdchTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/SdchTest.java
@@ -267,7 +267,7 @@ public class SdchTest extends CronetTestBase {
     // Returns whether a file contains a particular string.
     private boolean fileContainsString(String filename, String content) throws IOException {
         BufferedReader reader = new BufferedReader(
-                new FileReader(mTestFramework.getTestStorage() + "/" + filename));
+                new FileReader(CronetTestFramework.getTestStorage(getContext()) + "/" + filename));
         String line;
         while ((line = reader.readLine()) != null) {
             if (line.contains(content)) {
diff --git a/components/cronet/android/test/src/org/chromium/net/CronetTestApplication.java b/components/cronet/android/test/src/org/chromium/net/CronetTestApplication.java
index 4169547..1185ecd 100644
--- a/components/cronet/android/test/src/org/chromium/net/CronetTestApplication.java
+++ b/components/cronet/android/test/src/org/chromium/net/CronetTestApplication.java
@@ -5,28 +5,9 @@
 package org.chromium.net;
 
 import android.app.Application;
-import android.content.Context;
-import android.util.Log;
-
-import org.chromium.base.PathUtils;
 
 /**
  * Application for managing the Cronet Test.
  */
 public class CronetTestApplication extends Application {
-    private static final String TAG = "CronetTestApplication";
-
-    private static final String PRIVATE_DATA_DIRECTORY_SUFFIX = "cronet_test";
-
-    @Override
-    public void onCreate() {
-        super.onCreate();
-        initializeApplicationParameters(this);
-    }
-
-    public static void initializeApplicationParameters(Context context) {
-        PathUtils.setPrivateDataDirectorySuffix(PRIVATE_DATA_DIRECTORY_SUFFIX, context);
-        Log.i(TAG, "CronetTestApplication.initializeApplicationParameters()"
-                + " success.");
-    }
 }
diff --git a/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java b/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java
index 2428a53..4870946 100644
--- a/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java
+++ b/components/cronet/android/test/src/org/chromium/net/CronetTestFramework.java
@@ -10,7 +10,6 @@ import android.os.Environment;
 
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertTrue;
-import static junit.framework.Assert.fail;
 
 import org.chromium.base.Log;
 import org.chromium.base.PathUtils;
@@ -33,7 +32,6 @@ public class CronetTestFramework {
 
     public static final String COMMAND_LINE_ARGS_KEY = "commandLineArgs";
     public static final String POST_DATA_KEY = "postData";
-    public static final String CONFIG_KEY = "config";
     public static final String CACHE_KEY = "cache";
     public static final String SDCH_KEY = "sdch";
 
@@ -104,7 +102,8 @@ public class CronetTestFramework {
 
     // TODO(crbug.com/547160): Fix this findbugs error and remove the suppression.
     @SuppressFBWarnings("EI_EXPOSE_REP2")
-    public CronetTestFramework(String appUrl, String[] commandLine, Context context) {
+    public CronetTestFramework(
+            String appUrl, String[] commandLine, Context context, CronetEngine.Builder builder) {
         mCommandLine = commandLine;
         mContext = context;
         prepareTestStorage();
@@ -119,8 +118,7 @@ public class CronetTestFramework {
         }
 
         // Initializes CronetEngine.Builder from commandLine args.
-        mCronetEngineBuilder = initializeCronetEngineBuilder();
-        Log.i(TAG, "Using Config: " + mCronetEngineBuilder.toString());
+        mCronetEngineBuilder = initializeCronetEngineBuilderWithPresuppliedBuilder(builder);
 
         String initString = getCommandLineArg(LIBRARY_INIT_KEY);
         if (LIBRARY_INIT_SKIP.equals(initString)) {
@@ -149,15 +147,15 @@ public class CronetTestFramework {
      * Prepares the path for the test storage (http cache, QUIC server info).
      */
     private void prepareTestStorage() {
-        File storage = new File(getTestStorage());
+        File storage = new File(getTestStorage(mContext));
         if (storage.exists()) {
             assertTrue(recursiveDelete(storage));
         }
         assertTrue(storage.mkdir());
     }
 
-    String getTestStorage() {
-        return PathUtils.getDataDirectory(mContext) + "/test_storage";
+    static String getTestStorage(Context context) {
+        return PathUtils.getDataDirectory(context) + "/test_storage";
     }
 
     @SuppressFBWarnings("NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE")
@@ -176,31 +174,28 @@ public class CronetTestFramework {
         return mCronetEngineBuilder;
     }
 
-    private CronetEngine.Builder initializeCronetEngineBuilder() {
-        return createCronetEngineBuilder(mContext);
+    private CronetEngine.Builder initializeCronetEngineBuilderWithPresuppliedBuilder(
+            CronetEngine.Builder builder) {
+        return createCronetEngineBuilderWithPresuppliedBuilder(mContext, builder);
     }
 
     CronetEngine.Builder createCronetEngineBuilder(Context context) {
-        CronetEngine.Builder cronetEngineBuilder = new CronetEngine.Builder(context);
-        cronetEngineBuilder.enableHTTP2(true).enableQUIC(true);
-
-        // Override config if it is passed from the launcher.
-        String configString = getCommandLineArg(CONFIG_KEY);
-        if (configString != null) {
-            try {
-                cronetEngineBuilder = new CronetEngine.Builder(mContext, configString);
-            } catch (org.json.JSONException e) {
-                fail("Invalid Config." + e);
-                return null;
-            }
+        return createCronetEngineBuilderWithPresuppliedBuilder(context, null);
+    }
+
+    private CronetEngine.Builder createCronetEngineBuilderWithPresuppliedBuilder(
+            Context context, CronetEngine.Builder cronetEngineBuilder) {
+        if (cronetEngineBuilder == null) {
+            cronetEngineBuilder = new CronetEngine.Builder(context);
+            cronetEngineBuilder.enableHTTP2(true).enableQUIC(true);
         }
 
         String cacheString = getCommandLineArg(CACHE_KEY);
         if (CACHE_DISK.equals(cacheString)) {
-            cronetEngineBuilder.setStoragePath(getTestStorage());
+            cronetEngineBuilder.setStoragePath(getTestStorage(context));
             cronetEngineBuilder.enableHttpCache(CronetEngine.Builder.HTTP_CACHE_DISK, 1000 * 1024);
         } else if (CACHE_DISK_NO_HTTP.equals(cacheString)) {
-            cronetEngineBuilder.setStoragePath(getTestStorage());
+            cronetEngineBuilder.setStoragePath(getTestStorage(context));
             cronetEngineBuilder.enableHttpCache(
                     CronetEngine.Builder.HTTP_CACHE_DISK_NO_HTTP, 1000 * 1024);
         } else if (CACHE_IN_MEMORY.equals(cacheString)) {