[Cronet] Initial implementation of HttpURLConnection

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
new file mode 100644
index 0000000000000000000000000000000000000000..be16be0918f33855ca50425b97535335d4513b44
--- /dev/null
+++ b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
@@ -0,0 +1,218 @@
+// 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 org.chromium.net.ExtendedResponseInfo;
+import org.chromium.net.ResponseInfo;
+import org.chromium.net.UrlRequest;
+import org.chromium.net.UrlRequestContext;
+import org.chromium.net.UrlRequestException;
+import org.chromium.net.UrlRequestListener;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.nio.ByteBuffer;
+import java.util.concurrent.Executor;
+
+/**
+ * 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 final UrlRequestContext mUrlRequestContext;
+    private final MessageLoop mMessageLoop;
+    private final UrlRequest mRequest;
+
+    private ResponseInfo mResponseInfo;
+    private CronetInputStream mInputStream;
+    private ByteBuffer mResponseByteBuffer;
+
+    protected CronetHttpURLConnection(URL url,
+            UrlRequestContext urlRequestContext) {
+        super(url);
+        mUrlRequestContext = urlRequestContext;
+        mMessageLoop = new MessageLoop();
+        mRequest = mUrlRequestContext.createRequest(url.toString(),
+                new CronetUrlRequestListener(), new MessageLoopExecutor());
+    }
+
+    /**
+     * Wrapper executor class which posts tasks to message loop.
+     */
+    private class MessageLoopExecutor implements Executor {
+        public MessageLoopExecutor() {
+        }
+
+        @Override
+        public void execute(Runnable command) {
+            mMessageLoop.postTask(command);
+        }
+    }
+
+    /**
+     * 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.
+     */
+    @Override
+    public void connect() throws IOException {
+        maybeStartRequest();
+    }
+
+    /**
+     * Releases this connection so that its resources may be either reused or
+     * closed.
+     */
+    @Override
+    public void disconnect() {

[mmenke, 2014/11/26 17:09:12]

We should also cancel the URLRequest if started.  This makes sure its resources
are freed.  Otherwise, disconnecting after a request is started results in a
leak.

If not started, should we make sure we don't start it in the future?  Should
match whatever the other implementation does.

[mmenke, 2014/11/26 17:09:12]

Should we set connected to false here?  I assume we should leave it as true
after a request successfully completes.  What if a request fails?

Also note that setting connected to false here would break maybeStartRequest, so
would need changes there, too.

[xunjieli, 2014/11/26 18:37:14]

We should not set connected to false, since it indicates whether a connection
has been ever attempted.

[xunjieli, 2014/11/26 18:37:14]

Done. If not started (connected), the disconnect() should have no effect,
according to the documentation of okHttp.

+        if (mInputStream != null) {
+            try {
+                mInputStream.close();
+            } catch (IOException e) {
+                e.printStackTrace();
+            }
+        }
+    }
+
+    /**
+     * Returns the response message returned by the remote HTTP server.
+     */
+    @Override
+    public String getResponseMessage() {
+        maybeStartRequest();
+        return mResponseInfo.getHttpStatusText();
+    }
+
+    /**
+     * Returns the response code returned by the remote HTTP server.
+     */
+    @Override
+    public int getResponseCode() {
+        maybeStartRequest();
+        return mResponseInfo.getHttpStatusCode();

[mmenke, 2014/11/26 17:09:12]

Note that these currently throw null pointer exceptions if a request fails
without getting headers.

[xunjieli, 2014/11/26 18:37:14]

Done.

+    }
+
+    /**
+     * Returns an InputStream for reading data from the resource pointed by this
+     * URLConnection.
+     */
+    @Override
+    public InputStream getInputStream() {
+        if (mInputStream == null) {
+            maybeStartRequest();
+            mInputStream = new CronetInputStream(this);
+        }
+        return mInputStream;
+    }
+
+    /**
+     * Adds the given property to the request header.
+     */
+    @Override
+    public final void addRequestProperty(String key, String value) {
+        setRequestProperty(key, value);
+    }
+
+    /**
+     * Sets the value of the specified request header field.
+     */
+    @Override
+    public final void setRequestProperty(String key, String value) {
+        if (connected) {
+            throw new IllegalStateException(
+                    "Cannot set request property after connection is made");
+        }
+        mRequest.addHeader(key, value);
+    }
+
+    /**
+     * Returns whether this connection uses a proxy server or not.
+     */
+    @Override
+    public boolean usingProxy() {
+        // TODO(xunjieli): implement this.
+        return false;
+    }
+
+    /**
+     * 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() {
+        mMessageLoop.loop();
+        return mResponseByteBuffer;

[mmenke, 2014/11/26 17:09:12]

Suggest setting mResponseByteBuffer to NULL before returning it.  Having joint
ownership of the old buffer just seems a recipe for problems.  When we end up
returning the last used mResponseByteBuffer (On failure or complete), the
InputStream should already have read all of it, but it just seems like asking
for trouble.

[xunjieli, 2014/11/26 18:37:14]

Done.

+    }
+
+    private class CronetUrlRequestListener implements UrlRequestListener {
+        public CronetUrlRequestListener() {
+        }
+
+        @Override
+        public void onResponseStarted(UrlRequest request, ResponseInfo info) {
+            mResponseInfo = info;
+            // Quits the message loop since we have the headers now.
+            mMessageLoop.postQuitTask();
+        }
+
+        @Override
+        public void onDataReceived(UrlRequest request, ResponseInfo info,
+                ByteBuffer byteBuffer) {
+            mResponseInfo = info;
+            mResponseByteBuffer = ByteBuffer.allocate(byteBuffer.capacity());
+            mResponseByteBuffer.put(byteBuffer);
+            mResponseByteBuffer.rewind();

[mmenke, 2014/11/26 17:09:12]

Suggest using mResponseByteBuffer.flip() instead - think it's more common when
switching from reading to writing a buffer.  Also, if we switch to reusing byte
buffers, it's the one we'll probably need.

[xunjieli, 2014/11/26 18:37:14]

Done.

+            mMessageLoop.postQuitTask();
+        }
+
+        @Override
+        public void onRedirect(UrlRequest request, ResponseInfo info,
+                String newLocationUrl) {
+            // TODO(xunjieli): Handle redirect.
+            mResponseInfo = info;

[mmenke, 2014/11/26 17:09:12]

If we follow a couple redirects, and then get an error, getResponseCode and the
like will return the value for the last successfully received redirect.  Is this
what we should be doing?  If not, should just remove this line.

And should add a TODO about testing this (requires test infrastructure changes,
I suspect)

[xunjieli, 2014/11/26 18:37:14]

Done.

+        }
+
+        @Override
+        public void onSucceeded(UrlRequest request, ExtendedResponseInfo info) {

[mmenke, 2014/11/26 17:09:12]

Set mResponseInfo?

[xunjieli, 2014/11/26 18:37:14]

Done.

+            setResponseDataCompleted();
+        }
+
+        @Override
+        public void onFailed(UrlRequest request, ResponseInfo info,
+                UrlRequestException exception) {
+            // TODO(xunjieli): Handle failure.
+            setResponseDataCompleted();
+        }
+
+        /**
+         * Notifies {@link #mInputStream} that transferring of response data has
+         * completed.
+         */
+        private void setResponseDataCompleted() {
+            if (mInputStream != null) {
+                mInputStream.setResponseDataCompleted();
+            }
+            mMessageLoop.postQuitTask();
+        }
+    }
+
+    /**
+     * On first call, creates and starts UrlRequest, and waits until response
+     * headers are received. Does nothing on subsequent calls.
+     */
+    private void maybeStartRequest() {
+        if (connected) {
+            return;
+        }
+        connected = true;
+        mRequest.start();
+        // Blocks until onResponseStarted or onFailed is called.
+        mMessageLoop.loop();
+    }
+}