Cronet: a framework for providing alternative Cronet implementations

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

 // Copyright 2015 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.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;
 
 import javax.net.ssl.HttpsURLConnection;
 /**
  * An engine to process {@link UrlRequest}s, which uses the best HTTP stack
  * available on the current platform. An instance of this class can be created
  * 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
      * then {@link #build} is called to create the {@code CronetEngine}.
      */
     // NOTE(kapishnikov): In order to avoid breaking the existing API clients, all future methods
     // added to this class and other API classes must have default implementation.
     public static class Builder {
         /**
          * A class which provides a method for loading the cronet native library. Apps needing to
@@ skipping 21 matching lines @@
          * {@link CronetEngine}. The default configuration enables HTTP/2 and
          * disables QUIC, SDCH and the HTTP cache.
          *
          * @param context Android {@link Context}, which is used by
          *                {@link Builder} to retrieve the application
          *                context. A reference to only the application
          *                context will be kept, so as to avoid extending
          *                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?");
+            }
         }
 
         /**
          * Constructs a User-Agent string including application name and version,
          * system build version, model and id, and Cronet version.
          *
          * @return User-Agent string.
          */
         public String getDefaultUserAgent() {
             return mBuilderDelegate.getDefaultUserAgent();
@@ skipping 321 matching lines @@
      * {@code executor}'s threads. {@code executor} must not run tasks on the
      * thread calling {@link Executor#execute} to prevent blocking networking
      * operations and causing exceptions during shutdown.
      *
      * @param url URL for the generated requests.
      * @param callback callback object that gets invoked on different events.
      * @param executor {@link Executor} on which all callbacks will be invoked.
      */
     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;
+        }
+    }
 }