Coalesce small buffers in net::BidirectionalStream

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

Index: components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java
diff --git a/components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java b/components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java
index 5b6d04bfc427ba4052019ed18cb7c70cbb54004f..c9ab8b5a74c50097a53753cdb10b30f9947c976a 100644
--- a/components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java
+++ b/components/cronet/android/api/src/org/chromium/net/BidirectionalStream.java
@@ -47,6 +47,8 @@ public abstract class BidirectionalStream {
         // Priority of the stream. Default is medium.
         @StreamPriority private int mPriority = STREAM_PRIORITY_MEDIUM;
 
+        private boolean mDisableAutoFlush;
+
         /**
          * Creates a builder for {@link BidirectionalStream} objects. All callbacks for
          * generated {@code BidirectionalStream} objects will be invoked on
@@ -160,6 +162,15 @@ public abstract class BidirectionalStream {
         }
 
         /**
+         * By default, data is flushed after every {@link #write write()}. This
+         * is to disable auto flush.
+         */
+        public Builder setDisableAutoFlush() {

[kapishnikov, 2016/04/11 23:09:41]

I would change the method to accept the boolean parameter and rename it to
disableAutoFlush(Boolean disable).

Here are two reasons.
1. An app can have its own configuration that sets disableAutoFlush flag. It
will be easier for the app to call:
disableAutoFlush(config.disableAutoFlush) rather than to use IF statement.
2. It will make the method independent from the default behavior, e.g. let's say
that for some reason we decided to change the default behaviour to disable
auto-flushing. The method will become no-op and we will need a new method.

[xunjieli, 2016/04/12 18:01:48]

Done. Good idea!

+            mDisableAutoFlush = true;
+            return this;
+        }
+
+        /**
          * Creates a {@link BidirectionalStream} using configuration from this
          * {@link Builder}. The returned {@code BidirectionalStream} can then be started
          * by calling {@link BidirectionalStream#start}.
@@ -168,8 +179,8 @@ public abstract class BidirectionalStream {
          *         this {@link Builder}
          */
         public BidirectionalStream build() {
-            return mCronetEngine.createBidirectionalStream(
-                    mUrl, mCallback, mExecutor, mHttpMethod, mRequestHeaders, mPriority);
+            return mCronetEngine.createBidirectionalStream(mUrl, mCallback, mExecutor, mHttpMethod,
+                    mRequestHeaders, mPriority, mDisableAutoFlush);
         }
     }
 
@@ -178,12 +189,14 @@ public abstract class BidirectionalStream {
      */
     public abstract static class Callback {
         /**
-         * Invoked when request headers are sent. Indicates that stream has initiated the request.
-         * Consumer may call {@link BidirectionalStream#write write()} to start writing data.
+         * Invoked when the stream is ready for reading and writing.
+         * Consumer may call {@link BidirectionalStream#read read()} to start

[kapishnikov, 2016/04/11 23:09:41]

Does it mean that the consumer does not need to wait for
onResponseHeadersReceived() before calling read()?

[xunjieli, 2016/04/12 18:01:48]

Yes, that is allowed.

+         * reading. Consumer may call {@link BidirectionalStream#write write()}
+         * to start writing data.
          *
-         * @param stream the stream on which request headers were sent
+         * @param stream the stream that is ready
          */
-        public abstract void onRequestHeadersSent(BidirectionalStream stream);
+        public abstract void onStreamReady(BidirectionalStream stream);
 
         /**
          * Invoked when initial response headers are received. Headers are available from
@@ -365,6 +378,12 @@ public abstract class BidirectionalStream {
     public abstract void write(ByteBuffer buffer, boolean endOfStream);
 
     /**
+     * Flushes pending writes. This method should only be invoked when auto
+     * flush is disable through {@link Builder#setDisableAutoFlush}.

[kapishnikov, 2016/04/11 23:09:41]

disable => disabled

[xunjieli, 2016/04/12 18:01:48]

Done.

+     */
+    public abstract void flush();
+
+    /**
      * Pings remote end-point. {@code callback} methods will be invoked on {@code executor}.
      *
      * @param callback the callback that will be invoked when ping succeeds or fails