Cronet: a framework for providing alternative Cronet implementations

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

Index: components/cronet/android/api/src/org/chromium/net/CronetProvider.java
diff --git a/components/cronet/android/api/src/org/chromium/net/CronetProvider.java b/components/cronet/android/api/src/org/chromium/net/CronetProvider.java
new file mode 100644
index 0000000000000000000000000000000000000000..1bcf829d7c1e3b1159111bcdacf8608c20f909ad
--- /dev/null
+++ b/components/cronet/android/api/src/org/chromium/net/CronetProvider.java
@@ -0,0 +1,77 @@
+// 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;
+
+/**
+ * Provides a factory method to create {@link CronetEngine.Builder} instances.
+ * A {@code CronetEngine.Builder} instance can be used to create a specific {@link CronetEngine}
+ * implementation. To get the list of available {@link CronetProvider}s call
+ * {@link CronetProviders#getAllProviders(Context)}.
+ * <p/>
+ * Every implementation of {@code CronetProvider} <b>must</b> have a public
+ * constructor that accepts a single {@link Context} parameter.

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

I don't think embedders should generally be extending this class, so I don't
think we need this comment.  I'd vote for removing this comment as it detracts
from the very important NOTE below.  If someone extends this class they either:
1. will be calling it directly, in which case they don't care about how "new
CronetEngine.Builder" calls it, or
2. they'll figure out very quickly how it gets called from "new
CronetEngine.Builder"
If you really really want to keep this comment, can we move it to the
constructor (so it doesn't detract from the very important NOTE?

[kapishnikov, 2017/01/23 18:16:53]

Removed. Also, this class is hidden now.

+ * <p/>
+ * Since the same instance of {@code CronetProvider} can be accessed from multiple
+ * threads, all subclasses of this class should be thread-safe.

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

I don't think this is necessary.  I think APIs are generally supposed to be
thread-safe unless restrictions are explicitly stated and enforced.  I'd
recommend removing, but if you really want to keep it, I'd say move it below the
NOTE.  I don't want to detract from the fact that nearly all users shouldn't be
interacting with this class.

[kapishnikov, 2017/01/23 18:16:52]

Removed.

+ * <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 abstract class CronetProvider {
+    protected final Context mContext;
+
+    protected CronetProvider(Context context) {
+        mContext = context;
+    }
+
+    /**
+     * Creates and returns an instance of {@link CronetEngine.Builder}.

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

Perhaps we should restate the NOTE here:
This method 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)}.

[kapishnikov, 2017/01/23 18:16:52]

Done.

+     *
+     * @return {@code CronetEngine.Builder}.
+     * @throws IllegalStateException if the provider is not enabled.

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

enabled -> enabled (see {@link #isEnabled}

[kapishnikov, 2017/01/23 18:16:53]

Done.

+     */
+    public abstract CronetEngine.Builder createBuilder();
+
+    /**
+     * Returns the provider name. The well-know provider names include:
+     * <ul>
+     *     <li>{@link CronetProviders#PROVIDER_NAME_NATIVE}</li>
+     *     <li>{@link CronetProviders#PROVIDER_NAME_PLATFORM}</li>
+     * </ul>
+     *
+     * @return provider name.
+     */
+    public abstract String getName();
+
+    /**
+     * Returns the provider version. The version can be used to select the newest
+     * available provider if multiple providers are available.
+     *
+     * @return provider version.
+     */
+    public abstract String getVersion();
+
+    /**
+     * Returns whether the provider is enabled and can be used to instantiate the Cronet engine.
+     * Please read the provider documentation for the instructions how to enable it.

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

for the instructions how to enable it-> for enablement procedure.

[kapishnikov, 2017/01/23 18:16:52]

Done.

+     *
+     * @return true if the provider is enabled.

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

true -> {@code true}

+     */
+    public abstract boolean isEnabled();
+
+    @Override
+    public String toString() {
+        StringBuilder b = new StringBuilder("[");
+        b.append("class=").append(getClass().getName());
+        b.append(", ").append("name=").append(getName());
+        b.append(", ").append("version=").append(getVersion());
+        b.append(", ").append("enabled=").append(isEnabled());
+        b.append("]");
+        return b.toString();
+    }
+}