Cronet API Refactoring

components/cronet/android/java/src/org/chromium/net/impl/UrlRequestBuilderImpl.java

Index: components/cronet/android/java/src/org/chromium/net/impl/UrlRequestBuilderImpl.java
diff --git a/components/cronet/android/java/src/org/chromium/net/impl/UrlRequestBuilderImpl.java b/components/cronet/android/java/src/org/chromium/net/impl/UrlRequestBuilderImpl.java
new file mode 100644
index 0000000000000000000000000000000000000000..d721c7ab937c4243f598e4e44632e20f763b52c6
--- /dev/null
+++ b/components/cronet/android/java/src/org/chromium/net/impl/UrlRequestBuilderImpl.java
@@ -0,0 +1,252 @@
+// 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.impl;
+
+import android.util.Pair;
+
+import org.chromium.base.Log;
+import org.chromium.net.CronetEngine;
+import org.chromium.net.ExperimentalUrlRequest;
+import org.chromium.net.UploadDataProvider;
+import org.chromium.net.UrlRequest;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.concurrent.Executor;
+
+/**
+ * Implements {@link org.chromium.net.UrlRequest.Builder} and all experimental features.
+ */
+public class UrlRequestBuilderImpl extends ExperimentalUrlRequest.Builder {
+    private static final String ACCEPT_ENCODING = "Accept-Encoding";
+    private static final String TAG = "UrlRequestBuilder";
+
+    // All fields are temporary storage of UrlRequest configuration to be
+    // copied to built UrlRequests.
+
+    // CronetEngine to execute request.
+    private final CronetEngineBase mCronetEngine;
+    // URL to request.
+    private final String mUrl;
+    // Callback to receive progress callbacks.
+    private final UrlRequest.Callback mCallback;
+    // Executor to invoke callback on.
+    private final Executor mExecutor;
+    // HTTP method (e.g. GET, POST etc).
+    private String mMethod;
+
+    // List of request headers, stored as header field name and value pairs.
+    private final ArrayList<Pair<String, String>> mRequestHeaders = new ArrayList<>();
+    // Disable the cache for just this request.
+    private boolean mDisableCache;
+    // Disable connection migration for just this request.
+    private boolean mDisableConnectionMigration;
+    // Priority of request. Default is medium.
+    @CronetEngineBase.RequestPriority
+    private int mPriority = REQUEST_PRIORITY_MEDIUM;
+    // Request reporting annotations. Avoid extra object creation if no annotations added.
+    private Collection<Object> mRequestAnnotations = Collections.emptyList();
+    // If request is an upload, this provides the request body data.
+    private UploadDataProvider mUploadDataProvider;
+    // Executor to call upload data provider back on.
+    private Executor mUploadDataProviderExecutor;
+    private boolean mAllowDirectExecutor = false;
+
+    /**
+     * Creates a builder for {@link UrlRequest} objects. All callbacks for
+     * generated {@link UrlRequest} 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.
+     * @param cronetEngine {@link CronetEngine} used to execute this request.
+     */
+    UrlRequestBuilderImpl(String url, UrlRequest.Callback callback, Executor executor,
+            CronetEngineBase cronetEngine) {
+        super();
+        if (url == null) {
+            throw new NullPointerException("URL is required.");
+        }
+        if (callback == null) {
+            throw new NullPointerException("Callback is required.");
+        }
+        if (executor == null) {
+            throw new NullPointerException("Executor is required.");
+        }
+        if (cronetEngine == null) {
+            throw new NullPointerException("CronetEngine is required.");
+        }
+        mUrl = url;
+        mCallback = callback;
+        mExecutor = executor;
+        mCronetEngine = cronetEngine;
+    }
+
+    /**
+     * Sets the HTTP method verb to use for this request.
+     *
+     * <p>The default when this method is not called is "GET" if the request has
+     * no body or "POST" if it does.
+     *
+     * @param method "GET", "HEAD", "DELETE", "POST" or "PUT".
+     * @return the builder to facilitate chaining.
+     */

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

all these comments should be removed and replaced with simply @Override

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

Done. Same for BidirectionalStreamBuilderImpl. Kept comments in
CronetEngineBuilderImpl.

+    public UrlRequest.Builder setHttpMethod(String method) {
+        if (method == null) {
+            throw new NullPointerException("Method is required.");
+        }
+        mMethod = method;
+        return this;
+    }
+
+    /**
+     * Adds a request header.
+     *
+     * @param header header name.
+     * @param value header value.
+     * @return the builder to facilitate chaining.
+     */
+    public UrlRequestBuilderImpl addHeader(String header, String value) {
+        if (header == null) {
+            throw new NullPointerException("Invalid header name.");
+        }
+        if (value == null) {
+            throw new NullPointerException("Invalid header value.");
+        }
+        if (ACCEPT_ENCODING.equalsIgnoreCase(header)) {
+            Log.w(TAG, "It's not necessary to set Accept-Encoding on requests - cronet will do"
+                            + " this automatically for you, and setting it yourself has no "
+                            + "effect. See https://crbug.com/581399 for details.",
+                    new Exception());
+            return this;
+        }
+        mRequestHeaders.add(Pair.create(header, value));
+        return this;
+    }
+
+    /**
+     * Disables cache for the request. If context is not set up to use cache,
+     * this call has no effect.
+     * @return the builder to facilitate chaining.
+     */
+    public UrlRequestBuilderImpl disableCache() {
+        mDisableCache = true;
+        return this;
+    }
+
+    /**
+     * Disables connection migration for the request if enabled for
+     * the session.
+     * @return the builder to facilitate chaining.
+     */
+    public UrlRequestBuilderImpl disableConnectionMigration() {
+        mDisableConnectionMigration = true;
+        return this;
+    }
+
+    /**
+     * Sets priority of the request which should be one of the
+     * {@link #REQUEST_PRIORITY_IDLE REQUEST_PRIORITY_*} values.
+     * The request is given {@link #REQUEST_PRIORITY_MEDIUM} priority
+     * if this method is not called.
+     *
+     * @param priority priority of the request which should be one of the
+     *         {@link #REQUEST_PRIORITY_IDLE REQUEST_PRIORITY_*} values.
+     * @return the builder to facilitate chaining.
+     */
+    public UrlRequestBuilderImpl setPriority(@CronetEngineBase.RequestPriority int priority) {
+        mPriority = priority;
+        return this;
+    }
+
+    /**
+     * Sets upload data provider. Switches method to "POST" if not
+     * explicitly set. Starting the request will throw an exception if a
+     * Content-Type header is not set.
+     *
+     * @param uploadDataProvider responsible for providing the upload data.
+     * @param executor All {@code uploadDataProvider} methods will be invoked
+     *     using this {@code Executor}. May optionally be the same
+     *     {@code Executor} the request itself is using.
+     * @return the builder to facilitate chaining.
+     */
+    public UrlRequestBuilderImpl setUploadDataProvider(
+            UploadDataProvider uploadDataProvider, Executor executor) {
+        if (uploadDataProvider == null) {
+            throw new NullPointerException("Invalid UploadDataProvider.");
+        }
+        if (executor == null) {
+            throw new NullPointerException("Invalid UploadDataProvider Executor.");
+        }
+        if (mMethod == null) {
+            mMethod = "POST";
+        }
+        mUploadDataProvider = uploadDataProvider;
+        mUploadDataProviderExecutor = executor;
+        return this;
+    }
+
+    /**
+     * Marks that the executors this request will use to notify callbacks (for
+     * {@code UploadDataProvider}s and {@code UrlRequest.Callback}s) is intentionally performing
+     * inline execution, like Guava's directExecutor or
+     * {@link java.util.concurrent.ThreadPoolExecutor.CallerRunsPolicy}.
+     *
+     * <p><b>Warning:</b> This option makes it easy to accidentally block the network thread.
+     * It should not be used if your callbacks perform disk I/O, acquire locks, or call into
+     * other code you don't carefully control and audit.
+     */
+    public UrlRequestBuilderImpl allowDirectExecutor() {
+        mAllowDirectExecutor = true;
+        return this;
+    }
+
+    /**
+     * Associates the annotation object with this request. May add more than one.
+     * Passed through to a {@link org.chromium.net.RequestFinishedInfo.Listener},
+     * see {@link org.chromium.net.RequestFinishedInfo#getAnnotations}.
+     *
+     * @param annotation an object to pass on to the
+     * {@link org.chromium.net.RequestFinishedInfo.Listener} with a
+     * {@link org.chromium.net.RequestFinishedInfo}.
+     * @return the builder to facilitate chaining.
+     */
+    public UrlRequestBuilderImpl addRequestAnnotation(Object annotation) {
+        if (annotation == null) {
+            throw new NullPointerException("Invalid metrics annotation.");
+        }
+        if (mRequestAnnotations.isEmpty()) {
+            mRequestAnnotations = new ArrayList<>();
+        }
+        mRequestAnnotations.add(annotation);
+        return this;
+    }
+
+    /**
+     * Creates a {@link UrlRequest} using configuration within this
+     * builder. The returned {@code UrlRequest} can then be started
+     * by calling {@link UrlRequest#start}.
+     *
+     * @return constructed {@link UrlRequest}.
+     */
+    public UrlRequestBase build() {
+        final UrlRequestBase request = mCronetEngine.createRequest(mUrl, mCallback, mExecutor,
+                mPriority, mRequestAnnotations, mDisableCache, mDisableConnectionMigration,
+                mAllowDirectExecutor);
+        if (mMethod != null) {
+            request.setHttpMethod(mMethod);
+        }
+        for (Pair<String, String> header : mRequestHeaders) {
+            request.addHeader(header.first, header.second);
+        }
+        if (mUploadDataProvider != null) {
+            request.setUploadDataProvider(mUploadDataProvider, mUploadDataProviderExecutor);
+        }
+        return request;
+    }
+}