[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..7c0ed64a6c2e30015ad7ad79aa3f5f71ffa4e3a9
--- /dev/null
+++ b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
@@ -0,0 +1,213 @@
+// 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.BlockingQueue;
+import java.util.concurrent.Executor;
+import java.util.concurrent.LinkedBlockingQueue;
+
+/**
+ * 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 UrlRequest mRequest;
+
+    private ResponseInfo mResponseInfo;
+
+    private InputStream mResponseBody;
+
+    private final MessageLoop mMessageLoop;
+
+    private final BlockingQueue<MessageLoop.Message> mMessageQueue;

[mmenke, 2014/11/18 15:53:52]

nit:  Don't think we need a blank line between every pair of variables.  Suggest
grouping related ones.

[xunjieli, 2014/11/18 18:13:35]

Done.

+
+

[mmenke, 2014/11/18 15:53:52]

nit:  Remove extra line break.

[xunjieli, 2014/11/18 18:13:36]

Done.

+    protected CronetHttpURLConnection(URL url,
+            UrlRequestContext urlRequestContext) {
+        super(url);
+        mUrlRequestContext = urlRequestContext;
+        mMessageQueue = new LinkedBlockingQueue<MessageLoop.Message>();
+        mMessageLoop = new MessageLoop(mMessageQueue);
+    }
+
+

[mmenke, 2014/11/18 15:53:52]

nit:  Remove extra blank line.

[xunjieli, 2014/11/18 18:13:35]

Done.

+    /**
+     * Wrapper executor class which posts tasks to current looper.
+     */
+    private class HandlerThreadExecutor implements Executor {
+

[mmenke, 2014/11/18 15:53:52]

nit:  -blank line.

[xunjieli, 2014/11/18 18:13:35]

Done.

+        public HandlerThreadExecutor() {
+        }
+
+        @Override
+        public void execute(Runnable command) {
+            try {
+                mMessageQueue.put(new MessageLoop.Message(command));
+            } catch (InterruptedException e) {
+                e.printStackTrace();
+            }
+        }
+    }
+
+    /**
+     * Opens a connection to the resource.
+     */
+    @Override
+    public void connect() throws IOException {
+        maybeStartRequest();
+    }
+
+    /**
+     * Releases this connection so that its resources may be either reused or
+     * closed.
+     */
+    @Override
+    public void disconnect() {
+        if (mResponseBody != null) {
+            try {
+                mResponseBody.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();
+    }
+
+    /**
+     * Returns an InputStream for reading data from the resource pointed by this
+     * URLConnection.
+     */
+    @Override
+    public InputStream getInputStream() {
+        maybeStartRequest();
+        return mResponseBody;
+    }
+
+    /**
+     * 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");
+        }
+        maybeCreateRequest();
+        mRequest.addHeader(key, value);
+    }
+
+    /**
+     * Returns whether this connection uses a proxy server or not.
+     */
+    @Override
+    public boolean usingProxy() {
+        // TODO(xunjieli): implement this.
+        return false;
+    }
+
+    private class CronetUrlRequestListener implements UrlRequestListener {
+

[mmenke, 2014/11/18 15:53:52]

nit:  -blank line

[xunjieli, 2014/11/18 18:13:36]

Done.

+        public CronetUrlRequestListener() {
+        }
+
+        @Override
+        public void onResponseStarted(UrlRequest request, ResponseInfo info) {
+            mResponseInfo = info;
+        }
+
+        @Override
+        public void onDataReceived(UrlRequest request, ResponseInfo info,
+                ByteBuffer byteBuffer) {
+            mResponseInfo = info;
+            // TODO(xunjieli): handle streaming.

[mmenke, 2014/11/18 15:53:52]

nit:  Capitalize

[xunjieli, 2014/11/18 18:13:35]

Done.

+            mResponseBody = new ByteBufferInputStream(byteBuffer);
+        }
+
+        @Override
+        public void onRedirect(UrlRequest request, ResponseInfo info,
+                String newLocationUrl) {
+            // TODO(xunjieli): handle redirect.

[mmenke, 2014/11/18 15:53:52]

nit:  Capitalize

[xunjieli, 2014/11/18 18:13:35]

Done.

+            mResponseInfo = info;
+        }
+
+        @Override
+        public void onSucceeded(UrlRequest request, ExtendedResponseInfo info) {
+            mMessageLoop.quit();
+        }
+
+        @Override
+        public void onFailed(UrlRequest request, ResponseInfo info,
+                UrlRequestException exception) {
+            // TODO(xunjieli): handle failure.

[mmenke, 2014/11/18 15:53:52]

nit:  Capitalize

[xunjieli, 2014/11/18 18:13:36]

Done.

+            mMessageLoop.quit();
+        }
+    }
+
+    /**
+     * Maybe starts {@code mRequest}, and waits for it to complete.

[mmenke, 2014/11/18 15:53:52]

Should be clearer here.  Maybe "On first call, creates and starts URLRequest,
and waits for it to complete.  Does nothing on subsequent calls."

[xunjieli, 2014/11/18 18:13:36]

Done.

+     */
+    private void maybeStartRequest() {
+        if (connected) {
+            return;
+        }
+        maybeCreateRequest();
+        mRequest.start();
+        connected = true;
+        mMessageLoop.loop();

[mmenke, 2014/11/18 15:53:52]

Reading over the API docs, should we really get the entire body here, or should
we only block as needed?  Ir block until we have the headers, then only block as
the body is read?  The docs don't seem to be clear here, but should presumably
try and duplicate HttpURLConnection's behavior here - in particular, if it
provides bytes just as they are read, rather than caching the entire file into
RAM, we shouldn't be caching the entire file.

[xunjieli, 2014/11/18 18:13:35]

Done. Agreed, we should only block until we receive headers, and block again if
we try to read the body.
But we still have to cache the response in memory right? Not sure if I
understand your suggestion completely. But PTAL at the new patch.

[mmenke, 2014/11/18 18:40:59]

Which method(s) force us to to cache the response in memory?  getContent?  Or is
there another one I'm missing?

+    }
+
+    /**
+     * Maybe creates {@code mRequest} if it is null.
+     */
+    private void maybeCreateRequest() {
+        if (mRequest != null) {

[mmenke, 2014/11/18 15:53:52]

Can mRequest be non-NULL here?

[xunjieli, 2014/11/18 18:13:35]

Yes, if setRequestProperty is called before maybeStartRequest, then mRequest is
non-NULL.

+            return;
+        }
+        mRequest = mUrlRequestContext.createRequest(url.toString(),
+                new CronetUrlRequestListener(), new HandlerThreadExecutor());
+    }
+}