Cronet: a framework for providing alternative Cronet implementations

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

Index: components/cronet/android/api/src/org/chromium/net/CronetEngine.java
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 cd7f42558d03a9e3bcae83a468e22f5c47f6e1ef..ac0f6b53c9d982659c3cc0e3d5e44806920e255b 100644
--- a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
+++ b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
@@ -6,12 +6,16 @@ package org.chromium.net;
 
 import android.content.Context;
 import android.net.http.HttpResponseCache;
+import android.support.annotation.Nullable;
+import android.util.Log;
 
 import java.io.IOException;
 import java.net.URL;
 import java.net.URLConnection;
 import java.net.URLStreamHandlerFactory;
+import java.util.ArrayList;
 import java.util.Date;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Executor;
 
@@ -22,6 +26,8 @@ import javax.net.ssl.HttpsURLConnection;
  * using {@link Builder}.
  */
 public abstract class CronetEngine {
+    static final String TAG = "CronetApi";

[pauljensen, 2017/01/11 16:48:12]

private

[pauljensen, 2017/01/11 16:48:12]

"CronetApi" -> CronetEngine.class.getSimpleName()

[kapishnikov, 2017/01/17 22:29:19]

Gone in the latest CL.

[kapishnikov, 2017/01/17 22:29:19]

Gone in the latest CL.

+
     /**
      * A builder for {@link CronetEngine}s, which allows runtime configuration of
      * {@code CronetEngine}. Configuration options are set on the builder and
@@ -63,7 +69,28 @@ public abstract class CronetEngine {
          *                the lifetime of {@code context} unnecessarily.
          */
         public Builder(Context context) {
-            mBuilderDelegate = ImplLoader.load(context);
+            CronetImplProviderFinder finder = new CronetImplProviderFinder(context);
+            List<CronetImplProvider> providers = finder.findProviders();
+
+            // Default provider selector.
+            CronetImplProviderSelector selector = new CronetImplProviderSelector() {
+                @Override
+                @Nullable
+                public CronetImplProvider select(List<CronetImplProvider> providers) {
+                    if (providers.size() > 0) {
+                        return providers.get(0);
+                    }
+                    return null;
+                }
+            };
+
+            CronetImplProvider provider = selector.select(providers);
+            if (provider != null) {
+                mBuilderDelegate = provider.load(context);
+            } else {
+                throw new RuntimeException("Unable to find suitable CronetImplProvider."
+                        + " Have you included all required jars?");
+            }
         }
 
         /**
@@ -405,4 +432,99 @@ public abstract class CronetEngine {
      */
     public abstract UrlRequest.Builder newUrlRequestBuilder(
             String url, UrlRequest.Callback callback, Executor executor);
+
+    /**
+     * Searches for the available CronetImplProvider implementations. The {@link #findProviders()}
+     * method returns the list of availiable providers in the following order (if present):
+     * <ul>
+     * <li>Class name specified as "CronetProviderClassName" application string resource.</li>
+     * <li>The default Cronet implementation</li>
+     * </ul>
+     */
+    private static class CronetImplProviderFinder {
+        /**
+         * Name of the default {@link CronetImplProvider} class.
+         */
+        private static final String DEFAULT_CRONET_IMPL_PROVIDER_CLASS =
+                "org.chromium.net.impl.DefaultCronetImplProvider";
+
+        /**
+         * 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 final Context mContext;
+
+        private CronetImplProviderFinder(Context context) {
+            mContext = context;
+        }
+
+        /**
+         * Returns the found list of available providers.
+         *
+         * @return the list of providers.
+         */
+        private List<CronetImplProvider> findProviders() {
+            List<CronetImplProvider> providers = new ArrayList<>();
+            addCronetProviderFromResourceFile(providers);
+            addCronetProviderImplByClassName(DEFAULT_CRONET_IMPL_PROVIDER_CLASS, providers);
+            return providers;

[pauljensen, 2017/01/11 16:48:13]

We need to filter out providers that implement a version older than the API. 
Unfortunately using the Chromium version (e.g. 57.0.2977.0) doesn't tell you
what version of the API they support, which could cause problems, for example:

Today we branch M60 and trunk becomes M61.  Let's call them 60.0 and 61.0.
Tomorrow we update the API version and for whatever reason merge the change into
M60.  Let's call tomorrow's versions 60.1 and 61.1.
Now we can't use the Chromium version to tell which API is newer, for example
61.0 implements an API version older than 60.1.

Perhaps we could do this in a follow-on CL where we allow users to find out if
they want to update the impl (by getting an Intent to throw back).

[kapishnikov, 2017/01/17 22:29:19]

We are going to use API_LEVEL that is introduced in this CL.

+        }
+
+        /**
+         * Adds a new provider referenced by the class name to the end of the list.
+         *
+         * @param className the class name of the provider that shoul dbe instantiated.
+         * @param providers the list of providers to add the new provider to.
+         * @return true if the provider was added.
+         */
+        private static boolean addCronetProviderImplByClassName(
+                String className, List<CronetImplProvider> providers) {
+            ClassLoader loader = CronetImplProviderFinder.class.getClassLoader();
+            Class<? extends CronetImplProvider> providerClass;

[pauljensen, 2017/01/11 16:48:12]

move inside try block (variables should always have the smallest scope
possible), unless you go with my next comment

[kapishnikov, 2017/01/17 22:29:19]

Done. See CronetProviders.java

+            try {
+                providerClass = loader.loadClass(className).asSubclass(CronetImplProvider.class);
+                providers.add(providerClass.newInstance());

[pauljensen, 2017/01/11 16:48:13]

how about moving this line outside the try block?

[kapishnikov, 2017/01/17 22:29:19]

Kept it here for readability.

+                return true;
+            } catch (Exception ex) {

[pauljensen, 2017/01/11 16:48:12]

catching all exceptions is not recommended, how about just ClassNotFound?

[kapishnikov, 2017/01/17 22:29:19]

Changed to multiple catches. We cannot catch multiple exceptions at once since
it is only available in API Level 19 but we currently support API Level 16.

+                // Class is absent in the class path. That is expected for
+                // some deployment configurations.
+                return false;
+            }
+        }
+
+        /**
+         * Adds 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 true if the provider was added.
+         */
+        private boolean addCronetProviderFromResourceFile(List<CronetImplProvider> providers) {
+            String packageName = mContext.getPackageName();
+            int resId = mContext.getResources().getIdentifier(
+                    RES_KEY_CRONET_IMPL_CLASS, "string", packageName);

[pauljensen, 2017/01/11 16:48:13]

packageName->mContext.getPackageName()

[kapishnikov, 2017/01/17 22:29:19]

Done.

+            // Resource not found
+            if (resId == 0) {
+                // The resource wasn't included in the app; therefore, there is nothing to add.
+                return false;
+            }
+            String className = mContext.getResources().getString(resId);
+            CronetImplProvider provider;

[mef, 2017/01/10 23:30:03]

Should it call addCronetProviderImplByClassName(className) at this point?

[kapishnikov, 2017/01/17 22:29:19]

Done. See CronetProviders.java

+            try {
+                Class<? extends CronetImplProvider> cl =
+                        Class.forName(className).asSubclass(CronetImplProvider.class);
+                provider = cl.newInstance();
+            } catch (Exception e) {
+                Log.e(TAG, "Unable to instantiate Cronet implementation class " + className
+                                + " that is listed as " + RES_KEY_CRONET_IMPL_CLASS
+                                + " in the app string resource file",
+                        e);
+                return false;
+            }
+
+            providers.add(provider);
+            return true;
+        }
+    }
 }