Cronet: a framework for providing alternative Cronet implementations

components/cronet/android/api/src/org/chromium/net/CronetProviders.java

Index: components/cronet/android/api/src/org/chromium/net/CronetProviders.java
diff --git a/components/cronet/android/api/src/org/chromium/net/CronetProviders.java b/components/cronet/android/api/src/org/chromium/net/CronetProviders.java
new file mode 100644
index 0000000000000000000000000000000000000000..5e49f841861f39d32d630b348775c5c358b1d856
--- /dev/null
+++ b/components/cronet/android/api/src/org/chromium/net/CronetProviders.java
@@ -0,0 +1,175 @@
+// Copyright 2017 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.support.annotation.VisibleForTesting;
+import android.util.Log;
+
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * Provides the list of available {@link CronetProvider} instances.
+ * <p/>
+ * <b>NOTE:</b> This class is for advanced users that want to select a particular
+ * Cronet implementation. Most users should simply use {@code new} {@link
+ * CronetEngine.Builder#CronetEngine.Builder(android.content.Context)}.
+ */
+public class CronetProviders {
+    private static final String TAG = CronetProvider.class.getSimpleName();
+
+    /**
+     * String returned by {@link CronetProvider#getName} for {@link CronetProvider}
+     * that provides Cronet implementation based on the system's
+     * {@link java.net.HttpURLConnection} implementation. This implementation
+     * offers significantly degraded performance relative to native Cronet
+     * implementations (see {@link #PROVIDER_NAME_NATIVE}).
+     */
+    public static final String PROVIDER_NAME_PLATFORM = "Platform-Cronet-Provider";
+
+    /**
+     * String returned by {@link CronetProvider#getName} for {@link CronetProvider}
+     * that provides native Cronet implementation. This implementation
+     * offers significantly higher performance relative to the system Cronet
+     * implementations (see {@link #PROVIDER_NAME_PLATFORM}).

[pauljensen, 2017/01/23 14:44:29]

I don't understand "system Cronet implementation", this sounds like the
Java-only one is the default Cronet implementation.  Can we use my previously
suggested rewording, that doesn't mention the Java-only one (I really don't want
people mistakenly using it; I fear they'll assume Cronet always has performance
worse than Android's HttpURLConnection impl and never use Cronet again).

+     */
+    public static final String PROVIDER_NAME_NATIVE = "Native-Cronet-Provider";
+
+    /**
+     * The key in the app string resource file that is be searched
+     * for an alternative implementation.
+     */
+    private static final String RES_KEY_CRONET_IMPL_CLASS = "CronetProviderClassName";
+
+    private static CronetProviders sInstance = new CronetProviders();
+    private List<CronetProvider> mProviders;
+    private final Object mLock = new Object();

[pauljensen, 2017/01/23 14:44:29]

swap these two lines and add @GuardedBy

+
+    @VisibleForTesting
+    CronetProviders() {}
+
+    /**
+     * Returns an instance of {@link CronetProviders}.
+     *
+     * @return an instance of {@code CronetProviders}.
+     */
+    public static CronetProviders getInstance() {
+        return sInstance;
+    }
+
+    /**
+     * Name of the Java {@link CronetProvider} class.
+     */
+    private static final String JAVA_CRONET_PROVIDER_CLASS =
+            "org.chromium.net.impl.JavaCronetProvider";
+
+    /**
+     * Name of the native {@link CronetProvider} class.
+     */
+    private static final String NATIVE_CRONET_PROVIDER_CLASS =
+            "org.chromium.net.impl.NativeCronetProvider";
+
+    /**
+     * Returns an unmodifiable list of all available {@link CronetProvider}s.
+     * The providers are returned in no particular order. Some of the returned
+     * providers may be in a disabled state and should be enabled by the invoker.
+     * See {@link CronetProvider#isEnabled()}.
+     *
+     * @return the list of available providers.
+     */
+    public List<CronetProvider> getAllProviders(Context context) {

[pauljensen, 2017/01/23 14:44:29]

I really think we should move this to CronetProvider as a static function.  I
know it makes testing harder, but it cuts the number of API classes related to
this advanced feature in half, which I think is a good thing as it doesn't draw
as much attention to it, which I think is a good thing because I think only a
very small number of consumers will need it, and I don't want it misleading
others into thinking this is the typical way of getting a CronetEngine.Builder.

+        synchronized (mLock) {
+            if (mProviders != null) {
+                return mProviders;
+            }
+            List<CronetProvider> providers = new ArrayList<>();
+            addCronetProviderFromResourceFile(context, providers);
+            addCronetProviderImplByClassName(
+                    context, NATIVE_CRONET_PROVIDER_CLASS, providers, false);
+            addCronetProviderImplByClassName(context, JAVA_CRONET_PROVIDER_CLASS, providers, false);
+            mProviders = Collections.unmodifiableList(providers);
+            return mProviders;
+        }
+    }
+
+    /**
+     * Attempts to add a new provider referenced by the class name to the end of the list.
+     *
+     * @param className the class name of the provider that should be instantiated.
+     * @param providers the list of providers to add the new provider to.
+     * @return {@code true} if the provider was added to the list; {@code false}
+     *         if the provider couldn't be instantiated.
+     */
+    private static boolean addCronetProviderImplByClassName(
+            Context context, String className, List<CronetProvider> providers, boolean logError) {
+        ClassLoader loader = context.getClassLoader();
+        try {
+            Class<? extends CronetProvider> providerClass =
+                    loader.loadClass(className).asSubclass(CronetProvider.class);
+            Constructor<? extends CronetProvider> ctor =
+                    providerClass.getConstructor(Context.class);
+            providers.add(ctor.newInstance(context));
+            return true;
+        } catch (InstantiationException e) {
+            logReflectiveOperationException(className, logError, e);
+        } catch (InvocationTargetException e) {
+            logReflectiveOperationException(className, logError, e);
+        } catch (NoSuchMethodException e) {
+            logReflectiveOperationException(className, logError, e);
+        } catch (IllegalAccessException e) {
+            logReflectiveOperationException(className, logError, e);
+        } catch (ClassNotFoundException e) {
+            logReflectiveOperationException(className, logError, e);
+        }
+        return false;
+    }
+
+    /**
+     * De-duplicates exception handling logic in {@link #addCronetProviderImplByClassName}.
+     * It should be removed when support of API Levels lower than 19 is deprecated.
+     */
+    private static void logReflectiveOperationException(
+            String className, boolean logError, Exception e) {
+        if (logError) {
+            Log.e(TAG, "Unable to load provider class: " + className, e);
+        } else {
+            Log.d(TAG, "Tried to load " + className + " provider class but it wasn't"
+                            + " included in the app classpath");
+        }
+    }
+
+    /**
+     * Attempts to add a provider specified in the app resource file to the end
+     * of the provider list.
+     *
+     * @param providers the list of providers to add the new provider to.
+     * @return {@code true} if the provider was added to the list; {@code false}
+     *         if the app resources do not include the string with
+     *         {@link #RES_KEY_CRONET_IMPL_CLASS} key.
+     * @throws RuntimeException if the provider cannot be found or instantiated.
+     */
+    private static boolean addCronetProviderFromResourceFile(
+            Context context, List<CronetProvider> providers) {
+        int resId = context.getResources().getIdentifier(
+                RES_KEY_CRONET_IMPL_CLASS, "string", context.getPackageName());
+        // Resource not found
+        if (resId == 0) {
+            // The resource wasn't included in the app; therefore, there is nothing to add.
+            return false;
+        }
+        String className = context.getResources().getString(resId);
+
+        if (!addCronetProviderImplByClassName(context, className, providers, true)) {
+            throw new RuntimeException("Unable to instantiate Cronet implementation class "
+                    + className + " that is listed as in the app string resource file under"
+                    + RES_KEY_CRONET_IMPL_CLASS + " key");
+        }
+        return true;
+    }
+}