[Cronet] Fix CronetFixedModeOutputStream to not write more bytes than specified

components/cronet/android/java/src/org/chromium/net/urlconnection/CronetFixedModeOutputStream.java

Index: components/cronet/android/java/src/org/chromium/net/urlconnection/CronetFixedModeOutputStream.java
diff --git a/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetFixedModeOutputStream.java b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetFixedModeOutputStream.java
index f6e005335ae4104e6bf1b8444324ce89999908fb..d0fe4bfd3fe699cdfd17ace1b67a081b7eee39a5 100644
--- a/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetFixedModeOutputStream.java
+++ b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetFixedModeOutputStream.java
@@ -30,6 +30,17 @@ final class CronetFixedModeOutputStream extends CronetOutputStream {
     private final CronetHttpURLConnection mConnection;
     private final MessageLoop mMessageLoop;
     private final long mContentLength;
+    // Internal buffer for storing the bytes written by consumer and for providing

[pauljensen, 2016/06/10 17:45:44]

"written" has several possible meanings here (e.g. writing to the server,
writing to mBuffer, writing to the UploadDataSink), as does consumer (e.g.
Cronet API consumer,  consumer in producer consumer model), how about "Internal
buffer used to hold the bytes from the client"?

[xunjieli, 2016/06/10 18:48:12]

Done.

+    // bytes for the native stack to consume in UploadDataProvider.read().

[pauljensen, 2016/06/10 17:45:44]

the consuming isn't really done in UploadDataProvider.read(), how about "...from
the client until they are copied to the UploadDataSink in
UploadDataProvider.read()"

[xunjieli, 2016/06/10 18:48:12]

Done.

+    // CronetFixedModeOutputStream allows consumer to write up to sDefaultBufferLength,

[pauljensen, 2016/06/10 17:45:44]

consumer->the client

[pauljensen, 2016/06/10 17:45:45]

sDefaultBufferLength->sDefaultBufferLength bytes

[pauljensen, 2016/06/10 17:45:45]

write->provide

[xunjieli, 2016/06/10 18:48:12]

Done.

[xunjieli, 2016/06/10 18:48:12]

Done.

[xunjieli, 2016/06/10 18:48:13]

Done.

+    // and wait for UploadDataProvider.read() to be called so mBuffer is cleared

[pauljensen, 2016/06/10 17:45:44]

so->, after which point

[xunjieli, 2016/06/10 18:48:11]

Done.

+    // for next round of writing. During writing, the buffer's position points

[pauljensen, 2016/06/10 17:45:44]

for next round of writing->so the client can fill it again.

[pauljensen, 2016/06/10 17:45:44]

During writing->While the client is filling the buffer (via {@code write()})

[xunjieli, 2016/06/10 18:48:11]

Done.

[xunjieli, 2016/06/10 18:48:12]

Done.

+    // to the next byte to be written, and limit is the index at which no byte

[pauljensen, 2016/06/10 17:45:44]

"is the index..."->"points to the end of the buffer"

[pauljensen, 2016/06/10 17:45:44]

written->provided by the client

[xunjieli, 2016/06/10 18:48:12]

Done.

+    // should be written. The buffer is flipped before it is passed to the
+    // UploadDataProvider for consuming. Once it is flipped, buffer position
+    // points to the next byte to be read, and limit is the index at which no

[pauljensen, 2016/06/10 17:45:44]

read->copied to the UploadDataSink

[pauljensen, 2016/06/10 17:45:44]

"is the index..."->"points to the end of the data available to be copied to the
UploadDataSink"

[xunjieli, 2016/06/10 18:48:11]

Done.

[xunjieli, 2016/06/10 18:48:12]

Done.

+    // byte should be read. When the UploadDataProvider is done reading from

[pauljensen, 2016/06/10 17:45:44]

"is done..."->"has provided all remaining bytes from the buffer to the
UploadDataSink, it clears the buffer so the client can fill it again"

[xunjieli, 2016/06/10 18:48:11]

Done.

+    // mBuffer, it clears mBuffer for next round of writing.
     private final ByteBuffer mBuffer;
     private final UploadDataProvider mUploadDataProvider = new UploadDataProviderImpl();
     private long mBytesWritten;
@@ -60,17 +71,10 @@ final class CronetFixedModeOutputStream extends CronetOutputStream {
     @Override
     public void write(int oneByte) throws IOException {
         checkNotExceedContentLength(1);
-        while (mBuffer.position() == mBuffer.limit()) {
-            // Wait until buffer is consumed.
-            mMessageLoop.loop();
-        }
+        ensureBufferHasRemaining();
         mBuffer.put((byte) oneByte);
         mBytesWritten++;
-        if (mBytesWritten == mContentLength) {
-            // Entire post data has been received. Now wait for network stack to
-            // read it.
-            mMessageLoop.loop();
-        }
+        uploadIfComplete();
     }
 
     @Override
@@ -79,28 +83,49 @@ final class CronetFixedModeOutputStream extends CronetOutputStream {
             throw new IndexOutOfBoundsException();
         }
         checkNotExceedContentLength(count);
-        if (count == 0) {
-            return;
-        }
         int toSend = count;
         while (toSend > 0) {
-            if (mBuffer.position() == mBuffer.limit()) {
-                // Wait until buffer is consumed.
-                mMessageLoop.loop();
-            }
-            int sent = Math.min(toSend, mBuffer.limit() - mBuffer.position());
+            ensureBufferHasRemaining();
+            int sent = Math.min(toSend, mBuffer.remaining());
             mBuffer.put(buffer, offset + count - toSend, sent);
             toSend -= sent;
         }
         mBytesWritten += count;
+        uploadIfComplete();
+    }
+
+    /**
+     * If {@code mBuffer} is full, wait until it is consumed and there is
+     * space to write more data to it.
+     */
+    private void ensureBufferHasRemaining() {
+        if (!mBuffer.hasRemaining()) {
+            uploadBufferInternal();
+        }
+    }
+
+    /**
+     * Waits the native stack to upload data since all bytes have been written.

[pauljensen, 2016/06/10 17:45:44]

data->mBuffer's contents

[pauljensen, 2016/06/10 17:45:44]

since...->because the client has provided all bytes to be uploaded and there is
no need to wait for or expect the client to provide more bytes.

[pauljensen, 2016/06/10 17:45:44]

the->for the

[xunjieli, 2016/06/10 18:48:12]

Done.

[xunjieli, 2016/06/10 18:48:12]

Done.

[xunjieli, 2016/06/10 18:48:12]

Done.

+     */
+    private void uploadIfComplete() {
         if (mBytesWritten == mContentLength) {
             // Entire post data has been received. Now wait for network stack to
             // read it.
-            mMessageLoop.loop();
+            uploadBufferInternal();
         }
     }
 
     /**
+     * Helper function to upload the buffer to the native stack. This function

[pauljensen, 2016/06/10 17:45:44]

buffer->{@code mBuffer}

[xunjieli, 2016/06/10 18:48:12]

Done.

+     * blocks until {@code mBuffer} is consumed and there is space to write more
+     * data.
+     */
+    private void uploadBufferInternal() {
+        mBuffer.flip();
+        mMessageLoop.loop();
+    }
+
+    /**
      * Throws {@link java.net.ProtocolException} if adding {@code numBytes} will
      * exceed content length.
      */
@@ -141,25 +166,19 @@ final class CronetFixedModeOutputStream extends CronetOutputStream {
 
         @Override
         public void read(final UploadDataSink uploadDataSink, final ByteBuffer byteBuffer) {
-            final int availableSpace = byteBuffer.remaining();
-            if (availableSpace < mBuffer.position()) {
-                // byteBuffer does not have enough capacity, so only put a portion
-                // of mBuffer in it.
-                byteBuffer.put(mBuffer.array(), 0, availableSpace);
-                mBuffer.position(availableSpace);
-                // Move remaining buffer to the head of the buffer for use in the
-                // next read call.
-                mBuffer.compact();
-            } else {
-                // byteBuffer has enough capacity to hold the content of mBuffer.
-                mBuffer.flip();
+            if (byteBuffer.remaining() >= mBuffer.remaining()) {
                 byteBuffer.put(mBuffer);
                 // Reuse this buffer.
                 mBuffer.clear();
+                uploadDataSink.onReadSucceeded(false);
                 // Quit message loop so embedder can write more data.
                 mMessageLoop.quit();
+            } else {
+                mBuffer.position(mBuffer.position() + byteBuffer.remaining());
+                byteBuffer.put(mBuffer.array(), mBuffer.position() - byteBuffer.remaining(),
+                        byteBuffer.remaining());
+                uploadDataSink.onReadSucceeded(false);
             }
-            uploadDataSink.onReadSucceeded(false);
         }
 
         @Override