Cronet: a framework for providing alternative Cronet implementations

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

+// 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;
+
+/**
+ *

[pauljensen, 2017/01/20 17:13:35]

empty line

[kapishnikov, 2017/01/20 21:48:38]

Done.

+ * A Cronet implementation provider. Note: every implementation of CronetProvider

[pauljensen, 2017/01/20 17:13:35]

I don't understand the "Note"...there is only one constructor, remove?

[pauljensen, 2017/01/20 17:13:35]

I think we need to add more description here.  Like:
A Cronet implementation provider.  This class can be used to select specific
Cronet implementations to create {@link CronetEngine.Builder} instances.
To get the list of available {@link CronetProvider}s call {@link
CronetProviders#getAvailableProviders}

[kapishnikov, 2017/01/20 21:48:38]

A subclass can have a different constructor that accepts a second parameter in
addition to Context. That is not allowed since the provider instantiation logic
relies on the fact that the subclass is always going to have a constructor that
accepts single Context parameter.

Added comments.

+ * must have a public constructor that accepts a single {@link Context} parameter.
+ * <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)}.

[pauljensen, 2017/01/20 17:13:35]

Might want to put this NOTE at the top.

[kapishnikov, 2017/01/20 21:48:38]

Javadoc uses the first sentence to create the short description of the class in
the package content table. That is why I added it after the description of the
class.

+ */
+public abstract class CronetProvider {
+    protected final Context mContext;
+
+    protected CronetProvider(Context context) {
+        mContext = context;
+    }
+
+    /**
+     * Creates and returns {@link CronetEngine.Builder}.

[pauljensen, 2017/01/20 17:13:35]

returns->returns an instance of

[kapishnikov, 2017/01/20 21:48:38]

Done.

+     *
+     * @return {@code CronetEngine.Builder}.
+     */
+    public abstract CronetEngine.Builder createBuilder();
+
+    /**
+     * Returns the provider name.

[pauljensen, 2017/01/20 17:13:35]

Include @links to statically defined names.

[kapishnikov, 2017/01/20 21:48:38]

Done.

+     *
+     * @return provider name.
+     */
+    public abstract String getName();
+
+    /**
+     * Returns the provider version.

[pauljensen, 2017/01/20 17:13:35]

Mention that this can be used to select newest (i.e. highest performing, most
secure) provider

[kapishnikov, 2017/01/20 21:48:38]

Done.

+     *
+     * @return provider version.
+     */
+    public abstract String getVersion();
+
+    /**
+     * Returns whether the provider is enabled and can be used to instantiate the Cronet engine.

[pauljensen, 2017/01/20 17:13:35]

Mention that if isEnabled() returns false there are other APIs available to
enable the engine.  They should refer to the documentation for the particular
provider.

[kapishnikov, 2017/01/20 21:48:38]

Done.

+     *
+     * @return true if the provider is enabled.
+     */
+    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();
+    }
+}