[Cronet] Implement getOutputStream in CronetHttpURLConnection

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

Index: components/cronet/android/java/src/org/chromium/net/urlconnection/CronetBufferedOutputStream.java
diff --git a/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetBufferedOutputStream.java b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetBufferedOutputStream.java
new file mode 100644
index 0000000000000000000000000000000000000000..33771465d5b3394cdf76733d758d5ee068912812
--- /dev/null
+++ b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetBufferedOutputStream.java
@@ -0,0 +1,137 @@
+// 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.urlconnection;
+
+import org.chromium.net.UploadDataProvider;
+import org.chromium.net.UploadDataSink;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.ProtocolException;
+import java.nio.ByteBuffer;
+
+/**
+ * An implementation of {@link java.io.OutputStream} that buffers entire request
+ * body in memory. This is used when neither
+ * {@link CronetHttpURLConnection#setFixedLengthStreamingMode}
+ * or {@link CronetHttpURLConnection#setChunkedStreamingMode} is set.

[mmenke, 2015/03/25 18:34:57]

nit:  or -> nor

[xunjieli, 2015/03/27 17:46:44]

Done.

+ */
+final class CronetBufferedOutputStream extends OutputStream
+        implements UploadDataProvider {
+    // If content length is not passed in the constructor, this is -1.
+    private final int mInitialContentLength;
+    private final CronetHttpURLConnection mConnection;
+    // Internal buffer that is used to buffer the request body.
+    private final ByteArrayOutputStream mBuffer;
+    // Number of bytes consumed by the native UploadDataStream.
+    private int mBytesConsumed;
+
+    /**
+     * Package protected constructor.
+     * @param connection The CronetHttpURLConnection object.
+     * @param contentLength The content length of the request body. It must not
+     *            be smaller than 0 or bigger than {@link Integer.MAX_VALUE}.
+     */
+    CronetBufferedOutputStream(final CronetHttpURLConnection connection,
+            final long contentLength) {
+        if (connection == null) {
+            throw new NullPointerException();
+        }
+
+        if (contentLength > Integer.MAX_VALUE) {
+            throw new IllegalStateException("Use setFixedLengthStreamingMode()"
+                + " or setChunkedStreamingMode() for requests larger than 2GB.");
+        }
+        if (contentLength < 0) {
+            throw new IllegalArgumentException("Content length < 0.");
+        }
+        mConnection = connection;
+        mInitialContentLength = (int) contentLength;
+        mBuffer = new ByteArrayOutputStream(mInitialContentLength);
+        mBytesConsumed = 0;
+    }
+
+    /**
+     * Package protected constructor used when content length is not known.
+     * @param connection The CronetHttpURLConnection object.
+     */
+    CronetBufferedOutputStream(final CronetHttpURLConnection connection) {
+        if (connection == null) {
+            throw new NullPointerException();
+        }
+
+        mConnection = connection;
+        mInitialContentLength = -1;
+        // Bufferring without knowing content-length.

[mmenke, 2015/03/25 18:34:57]

nit:  Buffering.  Last letter often isn't doubled for words with more than on
syllable.  Don't ask me why.

[xunjieli, 2015/03/27 17:46:44]

Done. :)

+        mBuffer = new ByteArrayOutputStream();
+        mBytesConsumed = 0;
+    }
+
+    @Override
+    public void write(int oneByte) throws IOException {
+        checkNotExceedContentLength(1);

[mmenke, 2015/03/25 18:34:57]

Should we also check that the connection hasn't started yet?  We don't have any
protection against adding data after connection when mInitialContentLength is
-1.

[xunjieli, 2015/03/27 17:46:44]

Strangely the default implementation allows writing after connect() when initial
content length is not specified. Not sure whether that's working as intended or
a bug. We can't really match that behavior easily, so I added a java side check.

+        mBuffer.write((byte) oneByte);
+        if (mBuffer.size() == mInitialContentLength) {
+            // Entire post data has been received. Now start the request.
+            mConnection.connect();
+        }
+    }
+
+    @Override
+    public void write(byte[] buffer, int offset, int count) throws IOException {
+        checkNotExceedContentLength(count);
+        mBuffer.write(buffer, offset, count);
+        if (mBuffer.size() == mInitialContentLength) {
+            // Entire post data has been received. Now start the request.
+            mConnection.connect();
+        }
+    }
+
+    // TODO(xunjieli): implement close().
+
+    /**
+     * Throws {@link java.net.ProtocolException} if adding {@code numBytes} will
+     * exceed content length.
+     */
+    private void checkNotExceedContentLength(int numBytes) throws ProtocolException {
+        if (mInitialContentLength != -1
+                && mBuffer.size() + numBytes > mInitialContentLength) {
+            throw new ProtocolException("exceeded content-length limit of "
+                    + mInitialContentLength + " bytes");
+        }
+    }
+
+    // Below are UploadDataProvider implementations. Only intended to be used
+    // within Cronet.
+
+    @Override
+    public long getLength() {
+        // This method is supposed to be called just before starting the request.
+        // If content length is not initially passed in, the number of bytes
+        // written will be used as the content length.
+        if (mInitialContentLength == -1) {
+            return mBuffer.size();
+        }
+        return mInitialContentLength;
+    }
+
+    @Override
+    public void read(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) {
+        int toConsume = Math.min(byteBuffer.remaining(),
+                mBuffer.size() - mBytesConsumed);
+        if (toConsume > 0) {
+            byteBuffer.put(mBuffer.toByteArray(), mBytesConsumed, toConsume);

[mmenke, 2015/03/25 18:34:57]

The docs for toByteArray says it's a copy of the buffer's memory, so this may
copy the entire array on every read, which is really inefficient.

We could just cache the array, but that's still inefficient...

We could use byteBuffer.slice(), though, which creates another view of the same
buffer, and then set the position() and limit() of the slice appropriately.

Or perhaps better, just update them for mBuffer itself for the copy, and then
restore the values as needed.

[xunjieli, 2015/03/27 17:46:44]

Hmm.. Not sure if I understand, mBuffer is an ByteArrayOutputStream (used it
because it grows automatically). It does not support updating position and
limit. 

[mmenke, 2015/03/27 17:54:10]

Ahh, right.  This is still a problem, since ByteArrayOutputStream.toByteArray()
returns a copy, calling it every time is expensive.

Can we switch to a ByteBuffer, and then just set the position and limit as
needed?

+        }
+        mBytesConsumed += toConsume;
+        uploadDataSink.onReadSucceeded(false);
+    }
+
+    @Override
+    public void rewind(UploadDataSink uploadDataSink) {
+        mBytesConsumed = 0;
+        uploadDataSink.onRewindSucceeded();
+    }
+}