[Cronet] Remove JSON serialization of CronetEngine.Builder

components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java

Index: components/cronet/android/java/src/org/chromium/net/CronetUrlRequestContext.java
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 227bd41f9d8c4c389f99e220bfc1e45b54d049fc..8bc8584ea814a35a188eba4e0d0765b912618f3f 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,8 @@ class CronetUrlRequestContext extends CronetEngine {
     public CronetUrlRequestContext(CronetEngine.Builder builder) {
         CronetLibraryLoader.ensureInitialized(builder.getContext(), builder);
         nativeSetMinLogLevel(getLoggingLevel());
-        mUrlRequestContextAdapter = nativeCreateRequestContextAdapter(builder.toJSONString());
+        mUrlRequestContextAdapter =
+                nativeCreateRequestContextAdapter(createNativeUrlRequestContextConfig(builder));
         if (mUrlRequestContextAdapter == 0) {
             throw new NullPointerException("Context Adapter creation failed.");
         }
@@ -98,6 +99,21 @@ class CronetUrlRequestContext extends CronetEngine {
         }
     }
 
+    static long createNativeUrlRequestContextConfig(CronetEngine.Builder builder) {
+        final long urlRequestContextConfig = nativeCreateRequestContextConfig(
+                builder.getUserAgent(), builder.storagePath(), builder.quicEnabled(),
+                builder.http2Enabled(), builder.sdchEnabled(), builder.dataReductionProxyKey(),
+                builder.dataReductionProxyPrimaryProxy(), builder.dataReductionProxyFallbackProxy(),
+                builder.dataReductionProxySecureProxyCheckUrl(), builder.cacheDisabled(),
+                builder.httpCacheMode(), builder.httpCacheMaxSize(), builder.experimentalOptions(),
+                builder.mockCertVerifier());
+        for (Builder.QuicHint quicHint : builder.quicHints()) {
+            nativeAddQuicHint(urlRequestContextConfig, quicHint.mHost, quicHint.mPort,

[xunjieli, 2015/12/01 17:30:38]

I don't really like adding multiple JNI calls here. Can we consolidate them into
some structured info and pass it at once with the rest of the args?

[pauljensen, 2015/12/02 03:50:01]

I don't really like it either, but I'm not too worried about JNI as the overhead
is very low, like around 1 microsecond.
I don't like the alternatives any better:
1. could pass an array of classes but we'd have to use GetFieldID() which is
horribly slow as it uses reflection.
2. could pass arrays of strings (for the hosts) and arrays of ints (for the
ports) but that leads to a hideous looking API and AFAIK there is no handy
function for converting from Java arrays of strings to std::vectors of
std::strings.
I'm open to alternate suggestions.

+                    quicHint.mAlternatePort);
+        }
+        return urlRequestContextConfig;
+    }
+
     @Override
     public UrlRequest createRequest(String url, UrlRequest.Callback callback, Executor executor) {
         synchronized (mLock) {
@@ -400,7 +416,17 @@ class CronetUrlRequestContext extends CronetEngine {
     }
 
     // Native methods are implemented in cronet_url_request_context_adapter.cc.
-    private static native long nativeCreateRequestContextAdapter(String config);
+    private static native long nativeCreateRequestContextConfig(String userAgent,
+            String storagePath, boolean quicEnabled, boolean http2Enabled, boolean sdchEnabled,
+            String dataReductionProxyKey, String dataReductionProxyPrimaryProxy,
+            String dataReductionProxyFallbackProxy, String dataReductionProxySecureProxyCheckUrl,
+            boolean disableCache, int httpCacheMode, long httpCacheMaxSize,
+            String experimentalOptions, long mockCertVerifier);
+
+    private static native void nativeAddQuicHint(
+            long urlRequestContextConfig, String host, int port, int alternatePort);
+
+    private static native long nativeCreateRequestContextAdapter(long urlRequestContextConfig);

[xunjieli, 2015/12/01 17:30:38]

Not sure whether we should consolidate nativeCreateRequestContextContext with
nativeCreateRequestContextAdapter, so we have one JNI call. That will sacrifice
readability. But the JNI slides do recommend against traversing the boundary
unnecessarily. Not sure if this counts.

[pauljensen, 2015/12/02 03:50:01]

I split it for a few reasons:
1. The ChromiumURLRequestContext and the CronetURLRequestContext both share the
Config JNI (this is why createNativeUrlRequestContextConfig() is package-private
not private), but then they create different adapters from the config.
2. the QUIC hints must be added piece by piece in between creating the Config
and the Adapter
3. for testing, I want to ensure I create the Config properly but I don't want
to have to put in mountains of mocking to actually get an Adapter, I just want
to check the Config fields are passed through properly
I'm open to alternate suggestions.

 
     private static native int nativeSetMinLogLevel(int loggingLevel);