[Cronet] Implement getOutputStream in CronetHttpURLConnection

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

Index: components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
diff --git a/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
index 585b6e6af7c75855eac1b29dae12a5e015f4fae0..1fa7ceb7db8e446b2182f164edd6041f50da620a 100644
--- a/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
+++ b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
@@ -4,10 +4,12 @@
 
 package org.chromium.net.urlconnection;
 
+import android.util.Log;
 import android.util.Pair;
 
 import org.chromium.net.ExtendedResponseInfo;
 import org.chromium.net.ResponseInfo;
+import org.chromium.net.UploadDataProvider;
 import org.chromium.net.UrlRequest;
 import org.chromium.net.UrlRequestContext;
 import org.chromium.net.UrlRequestException;
@@ -16,8 +18,10 @@ import org.chromium.net.UrlRequestListener;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.OutputStream;
 import java.net.HttpURLConnection;
 import java.net.MalformedURLException;
+import java.net.ProtocolException;
 import java.net.URL;
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
@@ -33,18 +37,20 @@ import java.util.TreeMap;
  * attempted.
  */
 public class CronetHttpURLConnection extends HttpURLConnection {
+    private static final String TAG = "CronetHttpURLConnection";
     private final UrlRequestContext mUrlRequestContext;
     private final MessageLoop mMessageLoop;
     private final UrlRequest mRequest;
     private final List<Pair<String, String>> mRequestHeaders;
 
     private CronetInputStream mInputStream;
+    private OutputStream mOutputStream;
     private ResponseInfo mResponseInfo;
     private UrlRequestException mException;
     private ByteBuffer mResponseByteBuffer;
     private boolean mOnRedirectCalled = false;
 
-    protected CronetHttpURLConnection(URL url,
+    public CronetHttpURLConnection(URL url,
             UrlRequestContext urlRequestContext) {
         super(url);
         mUrlRequestContext = urlRequestContext;
@@ -63,20 +69,11 @@ public class CronetHttpURLConnection extends HttpURLConnection {
      */
     @Override
     public void connect() throws IOException {
-        if (connected) {
-            checkHasResponse();
-            return;
-        }
-        connected = true;
-        for (Pair<String, String> requestHeader : mRequestHeaders) {
-            mRequest.addHeader(requestHeader.first, requestHeader.second);
+        if (!connected) {
+            startRequest();
+            // Blocks until onResponseStarted or onFailed is called.
+            mMessageLoop.loop();
         }
-        if (!getUseCaches()) {
-            mRequest.disableCache();
-        }
-        mRequest.start();
-        // Blocks until onResponseStarted or onFailed is called.
-        mMessageLoop.loop();
         checkHasResponse();

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

I'm still hazy on this API.  If you're uploading, you aren't allowed to call
connect(), unless you're already done passing along the entire body for upload? 
In the CronetOutputStream case, the checkHasResponse() line will fail.  In the
other case, things will also go badly.

Should we explicitly check for that case?

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

Looking at Okhttp's implementation, connect() does not read the response, so we
probably should not block in connect(). Updated the implementation.

     }
 
@@ -198,6 +195,94 @@ public class CronetHttpURLConnection extends HttpURLConnection {
         return mInputStream;
     }
 
+    @Override
+    public OutputStream getOutputStream() throws IOException {
+        if (mOutputStream == null) {
+            if (connected) {
+                throw new ProtocolException(
+                        "Cannot write to OutputStream after receiving response.");
+            }
+            long fixedStreamingModeContentLength = getStreamingModeContentLength();
+            if (fixedStreamingModeContentLength != -1) {
+                mOutputStream = new CronetOutputStream(this,
+                        fixedStreamingModeContentLength);
+                // Start the request now since all headers can be sent.
+                startRequest();
+            } else {
+                // For the buffered case, start the request only when
+                // content-length bytes are received, or when an

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

nit:  an -> a

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

Done.

+                // connect action is initiated by the consumer.
+                Log.d(TAG, "Outputstream is being buffered in memory.");
+                String length = getRequestProperty("Content-Length");
+                if (length == null) {
+                    mOutputStream = new CronetBufferedOutputStream(this);
+                } else {
+                    long lengthParsed = Long.parseLong(length);
+                    mOutputStream = new CronetBufferedOutputStream(this, lengthParsed);
+                }
+            }
+        }
+        return mOutputStream;
+    }
+
+    /**
+     * Helper method to get content length passed in by
+     * {@link #setFixedLengthStreamingMode}
+     */
+    private long getStreamingModeContentLength() {
+        long contentLength = fixedContentLength;
+        // Use reflection to see whether fixedContentLengthLong (only added
+        // in API 19) is inherited.
+        try {
+            Class<?> parent = this.getClass();
+            long superFixedContentLengthLong =
+                    parent.getField("fixedContentLengthLong").getLong(this);
+            if (superFixedContentLengthLong != -1) {
+                contentLength = superFixedContentLengthLong;
+            }
+        } catch (Exception e) {
+            // Ignored.
+        }
+        return contentLength;
+    }
+
+    /**
+     * Starts the request if {@code connected} is false.
+     */
+    private void startRequest() throws IOException {
+        if (connected) {
+            return;
+        }
+        if (doOutput) {
+            long fixedStreamingModeContentLength = getStreamingModeContentLength();
+            if (fixedStreamingModeContentLength != -1) {
+                // Do not use UploadDataProvider.getLength() because if
+                // mOutputStream is null, the header won't be set.
+                addRequestProperty("Content-Length",
+                        Long.toString(fixedStreamingModeContentLength));
+            }
+            if (mOutputStream != null) {
+                mRequest.setUploadDataProvider(
+                        (UploadDataProvider) mOutputStream, mMessageLoop);
+            }
+            // Default Content-Type to application/x-www-form-urlencoded
+            if (getRequestProperty("Content-Type") == null) {
+                addRequestProperty("Content-Type",
+                        "application/x-www-form-urlencoded");
+            }
+        }
+        connected = true;
+        // Start the request. Note that connect() is not used since
+        // connect() blocks until headers are received.

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

This comment about connect is a little weird, since this is now the guts of
connect().

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

Done. Ah.. sorry should have removed it.

+        for (Pair<String, String> requestHeader : mRequestHeaders) {
+            mRequest.addHeader(requestHeader.first, requestHeader.second);
+        }
+        if (!getUseCaches()) {
+            mRequest.disableCache();
+        }
+        mRequest.start();
+    }
+
     /**
      * Returns an input stream from the server in the case of an error such as
      * the requested file has not been found on the remote server.
@@ -301,6 +386,15 @@ public class CronetHttpURLConnection extends HttpURLConnection {
     }
 
     /**
+     * Sets chunked streaming mode.
+     */
+    @Override
+    public void setChunkedStreamingMode(int chunklen) {
+        // TODO(xunjieli): implement this.
+        throw new UnsupportedOperationException("Chunked mode not supported yet");
+    }
+
+    /**
      * Used by {@link CronetInputStream} to get more data from the network
      * stack. This should only be called after the request has started. Note
      * that this call might block if there isn't any more data to be read.
@@ -312,6 +406,23 @@ public class CronetHttpURLConnection extends HttpURLConnection {
     }
 
     /**
+     * Used by {@link CronetOutputStream} to wait for data to be read by the
+     * network stack before letting the consumer write more data.
+     */
+    void waitForRead() throws IOException {
+        mMessageLoop.loop();
+    }
+
+    /**
+     * Used by {@link CronetOutputStream} to execute read after consumer writes
+     * more data.
+     */
+    void postponeRead(Runnable readTask) {
+        mMessageLoop.postQuitTask();
+        mMessageLoop.execute(readTask);
+    }
+
+    /**
      * Returns the index of request header in {@link #mRequestHeaders} or
      * -1 if not found.
      */