[Cronet] Implement getOutputStream in CronetHttpURLConnection

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

 // Copyright 2014 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 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;
 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;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
 /**
  * An implementation of HttpURLConnection that uses Cronet to send requests and
  * receive response. This class inherits a {@code connected} field from the
  * superclass. That field indicates whether a connection has ever been
  * 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;
+    // Use a member variable for the long version of fixed content length, since
+    // super.fixedContentLengthLong is only added in API 19.
+    private long mFixedContentLength = -1;
 
-    protected CronetHttpURLConnection(URL url,
+    public CronetHttpURLConnection(URL url,
             UrlRequestContext urlRequestContext) {
         super(url);
         mUrlRequestContext = urlRequestContext;
         mMessageLoop = new MessageLoop();
         mRequest = mUrlRequestContext.createRequest(url.toString(),
                 new CronetUrlRequestListener(), mMessageLoop);
         mInputStream = new CronetInputStream(this);
         mRequestHeaders = new ArrayList<Pair<String, String>>();
     }
 
     /**
      * Opens a connection to the resource. If the connect method is called when
      * the connection has already been opened (indicated by the connected field
      * having the value true), the call is ignored unless an exception is thrown
      * previously, in which case, the exception will be rethrown.
      */
     @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 (!getUseCaches()) {
-            mRequest.disableCache();
-        }
-        mRequest.start();
-        // Blocks until onResponseStarted or onFailed is called.
-        mMessageLoop.loop();
-        checkHasResponse();
+        startRequest(true);
     }
 
     /**
      * Releases this connection so that its resources may be either reused or
      * closed.
      */
     @Override
     public void disconnect() {
         // Disconnect before connection is made should have no effect.
         if (connected) {
@@ skipping 100 matching lines @@
             throw new IOException("Cannot read response body of a redirect.");
         }
         // Emulate default implementation's behavior to throw
         // FileNotFoundException when we get a 400 and above.
         if (mResponseInfo.getHttpStatusCode() >= HTTP_BAD_REQUEST) {
             throw new FileNotFoundException(url.toString());
         }
         return mInputStream;
     }
 
