[Cronet] Implement getOutputStream in CronetHttpURLConnection

components/cronet/android/java/src/org/chromium/net/urlconnection/CronetBufferedOutputStream.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.urlconnection;
+
+import org.chromium.net.UploadDataProvider;
+import org.chromium.net.UploadDataSink;
+
+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}
+ * nor {@link CronetHttpURLConnection#setChunkedStreamingMode} is set.
+ */
+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 ByteBuffer mBuffer;
+    private boolean mConnected = false;
+    private boolean mFirstReadInitiated = false;
+
+    /**
+     * 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 = ByteBuffer.allocate(mInitialContentLength);
+    }
+
+    /**
+     * 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;
+        // Buffering without knowing content-length.
+        mBuffer = ByteBuffer.allocate(2048);
+    }
+
+    @Override
+    public void write(int oneByte) throws IOException {
+        checkNotExceedContentLength(1);
+        if (mInitialContentLength == -1 && mBuffer.position() == mBuffer.limit()) {
+            growInternalBuffer(1);
+        }

[mmenke, 2015/03/31 18:19:32]

optional:  Suggest moving this surrounding logic into growInternalBuffer, since
it's duplicated in both write functions.  And maybe rename it (ensureCanWrite or
something?  Then the check that we're not connected could be moved here, too -
it's a little weird in "checkNotExceedContentLength")

[xunjieli, 2015/04/01 17:04:08]

Done. You are totally right! that does make the code cleaner.

+        mBuffer.put((byte) oneByte);
+    }
+
+    @Override
+    public void write(byte[] buffer, int offset, int count) throws IOException {
+        checkNotExceedContentLength(count);
+        if (mInitialContentLength == -1 && mBuffer.limit() - mBuffer.position() < count) {
+            growInternalBuffer(count);
+        }
+        mBuffer.put(buffer, offset, count);
+    }
+
+    /**
+     * Sets {@link #mConnected} to {@code true}.
+     */
+    void setConnected() {
+        mConnected = true;
+    }
+
+    // TODO(xunjieli): implement close().
+
+    /**
+     * Doubles the internal buffer or grows it at least by {@code count} bytes
+     * This method throws an IllegalStateException if the buffer
+     * is not meant to grow because content length is specified.
+     */
+    private void growInternalBuffer(int count) {
+        if (mInitialContentLength != -1) {
+            throw new IllegalStateException("Buffer should not grow.");
+        }
+        int afterSize = Math.max(mBuffer.capacity() * 2, mBuffer.capacity() + count);
+        ByteBuffer newByteBuffer = ByteBuffer.allocate(afterSize);
+        mBuffer.flip();
+        newByteBuffer.put(mBuffer);
+        mBuffer = newByteBuffer;
+    }
+
+    /**
+     * Throws {@link java.net.ProtocolException} if adding {@code numBytes} will
+     * exceed content length.
+     */
+    private void checkNotExceedContentLength(int numBytes) throws ProtocolException {

[mmenke, 2015/03/31 18:19:32]

nit:  checkContentLengthNotExceeded?  Or could just inline it, if you follow my
suggestion about changing growInternalBuffer.

[xunjieli, 2015/04/01 17:04:08]

Done.thanks!

+        if (mConnected) {
+            throw new IllegalStateException("Cannot write after being connected.");
+        }
+        if (mInitialContentLength != -1
+                && mBuffer.position() + numBytes > mInitialContentLength) {
+            // Error message is to match that of the default implementation.
+            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.position();
+        }
+        return mInitialContentLength;
+    }
+
+    @Override
+    public void read(UploadDataSink uploadDataSink, ByteBuffer byteBuffer) {
+        if (!mFirstReadInitiated) {
+            // This is the first read, so flip the buffer.
+            mBuffer.flip();
+            mFirstReadInitiated = true;
+        }

[mmenke, 2015/03/31 18:19:32]

Can we move this to setConnected, and get rid of mFirstReadInitiated?  If we
need mFirstReadInitiated, I'm fine with keeping it, just not a big fan of
variables that aren't actually needed.

[xunjieli, 2015/04/01 17:04:08]

Done. Good idea!

+        int oldPos = byteBuffer.position();
+        int availableSpace = byteBuffer.capacity() - byteBuffer.position();
+        if (availableSpace  < mBuffer.limit() - mBuffer.position()) {
+            byteBuffer.put(mBuffer.array(), mBuffer.position(), availableSpace);
+            mBuffer.position(mBuffer.position() + availableSpace);
+        } else {
+            byteBuffer.put(mBuffer);
+        }
+        uploadDataSink.onReadSucceeded(false);
+    }
+
+    @Override
+    public void rewind(UploadDataSink uploadDataSink) {
+        mBuffer.position(0);
+        uploadDataSink.onRewindSucceeded();
+    }
+}