Initial implementation of Cronet Async API.

components/cronet/android/cronet_url_request.cc

Index: components/cronet/android/cronet_url_request.cc
diff --git a/components/cronet/android/cronet_url_request.cc b/components/cronet/android/cronet_url_request.cc
new file mode 100644
index 0000000000000000000000000000000000000000..652889ba25601da44092ec7e6a16161a0d63d05e
--- /dev/null
+++ b/components/cronet/android/cronet_url_request.cc
@@ -0,0 +1,276 @@
+// 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.
+
+#include "components/cronet/android/cronet_url_request.h"
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "components/cronet/android/cronet_url_request_adapter.h"
+#include "components/cronet/android/cronet_url_request_context_adapter.h"
+#include "jni/CronetUrlRequest_jni.h"
+#include "net/base/net_errors.h"
+#include "net/base/request_priority.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_util.h"
+
+using base::android::ConvertUTF8ToJavaString;
+
+// This file contains all the plumbing to handle the bidirectional communication
+// between the Java CronetURLRequest and C++ CronetURLRequestAdapter."
+
+namespace cronet {
+namespace {
+
+// A delegate of CronetURLRequestAdapter that delivers callbacks to the Java
+// layer. Created on a Java thread, but always called and destroyed on the
+// Network thread.
+class JniCronetURLRequestAdapterDelegate
+    : public CronetURLRequestAdapter::CronetURLRequestAdapterDelegate {
+ public:
+  JniCronetURLRequestAdapterDelegate(JNIEnv* env, jobject owner) {
+    owner_.Reset(env, owner);
+  }
+
+  // CronetURLRequestAdapter::CronetURLRequestAdapterDelegate implementation.
+
+  void OnRedirect(const GURL& new_location, int http_status_code) override {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    cronet::Java_CronetUrlRequest_onRedirect(
+        env,
+        owner_.obj(),
+        ConvertUTF8ToJavaString(env, new_location.spec()).obj(),
+        http_status_code);
+  }
+
+  void OnResponseStarted(int http_status_code) override {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    cronet::Java_CronetUrlRequest_onResponseStarted(env,
+                                                    owner_.obj(),
+                                                    http_status_code);
+  }
+
+  void OnBytesRead(unsigned char* bytes_buffer,

[mmenke, 2014/11/07 15:19:08]

|bytes_buffer| seems too easy to confuse with the Java byte buffer class (Which,
admittedly, we wrap this in immediately...But that's an implementation detail
the adapter probably shouldn't care about).

My suggestion is just to call it buffer, or bytes, or data, or whatever.

[mef, 2014/11/07 16:22:25]

Done.

+                   int bytes_read) override {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    base::android::ScopedJavaLocalRef<jobject> java_buffer(

[mef, 2014/11/06 23:16:53]

So, it seems that theoretically |bytes_read| could wary from call to call, can't
it?
In this case we either need to pass it back explicitly to the listener, or to
recreate java_buffer every time that it changes. WDYT?

[mmenke, 2014/11/06 23:35:06]

We'd create a Java byte buffer with a capacity of the entire length of
|bytes_buffer| initially.  On each read, we'd then set its position to be
bytes_read, rather than setting its capacity.

[mmenke, 2014/11/07 15:19:08]

Could put it off until a future CL - if we switch to a pull-based API, we'll
need something different, anyways.

That having been said, fine if you want to make the switch on this CL, but I
think we should change the interface if we do.  Add a SetBuffer method, which
takes in the buffer and the size, and then make this just take in a bytes_read -
that makes it clearer from the interface that the buffer doesn't change, and
don't need to worry about whether you should handle the buffer pointer changing
from one call to another in here or not.

Could either pass the buffer to Java in the new function, and keep the pointer
cached there, or pass it in to Java on each read.  Second option may be slightly
preferable, just because the Java class is quite large already, and this is a
pretty small class, so it makes for more readable code...not really a big issue,
either way, though.

[mef, 2014/11/07 16:22:25]

Acknowledged.

+        env, env->NewDirectByteBuffer(bytes_buffer, bytes_read));
+    cronet::Java_CronetUrlRequest_onDataReceived(
+        env, owner_.obj(), java_buffer.obj());
+  }
+
+  void OnRequestFinished() override {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    cronet::Java_CronetUrlRequest_onSucceeded(env, owner_.obj());
+  }
+
+  void OnError(int net_error) override {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    cronet::Java_CronetUrlRequest_onError(
+        env,
+        owner_.obj(),
+        net_error,
+        ConvertUTF8ToJavaString(env, net::ErrorToString(net_error)).obj());
+  }
+
+ private:
+  ~JniCronetURLRequestAdapterDelegate() override {
+  }
+
+  // Java object that owns the CronetURLRequestContextAdapter, which owns this
+  // delegate.
+  base::android::ScopedJavaGlobalRef<jobject> owner_;
+
+  DISALLOW_COPY_AND_ASSIGN(JniCronetURLRequestAdapterDelegate);
+};
+
+}  // namespace
+
+// Explicitly register static JNI functions.
+bool CronetUrlRequestRegisterJni(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+static jlong CreateRequestAdapter(JNIEnv* env,
+                                  jobject jurl_request,
+                                  jlong jurl_request_context_adapter,
+                                  jstring jurl_string,
+                                  jint jpriority) {
+  CronetURLRequestContextAdapter* context_adapter =
+      reinterpret_cast<CronetURLRequestContextAdapter*>(
+          jurl_request_context_adapter);
+  DCHECK(context_adapter);
+
+  GURL url(base::android::ConvertJavaStringToUTF8(env, jurl_string));
+
+  VLOG(1) << "New chromium network request_adapter: "
+          << url.possibly_invalid_spec();
+
+  scoped_ptr<CronetURLRequestAdapter::CronetURLRequestAdapterDelegate> delegate(
+      new JniCronetURLRequestAdapterDelegate(env, jurl_request));
+
+  CronetURLRequestAdapter* adapter = new CronetURLRequestAdapter(
+      context_adapter,
+      delegate.Pass(),
+      url,
+      static_cast<net::RequestPriority>(jpriority));
+
+  return reinterpret_cast<jlong>(adapter);
+}
+
+static jboolean SetHttpMethod(JNIEnv* env,
+                              jobject jurl_request,
+                              jlong jurl_request_adapter,
+                              jstring jmethod) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(!request_adapter->IsOnNetworkThread());
+  std::string method(base::android::ConvertJavaStringToUTF8(env, jmethod));
+  // Http method is a token, just as header name.
+  if (!net::HttpUtil::IsValidHeaderName(method))
+    return JNI_FALSE;
+  request_adapter->set_method(method);
+  return JNI_TRUE;
+}
+
+static jboolean AddHeader(JNIEnv* env,
+                          jobject jurl_request,
+                          jlong jurl_request_adapter,
+                          jstring jname,
+                          jstring jvalue) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(!request_adapter->IsOnNetworkThread());
+  std::string name(base::android::ConvertJavaStringToUTF8(env, jname));
+  std::string value(base::android::ConvertJavaStringToUTF8(env, jvalue));
+  if (!net::HttpUtil::IsValidHeaderName(name) ||
+      !net::HttpUtil::IsValidHeaderValue(value)) {
+    return JNI_FALSE;
+  }
+  request_adapter->AddRequestHeader(name, value);
+  return JNI_TRUE;
+}
+
+static void Start(JNIEnv* env,
+                  jobject jurl_request,
+                  jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(!request_adapter->IsOnNetworkThread());
+  request_adapter->PostTaskToNetworkThread(
+      FROM_HERE,
+      base::Bind(&CronetURLRequestAdapter::Start,
+                 base::Unretained(request_adapter)));
+}
+
+static void DestroyRequestAdapter(JNIEnv* env,
+                                  jobject jurl_request,
+                                  jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(!request_adapter->IsOnNetworkThread());
+  request_adapter->PostTaskToNetworkThread(
+      FROM_HERE,
+      base::Bind(&CronetURLRequestAdapter::Destroy,
+                 base::Unretained(request_adapter)));

[mmenke, 2014/11/07 15:19:08]

I guess there's no standard templated delete task, and we can't use DeleteSoon
here, since we don't have access to the task runner?

[mef, 2014/11/07 16:22:25]

Yeah, I had a choice between custom 'Destroy' explicitly posted on network
thread or custom 'DestroySoon' implicitly posting DeleteSoon from arbitrary
thread. This seemed a little cleaner.

+}
+
+static void ReceiveData(JNIEnv* env,
+                        jobject jcaller,
+                        jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(!request_adapter->IsOnNetworkThread());
+  request_adapter->PostTaskToNetworkThread(
+      FROM_HERE,
+      base::Bind(&CronetURLRequestAdapter::ReadData,
+                 base::Unretained(request_adapter)));
+}
+
+static void FollowDeferredRedirect(JNIEnv* env,
+                                   jobject jcaller,
+                                   jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(!request_adapter->IsOnNetworkThread());
+  request_adapter->PostTaskToNetworkThread(
+      FROM_HERE,
+      base::Bind(&CronetURLRequestAdapter::FollowDeferredRedirect,
+                 base::Unretained(request_adapter)));
+}
+
+static void PopulateResponseHeaders(JNIEnv* env,
+                                    jobject jurl_request,
+                                    jlong jurl_request_adapter,
+                                    jobject jheaders_map) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(request_adapter->IsOnNetworkThread());
+
+  const net::HttpResponseHeaders* headers =
+      request_adapter->GetResponseHeaders();
+  if (headers == nullptr)
+    return;
+
+  void* iter = nullptr;
+  std::string header_name;
+  std::string header_value;
+  while (headers->EnumerateHeaderLines(&iter, &header_name, &header_value)) {
+    ScopedJavaLocalRef<jstring> name =
+        ConvertUTF8ToJavaString(env, header_name);
+    ScopedJavaLocalRef<jstring> value =
+        ConvertUTF8ToJavaString(env, header_value);
+    Java_CronetUrlRequest_onAppendResponseHeader(
+        env, jurl_request, jheaders_map, name.obj(), value.obj());
+  }
+
+  // Some implementations (notably HttpURLConnection) include a mapping for the
+  // null key; in HTTP's case, this maps to the HTTP status line.
+  ScopedJavaLocalRef<jstring> status_line =
+      ConvertUTF8ToJavaString(env, headers->GetStatusLine());
+  Java_CronetUrlRequest_onAppendResponseHeader(
+      env, jurl_request, jheaders_map, nullptr, status_line.obj());

[mmenke, 2014/11/07 15:19:08]

You said you're removed this, but didn't.  Do you disagree with me about keeping
this, or did you forget to remove it?

[mef, 2014/11/07 16:22:25]

Done.

+}
+
+static jstring GetNegotiatedProtocol(JNIEnv* env,
+                                     jobject jurl_request,
+                                     jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(request_adapter->IsOnNetworkThread());
+  return ConvertUTF8ToJavaString(env,
+      request_adapter->GetNegotiatedProtocol()).Release();

[mmenke, 2014/11/07 15:19:08]

Fix formatting.  Just moving env onto the second line of the statement would be
fine.

[mef, 2014/11/07 16:22:25]

Done.

+}
+
+static jboolean GetWasCached(JNIEnv* env,
+                             jobject jurl_request,
+                             jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(request_adapter->IsOnNetworkThread());
+  return request_adapter->GetWasCached() ? JNI_TRUE : JNI_FALSE;
+}
+
+static jlong GetTotalReceivedBytes(JNIEnv* env,
+                                   jobject jurl_request,
+                                   jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(request_adapter->IsOnNetworkThread());
+  return request_adapter->GetTotalReceivedBytes();
+}
+
+}  // namespace cronet