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..a9ca997b27237a7f0177b0b1f72a7bf72bdc201f 100644
--- a/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
+++ b/components/cronet/android/api/src/org/chromium/net/CronetEngine.java
@@ -12,6 +12,7 @@ import java.net.URL;
 import java.net.URLConnection;
 import java.net.URLStreamHandlerFactory;
 import java.util.Date;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Executor;
 
@@ -63,7 +64,45 @@ public abstract class CronetEngine {
          *                the lifetime of {@code context} unnecessarily.
          */
         public Builder(Context context) {
-            mBuilderDelegate = ImplLoader.load(context);
+            CronetProviders providers = CronetProviders.instance();
+
+            // Check that there is at least one available provider.
+            List<CronetProvider> providerList = providers.getAvailableProviders(context);
+            if (providerList.size() == 0) {
+                throw new RuntimeException("Unable to find any Cronet provider."
+                        + " Have you included all necessary jars?");
+            }

[pauljensen, 2017/01/18 17:03:04]

nit: how about we get rid of this if statement, the if statement on line 87
should catch this and the exception message includes the provider list?

[kapishnikov, 2017/01/19 01:25:24]

I added more error messages in a new PS ad some extra logic of filtering out
disabled providers from the list, so the final list is different from the
original one. PTAL. Some messages can be compacted into a single message but I
wanted to be more verbose.

+
+            // Try to load implementation using available providers. Assuming that
+            // providers in the beginning of the list have higher priority.
+            ICronetEngineBuilder builderDelegate = null;
+            for (CronetProvider provider : providerList) {
+                Builder builder = provider.createBuilder();
+                if (builder != null) {
+                    builderDelegate = builder.mBuilderDelegate;
+                    break;
+                }
+            }
+
+            if (builderDelegate == null) {
+                throw new RuntimeException("Unable to load any Cronet implementation."
+                        + " The list of available providers: " + providerList);

[mef, 2017/01/18 20:57:54]

CronetProvider probably needs a meaningful toString() method for this.

[kapishnikov, 2017/01/19 01:25:24]

Good point. It will give the name of the class but having toString() with more
info is better. Done.

+            }
+
+            mBuilderDelegate = builderDelegate;

[pauljensen, 2017/01/18 17:03:04]

nit: perhaps change the body of this function to
this(createBuilderDelegate(context))

[kapishnikov, 2017/01/19 01:25:24]

Good idea. Done.

+        }
+
+        /**
+         * Constructs {@link Builder} with a given delegate that provides the actual implementation
+         * of the {@code Builder} methods. This constructor is used only by the internal
+         * implementation.
+         *
+         * @param builderDelegate delegate that provides the actual implementation.
+         *
+         * {@hide}
+         */
+        public Builder(ICronetEngineBuilder builderDelegate) {
+            mBuilderDelegate = builderDelegate;
         }
 
         /**