Cronet API Refactoring

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

 // Copyright 2015 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.annotation.SuppressLint;
-import android.support.annotation.IntDef;
 
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
 import java.nio.ByteBuffer;
-import java.util.AbstractMap;
-import java.util.ArrayList;
-import java.util.Map;
 import java.util.concurrent.Executor;
 
 /**
  * Class for bidirectional sending and receiving of data over HTTP/2 or QUIC connections.
  * Created using {@link BidirectionalStream.Builder}.

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

This needs to be updated.

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

Done. Also added {@hide} since this is an experimental class.

  *
  * Note: There are ordering restrictions on methods of {@link BidirectionalStream};
  * please see individual methods for description of restrictions.
  */
 public abstract class BidirectionalStream {

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

can this now be an interface?

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

Making it an interface will prohibit us from adding new methods in the future
without potentially breaking the client code. E.g., a client may wrap
BidirectionalStream methods in order to add some logging, like this:

class VerboseBidirectionalStream implements BidirectionalStream {
  void start() {
    Log("Start");
    delegate.start();
  }
  ...
}

If we add a new method to BidirectionalStream, VerboseBidirectionalStream won't
compile.

     /**
      * Builder for {@link BidirectionalStream}s. Allows configuring stream before constructing
      * it via {@link Builder#build}.
      */
-    public static class Builder {
-        // All fields are temporary storage of BidirectionalStream configuration to be
-        // copied to BidirectionalStream.
-
-        // CronetEngine to create the stream.
-        private final CronetEngine mCronetEngine;
-        // URL to request.
-        private final String mUrl;
-        // Callback to receive progress callbacks.
-        private final Callback mCallback;
-        // Executor on which callbacks will be invoked.
-        private final Executor mExecutor;
-        // List of request headers, stored as header field name and value pairs.
-        private final ArrayList<Map.Entry<String, String>> mRequestHeaders =
-                new ArrayList<Map.Entry<String, String>>();
-
-        // HTTP method for the request. Default to POST.
-        private String mHttpMethod = "POST";
-        // Priority of the stream. Default is medium.
-        @StreamPriority private int mPriority = STREAM_PRIORITY_MEDIUM;
-
-        private boolean mDelayRequestHeadersUntilFirstFlush;
-
-        /**
-         * Creates a builder for {@link BidirectionalStream} objects. All callbacks for
-         * generated {@code BidirectionalStream} objects will be invoked on
-         * {@code executor}. {@code executor} must not run tasks on the
-         * current thread, otherwise the networking operations may block and exceptions
-         * may be thrown at shutdown time.
-         *
-         * @param url the URL for the generated stream
-         * @param callback the {@link Callback} object that gets invoked upon different events
-         *     occuring
-         * @param executor the {@link Executor} on which {@code callback} methods will be invoked
-         * @param cronetEngine the {@link CronetEngine} used to create the stream
-         */
-        public Builder(
-                String url, Callback callback, Executor executor, CronetEngine cronetEngine) {
-            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;
-        }
-
+    public abstract static class Builder {

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

ditto

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

Same reason.

         /**
          * Sets the HTTP method for the request. Returns builder to facilitate chaining.
          *
          * @param method the method to use for request. Default is 'POST'
          * @return the builder to facilitate chaining
          */
-        public Builder setHttpMethod(String method) {
-            if (method == null) {
-                throw new NullPointerException("Method is required.");
-            }
-            mHttpMethod = method;
-            return this;
-        }
+        public abstract Builder setHttpMethod(String method);
 
         /**
          * Adds a request header. Returns builder to facilitate chaining.
          *
          * @param header the header name
          * @param value the header value
          * @return the builder to facilitate chaining
          */
-        public Builder addHeader(String header, String value) {
-            if (header == null) {
-                throw new NullPointerException("Invalid header name.");
-            }
-            if (value == null) {
-                throw new NullPointerException("Invalid header value.");
-            }
-            mRequestHeaders.add(
-                    new AbstractMap.SimpleImmutableEntry<String, String>(header, value));
-            return this;
-        }
-
-        /** @hide */
-        @IntDef({
-                STREAM_PRIORITY_IDLE, STREAM_PRIORITY_LOWEST, STREAM_PRIORITY_LOW,
-                STREAM_PRIORITY_MEDIUM, STREAM_PRIORITY_HIGHEST,
-        })
-        @Retention(RetentionPolicy.SOURCE)
-        public @interface StreamPriority {}
+        public abstract Builder addHeader(String header, String value);
 
         /**
          * Lowest stream priority. Passed to {@link #setPriority}.
          */
         public static final int STREAM_PRIORITY_IDLE = 0;
         /**
          * Very low stream priority. Passed to {@link #setPriority}.
          */
         public static final int STREAM_PRIORITY_LOWEST = 1;
         /**
@@ skipping 13 matching lines @@
         /**
          * Sets priority of the stream which should be one of the
          * {@link #STREAM_PRIORITY_IDLE STREAM_PRIORITY_*} values.
          * The stream is given {@link #STREAM_PRIORITY_MEDIUM} priority if {@link
          * #setPriority} is not called.
          *
          * @param priority priority of the stream which should be one of the
          *         {@link #STREAM_PRIORITY_IDLE STREAM_PRIORITY_*} values.
          * @return the builder to facilitate chaining.
          */
-        public Builder setPriority(@StreamPriority int priority) {
-            mPriority = priority;
-            return this;
-        }
+        public abstract Builder setPriority(int priority);
 
         /**
          * Delays sending request headers until {@link BidirectionalStream#flush()}
          * is called. This flag is currently only respected when QUIC is negotiated.
          * When true, QUIC will send request header frame along with data frame(s)
          * as a single packet when possible.
          *
          * @param delayRequestHeadersUntilFirstFlush if true, sending request headers will
          *         be delayed until flush() is called.
          * @return the builder to facilitate chaining.
          */
-        public Builder delayRequestHeadersUntilFirstFlush(
-                boolean delayRequestHeadersUntilFirstFlush) {
-            mDelayRequestHeadersUntilFirstFlush = delayRequestHeadersUntilFirstFlush;
-            return this;
-        }
+        public abstract Builder delayRequestHeadersUntilFirstFlush(
+                boolean delayRequestHeadersUntilFirstFlush);
 
         /**
          * Creates a {@link BidirectionalStream} using configuration from this
          * {@link Builder}. The returned {@code BidirectionalStream} can then be started
          * by calling {@link BidirectionalStream#start}.
          *
          * @return constructed {@link BidirectionalStream} using configuration from
          *         this {@link Builder}
          */
         @SuppressLint("WrongConstant") // TODO(jbudorick): Remove this after rolling to the N SDK.
-        public BidirectionalStream build() {
-            return mCronetEngine.createBidirectionalStream(mUrl, mCallback, mExecutor, mHttpMethod,
-                    mRequestHeaders, mPriority, mDelayRequestHeadersUntilFirstFlush);
-        }
+        public abstract BidirectionalStream build();
     }
 
     /**
      * Callback class used to receive callbacks from a {@link BidirectionalStream}.
      */
     public abstract static class Callback {
         /**
          * Invoked when the stream is ready for reading and writing.
          * Consumer may call {@link BidirectionalStream#read read()} to start reading data.
          * Consumer may call {@link BidirectionalStream#write write()} to start writing data.
@@ skipping 182 matching lines @@
     /**
      * Returns {@code true} if the stream was successfully started and is now
      * done (succeeded, canceled, or failed).
      *
      * @return {@code true} if the stream was successfully started and is now
      *         done (completed, canceled, or failed), otherwise returns {@code false}
      *         to indicate stream is not yet started or is in progress.
      */
     public abstract boolean isDone();
 }