[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.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+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.
+ */
+final class CronetBufferedOutputStream extends OutputStream
+        implements UploadDataProvider {
+    private final int mInitialContentLength;
+    private final CronetHttpURLConnection mConnection;

[pauljensen, 2015/03/04 16:44:10]

I think this is dead if you agree with my logic on line 74.

[xunjieli, 2015/03/05 17:43:10]

Sorry, getOutputStream() is not organized properly so it isn't very easy to see
that startRequest() isn't invoked there when the OutputStream is of this type.
If we choose to buffer the request body (using this class), we only start the
request when the consumer initiates a connect action, or in the case when
content-length is known, we start the request automatically when we reached that
number of bytes (line 62 & 74).

Since CronetBufferedOutputStream#getLength() is only invoked when we start the
request, the result returned by getLength() is correct.

+    // 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;
+
+    /**
+     * Packaged protected constructor.
+     * @param connection The CronetHttpURLConnection object.
+     * @param contentLength The content length of the request body. If content
+     *            length cannot be determined in advance, use -1.
+     */
+    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.");
+        }
+        mConnection = connection;
+        mInitialContentLength = (int) contentLength;
+        if (mInitialContentLength == -1) {
+            // Bufferring without knowing content-length.
+            mBuffer = new ByteArrayOutputStream();
+        } else {
+            mBuffer = new ByteArrayOutputStream(mInitialContentLength);
+        }
+        mBytesConsumed = 0;
+    }
+
+    @Override
+    public void write(int oneByte) throws IOException {
+        mBuffer.write((byte) oneByte);
+        if (mBuffer.size() == mInitialContentLength) {
+            // Entire post data has been received. Now start the request.
+            mConnection.connect();

[pauljensen, 2015/03/04 16:44:10]

Ditto with line 74.

[xunjieli, 2015/03/05 17:43:10]

above.

+        }
+    }
+
+    @Override
+    public void write(byte[] buffer, int offset, int count) throws IOException {
+        mBuffer.write(buffer, offset, count);
+        if (mInitialContentLength != -1 && mBuffer.size() > mInitialContentLength) {
+            throw new IllegalStateException("Writing out of bound.");
+        }
+        if (mBuffer.size() == mInitialContentLength) {
+            // Entire post data has been received. Now start the request.
+            mConnection.connect();

[pauljensen, 2015/03/04 16:44:10]

If getOutputStream() starts the request, and we know getOutputStream() has
already been called if we have a CronetBufferedOutputStream, then isn't this
code unnecessary?

[xunjieli, 2015/03/05 17:43:10]

above.

+        }
+    }
+
+    @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 < 0) {
+            return mBuffer.size();

[pauljensen, 2015/03/04 16:44:10]

If getOutputStream() starts the request, which AFAIK prompts the call to
getLength(), won't that return 0 which isn't correct?

[xunjieli, 2015/03/05 17:43:10]

above.

+        }
+        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);
+        }
+        mBytesConsumed += toConsume;
+        uploadDataSink.onReadSucceeded(false);
+    }
+
+    @Override
+    public void rewind(UploadDataSink uploadDataSink) {
+        mBytesConsumed = 0;
+        uploadDataSink.onRewindSucceeded();
+    }
+}