Cronet API Refactoring

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

Index: components/cronet/android/api/src/org/chromium/net/ExperimentalCronetEngine.java
diff --git a/components/cronet/android/api/src/org/chromium/net/ExperimentalCronetEngine.java b/components/cronet/android/api/src/org/chromium/net/ExperimentalCronetEngine.java
new file mode 100644
index 0000000000000000000000000000000000000000..1f32d21bcef780da8ad5c6621eb5153bf8178e83
--- /dev/null
+++ b/components/cronet/android/api/src/org/chromium/net/ExperimentalCronetEngine.java
@@ -0,0 +1,209 @@
+// Copyright 2016 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 java.io.IOException;
+import java.net.Proxy;
+import java.net.URL;
+import java.net.URLConnection;
+import java.util.concurrent.Executor;
+
+/**
+ * {@link CronetEngine} that exposes experimental features. Use {@link Builder} to build an
+ * instance of this class.
+ * {@hide} since this class exposes experimental features that should be hidden.
+ */
+public abstract class ExperimentalCronetEngine extends CronetEngine {

[pauljensen, 2016/09/20 19:01:37]

can this be an interface?

[kapishnikov, 2016/09/22 21:32:03]

See the previous comment.

+    /**
+     * Builder for building {@link ExperimentalCronetEngine}.
+     * {@hide} since this class exposes experimental features that should be hidden.

[pauljensen, 2016/09/20 19:01:37]

Is the inner @hide necessary?  I wouldn't have thought so

[kapishnikov, 2016/09/22 21:32:03]

Removed.

+     */
+    public static class Builder extends CronetEngine.Builder {
+        /**
+         * Default config enables SPDY, disables QUIC, SDCH and HTTP cache.
+         *
+         * @param context Android {@link Context} for engine to use.
+         */
+        public Builder(Context context) {
+            super(context);
+        }
+
+        /**
+         * Enables the network quality estimator, which collects and reports
+         * measurements of round trip time (RTT) and downstream throughput at
+         * various layers of the network stack. After enabling the estimator,
+         * listeners of RTT and throughput can be added with
+         * {@link #addRttListener} and {@link #addThroughputListener} and
+         * removed with {@link #removeRttListener} and
+         * {@link #removeThroughputListener}. The estimator uses memory and CPU
+         * only when enabled.
+         * @param value {@code true} to enable network quality estimator,
+         *            {@code false} to disable.
+         * @return the builder to facilitate chaining.
+         */
+        public Builder enableNetworkQualityEstimator(boolean value) {
+            mBuilderDelegate.enableNetworkQualityEstimator(value);
+            return this;
+        }
+
+        /**
+         * Initializes CachingCertVerifier's cache with certVerifierData which has
+         * the results of certificate verification.
+         * @param certVerifierData a serialized representation of certificate
+         *        verification results.
+         * @return the builder to facilitate chaining.
+         */
+        public Builder setCertVerifierData(String certVerifierData) {
+            mBuilderDelegate.setCertVerifierData(certVerifierData);
+            return this;
+        }
+
+        public Builder setDataReductionProxyOptions(
+                String primaryProxy, String fallbackProxy, String secureProxyCheckUrl) {
+            mBuilderDelegate.setDataReductionProxyOptions(
+                    primaryProxy, fallbackProxy, secureProxyCheckUrl);
+            return this;
+        }
+
+        /**
+         * Build an instance of {@link ExperimentalCronetEngine}
+         *
+         * @return the constructed experimental engine.
+         */
+        public ExperimentalCronetEngine build() {
+            return (ExperimentalCronetEngine) mBuilderDelegate.build();
+        }
+    }
+
+    public abstract BidirectionalStream.Builder newBidirectionalStreamBuilder(

[pauljensen, 2016/09/20 19:01:37]

this should either:
1. be moved to CronetEngine, or
2. BiDirectionalStream should be @hide'd

[pauljensen, 2016/09/20 19:01:37]

this needs a comment

[kapishnikov, 2016/09/22 21:32:03]

Made BidirectionalStream hidden since it is considered to be experimental for
now.

[kapishnikov, 2016/09/22 21:32:03]

Done.

+            String url, BidirectionalStream.Callback callback, Executor executor);
+
+    /**
+     * Starts NetLog logging to a specified directory with a bounded size. The NetLog will contain
+     * events emitted by all live CronetEngines. The NetLog is useful for debugging.
+     * The log can be viewed by stitching the files using net/log/stitch_net_log_files.py and
+     * using a Chrome browser navigated to chrome://net-internals/#import
+     * @param dirPath the directory where the log files will be created. It must already exist.
+     *            NetLog files must not already exist in the directory. If actively logging,
+     *            this method is ignored.
+     * @param logAll {@code true} to include basic events, user cookies,
+     *            credentials and all transferred bytes in the log. This option presents a
+     *            privacy risk, since it exposes the user's credentials, and should only be
+     *            used with the user's consent and in situations where the log won't be public.
+     *            {@code false} to just include basic events.
+     * @param maxSize the maximum total disk space in bytes that should be used by NetLog. Actual
+     *            disk space usage may exceed this limit slightly.
+     */
+    public abstract void startNetLogToDisk(String dirPath, boolean logAll, int maxSize);
+
+    /**
+     * Returns the effective connection type computed by the network quality
+     * estimator.
+     */
+    public abstract int getEffectiveConnectionType();
+
+    /**
+     * Configures the network quality estimator for testing. This must be called
+     * before round trip time and throughput listeners are added, and after the
+     * network quality estimator has been enabled.
+     * @param useLocalHostRequests include requests to localhost in estimates.
+     * @param useSmallerResponses include small responses in throughput
+     * estimates.
+     */
+    public abstract void configureNetworkQualityEstimatorForTesting(
+            boolean useLocalHostRequests, boolean useSmallerResponses);
+
+    /**
+     * Registers a listener that gets called whenever the network quality
+     * estimator witnesses a sample round trip time. This must be called
+     * after {@link Builder#enableNetworkQualityEstimator}, and with throw an
+     * exception otherwise. Round trip times may be recorded at various layers
+     * of the network stack, including TCP, QUIC, and at the URL request layer.
+     * The listener is called on the {@link java.util.concurrent.Executor} that
+     * is passed to {@link Builder#enableNetworkQualityEstimator}.
+     * @param listener the listener of round trip times.
+     */
+    public abstract void addRttListener(NetworkQualityRttListener listener);
+
+    /**
+     * Removes a listener of round trip times if previously registered with
+     * {@link #addRttListener}. This should be called after a
+     * {@link NetworkQualityRttListener} is added in order to stop receiving
+     * observations.
+     * @param listener the listener of round trip times.
+     */
+    public abstract void removeRttListener(NetworkQualityRttListener listener);
+
+    /**
+     * Registers a listener that gets called whenever the network quality
+     * estimator witnesses a sample throughput measurement. This must be called
+     * after {@link Builder#enableNetworkQualityEstimator}. Throughput observations
+     * are computed by measuring bytes read over the active network interface
+     * at times when at least one URL response is being received. The listener
+     * is called on the {@link java.util.concurrent.Executor} that is passed to
+     * {@link Builder#enableNetworkQualityEstimator}.
+     * @param listener the listener of throughput.
+     */
+    public abstract void addThroughputListener(NetworkQualityThroughputListener listener);
+
+    /**
+     * Removes a listener of throughput. This should be called after a
+     * {@link NetworkQualityThroughputListener} is added with
+     * {@link #addThroughputListener} in order to stop receiving observations.
+     * @param listener the listener of throughput.
+     */
+    public abstract void removeThroughputListener(NetworkQualityThroughputListener listener);
+
+    /**
+     * Establishes a new connection to the resource specified by the {@link URL} {@code url}
+     * using the given proxy.
+     * <p>
+     * <b>Note:</b> Cronet's {@link java.net.HttpURLConnection} implementation is subject to certain
+     * limitations, see {@link #createURLStreamHandlerFactory} for details.
+     *
+     * @param url URL of resource to connect to.
+     * @param proxy proxy to use when establishing connection.
+     * @return an {@link java.net.HttpURLConnection} instance implemented by this CronetEngine.
+     * @throws IOException if an error occurs while opening the connection.
+     * TODO(pauljensen): Expose once implemented, http://crbug.com/418111

[pauljensen, 2016/09/20 19:01:37]

Move this comment to just below the "*/" and use a "//" comment.  This way if it
gets unhidden, we don't end up with a TODO in the javadocs.

[kapishnikov, 2016/09/22 21:32:03]

Good point. Done.

+     */
+    public abstract URLConnection openConnection(URL url, Proxy proxy) throws IOException;
+
+    /**
+     * Registers a listener that gets called after the end of each request with the request info.
+     *
+     * <p>This must be called after {@link Builder#enableNetworkQualityEstimator} and will throw an
+     * exception otherwise.
+     *
+     * <p>The listener is called on the {@link java.util.concurrent.Executor} that
+     * is passed to {@link Builder#enableNetworkQualityEstimator}.
+     *
+     * @param listener the listener for finished requests.
+     */
+
+    public abstract void addRequestFinishedListener(RequestFinishedInfo.Listener listener);
+
+    /**
+     * Removes a finished request listener.
+     *
+     * @param listener the listener to remove.
+     */
+    public abstract void removeRequestFinishedListener(RequestFinishedInfo.Listener listener);
+
+    /**
+     * Creates a builder for {@link ExperimentalUrlRequest} objects. All callbacks for
+     * generated {@code ExperimentalUrlRequest} objects will be invoked on
+     * {@code executor}'s thread. {@code executor} must not run tasks on the
+     * current thread to prevent blocking networking operations and causing
+     * exceptions during shutdown.
+     *
+     * @param url {@link java.net.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(

[pauljensen, 2016/09/20 19:01:37]

I think this should be removed, it's redundant with the one in the parent class

[kapishnikov, 2016/09/22 21:32:03]

Done.

+            String url, UrlRequest.Callback callback, Executor executor);
+}