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..e4fdfe2a1910ff453a20e6509e3015189693b79c 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,17 @@ public abstract class BidirectionalStream {
         }
 
         /**
+         * By default, data is flushed after every {@link #write write()}. This

[kapishnikov, 2016/04/19 03:22:22]

The first sentence of the method javadoc is used in the "Method Summary" table
to provide the quick summary of all available methods. We should probably start
it with something like "Disables or enables auto flush. By default, data is
flushed after every {@link #write write()}. If the auto flush is disabled, the
client should explicitly call {@link #flush flush()} to flush the data."

[xunjieli, 2016/04/19 19:30:03]

Done. 

+         * is to disable auto flush.
+         * @param disableAutoFlush if true, auto flush will be disabled.
+         * @return the builder to facilitate chaining.
+         */
+        public Builder disableAutoFlush(boolean disableAutoFlush) {
+            mDisableAutoFlush = disableAutoFlush;
+            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 +181,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,17 +191,19 @@ 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
+         * 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

[kapishnikov, 2016/04/19 03:22:22]

"."

[xunjieli, 2016/04/19 19:30:03]

Done.

          */
-        public abstract void onRequestHeadersSent(BidirectionalStream stream);
+        public abstract void onStreamReady(BidirectionalStream stream);
 
         /**
          * Invoked when initial response headers are received. Headers are available from
          * {@code info.}{@link UrlResponseInfo#getAllHeaders getAllHeaders()}.
-         * Consumer must call {@link BidirectionalStream#read read()} to start reading.
+         * Consumer may call {@link BidirectionalStream#read read()} to start reading.
          * Consumer may call {@link BidirectionalStream#write write()} to start writing or close the
          * stream.
          *
@@ -315,6 +330,7 @@ public abstract class BidirectionalStream {
     /**
      * Reads data from the stream into the provided buffer.
      * Must only be called at most once in response to each invocation of the

[kapishnikov, 2016/04/19 03:22:22]

The doc says that the method must only be called at most once in response to
each invocation of onStreamReady() and onResponseHeadersReceived(). Is it
allowed to call read() twice (i.e. once for each method) in response to these
methods? If not, should we rephrase it "... at most once in response to either
onStreamReady() or onResponseHeadersReceived()...". Or maybe, we shouldn't
mention onResponseHeadersReceived() at all?

[xunjieli, 2016/04/19 19:30:03]

Done. I agree it can be a bit confusing. I think it is the same case for
write(), since there can only be one write() in flight (if auto flush is on). 
I adjusted the documentation to say that read() can only be called in
onStreamReady() or onResponseHeadersReceived(). Not sure if it's clearer. PTAL.

+     * {@link Callback#onStreamReady onStreamReady()},
      * {@link Callback#onResponseHeadersReceived onResponseHeadersReceived()} and {@link
      * Callback#onReadCompleted onReadCompleted()} methods of the {@link
      * Callback}. Each call will result in an invocation of one of the
@@ -340,7 +356,7 @@ public abstract class BidirectionalStream {
     /**
      * Attempts to write data from the provided buffer into the stream.
      * Must only be called at most once in response to each invocation of the
-     * {@link Callback#onRequestHeadersSent onRequestHeadersSent()} or {@link
+     * {@link Callback#onStreamReady onStreamReady()} or {@link
      * Callback#onWriteCompleted onWriteCompleted()} methods of the {@link Callback}.
      * Each call will result in an asynchronous call to one of the {@link Callback Callback}'s
      * {@link Callback#onWriteCompleted onWriteCompleted()} method if data
@@ -365,6 +381,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 disabled through {@link Builder#disableAutoFlush}.
+     */
+    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