+    @Override
+    public OutputStream getOutputStream() throws IOException {
+        if (mOutputStream == null) {
+            if (connected) {
+                throw new ProtocolException(
+                        "Cannot write to OutputStream after receiving response.");
+            }
+            if (mFixedContentLength != -1) {
+                addRequestProperty("Content-Length",
+                        Long.toString(mFixedContentLength));
+                mOutputStream = new CronetOutputStream(this, mFixedContentLength);
+                // Start the request now since all headers can be sent.
+                startRequest(false);
+            } else {
+                // For the buffered case, start the request only when
+                // content-length bytes are received, or when an
+                // 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, -1);
+                } else {
+                    long lengthParsed = Long.parseLong(length);
+                    mOutputStream = new CronetBufferedOutputStream(this, lengthParsed);
+                }
+            }
+        }
+        return mOutputStream;
+    }
+
+    /**
+     * Starts the request if {@code connected} is false. If {@code readResponse}
+     * is true, block until the response is received.
+     */
+    private void startRequest(boolean readResponse) throws IOException {
+        if (connected) {
+            if (readResponse) {
+                checkHasResponse();
+            }
+            return;
+        }
+        if (mOutputStream != null) {
+            // Default Content-Type to application/x-www-form-urlencoded
+            if (getRequestProperty("Content-Type") == null) {
+                addRequestProperty("Content-Type",
+                        "application/x-www-form-urlencoded");
+            }
+            mRequest.setUploadDataProvider((UploadDataProvider) mOutputStream,
+                    mMessageLoop);
+        }
+        connected = true;
+        // Start the request. Note that connect() is not used since

[pauljensen, 2015/03/10 17:31:05]

Can we move this comment down so it's right before mRequest.start()?  I think it
makes more sense there.

[xunjieli, 2015/03/12 21:55:09]

Done.

+        // connect() blocks until headers are received.

[pauljensen, 2015/03/10 17:31:05]

I think it's a little odd to mention using connect() in a function that
implements connect().  Part of me wonders if we should remove the readResponse
logic and change connect() to:
  if (connected) checkHasResponse();
  startRequest();
  // Blocks until onResponseStarted or onFailed is called.
  mMessageLoop.loop();
  checkHasResponse();
or perhaps even simpler:
  if (!connected) {
    startRequest();
    // Blocks until onResponseStarted or onFailed is called.
    mMessageLoop.loop();
  }
  checkHasResponse();

[xunjieli, 2015/03/12 21:55:09]

Done. Good idea! I was thinking connect() shouldn't block when I did the
readResponse logic, but ended up not finding a way around it. Thanks!

+        for (Pair<String, String> requestHeader : mRequestHeaders) {
+            mRequest.addHeader(requestHeader.first, requestHeader.second);
+        }
+        if (!getUseCaches()) {
+            mRequest.disableCache();
+        }
+        mRequest.start();
+        if (readResponse) {
+            // Blocks until onResponseStarted or onFailed is called.
+            mMessageLoop.loop();
+            checkHasResponse();
+        }
+    }
+
     /**
      * 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.
      */
     @Override
     public InputStream getErrorStream() {
         try {
             connect();
         } catch (IOException e) {
             return null;
@@ skipping 83 matching lines @@
     /**
      * Returns whether this connection uses a proxy server.
      */
     @Override
     public boolean usingProxy() {
         // TODO(xunjieli): implement this.
         return false;
     }
 
     /**
+     * Sets chunked streaming mode.
+     */
+    @Override
+    public void setChunkedStreamingMode(int chunklen) {
+        // TODO(xunjieli): implement this.
+        throw new UnsupportedOperationException("Chunked mode not supported yet");
+    }
+
+    @Override
+    public void setFixedLengthStreamingMode(int contentLength) {
+        setFixedLengthStreamingMode((long) contentLength);
+    }
+
+    @Override

[pauljensen, 2015/03/10 17:31:05]

Can we be overriding this if it is not present on prior API versions?

[xunjieli, 2015/03/12 21:55:09]

There's no warning, so I guess it's not prohibited. I have adopted your
reflection suggestion.

+    public void setFixedLengthStreamingMode(long contentLength) {
+        if (super.connected) {
+            throw new IllegalStateException("Already connected");
+        }
+        if (super.chunkLength > 0) {
+            throw new IllegalStateException("Already in chunked mode");
+        }
+        if (contentLength < 0) {
+            throw new IllegalArgumentException("contentLength < 0");
+        }
+        mFixedContentLength = contentLength;
+        super.fixedContentLength = (int) Math.min(contentLength, Integer.MAX_VALUE);

[pauljensen, 2015/03/10 17:31:05]

I still feel like this should call super.setFixedLengthStreamingMode(),
otherwise the super's long version won't be updated properly.  Part of me
wonders if we should be overriding this at all, and instead replace uses of
mFixedContentLength with accesses to super.fixedContentLength?  Perhaps we'll
have to use some reflection to see if the parent has the long field.

[xunjieli, 2015/03/12 21:55:09]

Done. Thanks for the suggestion!

+    }
+
+    /**
      * 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.
      */
     ByteBuffer getMoreData() throws IOException {
         mResponseByteBuffer = null;
         mMessageLoop.loop();
         return mResponseByteBuffer;
     }
 
     /**
+     * Used by {@link CronetOutputStream} to wait for data consumed by the
+     * network stack before letting the consumer write more data.
+     */
+    void waitForPostDataConsumed() throws IOException {
+        mMessageLoop.loop();
+    }
+
+    /**
+     * Used by {@link CronetOutputStream} to wait for consumer to write more data.
+     */
+    void waitForPostData() {
+        mMessageLoop.postQuitTask();
+    }
+
+    /**
      * Returns the index of request header in {@link #mRequestHeaders} or
      * -1 if not found.
      */
     private int findRequestProperty(String key) {
         for (int i = 0; i < mRequestHeaders.size(); i++) {
             Pair<String, String> entry = mRequestHeaders.get(i);
             if (entry.first.equalsIgnoreCase(key)) {
                 return i;
             }
         }
@@ skipping 92 matching lines @@
             return null;
         }
         List<Pair<String, String>> headers =
                 mResponseInfo.getAllHeadersAsList();
         if (pos >= headers.size()) {
             return null;
         }
         return headers.get(pos);
     }
 }