[Cronet] Clear mock cert verifier reference in CronetEngine.Builder

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

 // Copyright 2014 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 android.content.Context;
 import android.os.ConditionVariable;
 import android.os.Environment;
 
@@ skipping 16 matching lines @@
  * Framework for testing Cronet.
  */
 @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
 public class CronetTestFramework {
     private static final String TAG = "CronetTestFramework";
 
     public static final String COMMAND_LINE_ARGS_KEY = "commandLineArgs";
     public static final String POST_DATA_KEY = "postData";
     public static final String CACHE_KEY = "cache";
     public static final String SDCH_KEY = "sdch";
-
     public static final String LIBRARY_INIT_KEY = "libraryInit";
-    /**
-      * Skips library initialization.
-      */
-    public static final String LIBRARY_INIT_SKIP = "skip";
 
     // Uses disk cache.
     public static final String CACHE_DISK = "disk";
 
     // Uses disk cache but does not store http data.
     public static final String CACHE_DISK_NO_HTTP = "diskNoHttp";
 
     // Uses in-memory cache.
     public static final String CACHE_IN_MEMORY = "memory";
 
     // Enables Sdch.
     public static final String SDCH_ENABLE = "enable";
 
     /**
-      * Initializes Cronet Async API only.
-      */
-    public static final String LIBRARY_INIT_CRONET_ONLY = "cronetOnly";
+     * Library init type strings to use along with {@link LIBRARY_INIT_KEY}.
+     * If unspecified, {@link LibraryInitType.CRONET} will be used.
+     */
+    public static final class LibraryInitType {
+        // Initializes Cronet Async API.
+        public static final String CRONET = "cronet";
+        // Initializes Cronet legacy API.
+        public static final String LEGACY = "legacy";
+        // Initializes Cronet HttpURLConnection API.
+        public static final String HTTP_URL_CONNECTION = "http_url_connection";
+        // Do not initialize.
+        public static final String NONE = "none";
 
-    /**
-      * Initializes Cronet HttpURLConnection Wrapper API.
-      */
-    public static final String LIBRARY_INIT_WRAPPER = "wrapperOnly";
+        private LibraryInitType() {}
+    }
 
     public URLStreamHandlerFactory mStreamHandlerFactory;
     public CronetEngine mCronetEngine;
     @SuppressWarnings("deprecation") HttpUrlRequestFactory mRequestFactory;
 
     private final String[] mCommandLine;
     private final Context mContext;
 
     private String mUrl;
     private int mHttpStatusCode = 0;
@@ skipping 18 matching lines @@
         }
 
         public void blockForComplete() {
             mComplete.block();
         }
     }
 
     // TODO(crbug.com/547160): Fix this findbugs error and remove the suppression.
     @SuppressFBWarnings("EI_EXPOSE_REP2")
     public CronetTestFramework(
             String appUrl, String[] commandLine, Context context, CronetEngine.Builder builder) {

[mef, 2015/11/24 20:38:33]

I think the whole idea of commandLine is now outdated and kludgy. 

It was used to pass configuration from instrumentation tests apk to the test
app, but now I think it should be replaced by a builder with explicit setters.

[xunjieli, 2015/11/24 21:28:06]

Yea. Paul considered doing that too, but it was not very straightforward. Let's
do that in a separate CL.

[mef, 2015/11/24 21:43:42]

Acknowledged.

         mCommandLine = commandLine;
         mContext = context;
         prepareTestStorage();
 
         // Print out extra arguments passed in starting this activity.
         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 CronetEngine.Builder from commandLine args.
         mCronetEngineBuilder = initializeCronetEngineBuilderWithPresuppliedBuilder(builder);
 
         String initString = getCommandLineArg(LIBRARY_INIT_KEY);
-        if (LIBRARY_INIT_SKIP.equals(initString)) {
-            return;
+
+        if (initString == null) {
+            initString = LibraryInitType.CRONET;
         }
 
-        mCronetEngine = initCronetEngine();
-
-        if (LIBRARY_INIT_WRAPPER.equals(initString)) {
-            mStreamHandlerFactory = mCronetEngine.createURLStreamHandlerFactory();
-        }
-
-        // Start collecting metrics.
-        mCronetEngine.getGlobalMetricsDeltas();
-
-        if (LIBRARY_INIT_CRONET_ONLY.equals(initString)) {
-            return;
-        }
-
-        mRequestFactory = initRequestFactory();
-        if (appUrl != null) {
-            startWithURL(appUrl);
+        switch (initString) {
+            case LibraryInitType.NONE:
+                break;
+            case LibraryInitType.LEGACY:
+                mRequestFactory = initRequestFactory();
+                if (appUrl != null) {
+                    startWithURL(appUrl);
+                }
+                break;
+            case LibraryInitType.HTTP_URL_CONNECTION:

[mef, 2015/11/24 20:38:33]

can this be combined with default?

[xunjieli, 2015/11/24 21:28:06]

I am not sure how to combine it with the default. The mStreamHandlerFactory
happens after the mCronetEngine line. Java cannot switch on two cases. I can
change this block to if-else, but that might reduce readability.

[mef, 2015/11/24 21:43:42]

Would it hurt anything if we always createURLStreamHandlerFactory? 
We don't have to set it as default handler, right?
Before we had distinct cases because CronetHttpURLStreamHandler had its own
UrlRequestContext, but now it just takes CronetEngine.

[xunjieli, 2015/11/24 21:54:13]

Yes, you are right. But I'd like to think that the three LibraryInitTypes are
disjoint, and each has their own paths. This makes things easier to reason
about. When we only init Cronet, we do not care about mStreamHandlerFactory, so
it makes sense that it is null. It surely does not hurt if we always
createURLStreamHandlerFactory now, but I am a little apprehensive given how
often we ran into cases like this where initializing one API influences another
because of a new feature added. (In the future we might do something crazy in
createURLStreamHandlerFactory that might inadvertently influence the async API)

+                mCronetEngine = initCronetEngine();
+                mStreamHandlerFactory = mCronetEngine.createURLStreamHandlerFactory();
+                break;
+            default:
+                mCronetEngine = initCronetEngine();
+                // Start collecting metrics.
+                mCronetEngine.getGlobalMetricsDeltas();
+                break;
         }
     }
 
     /**
      * Prepares the path for the test storage (http cache, QUIC server info).
      */
     private void prepareTestStorage() {
         File storage = new File(getTestStorageDirectory(mContext));
         if (storage.exists()) {
             assertTrue(recursiveDelete(storage));
@@ skipping 153 matching lines @@
 
     public void stopNetLog() {
         if (mRequestFactory != null) {
             mRequestFactory.stopNetLog();
         }
         if (mCronetEngine != null) {
             mCronetEngine.stopNetLog();
         }
     }
 }