Use "@NativeClassQualifiedName" in CronetUrlRequest.java

components/cronet/android/cronet_url_request_adapter.cc

Index: components/cronet/android/cronet_url_request_adapter.cc
diff --git a/components/cronet/android/cronet_url_request_adapter.cc b/components/cronet/android/cronet_url_request_adapter.cc
index ff36601caf397ea73e79e892620c55fee18fe6a3..7a43c776bea2abe7940b34c47d3b58e5c4747b5c 100644
--- a/components/cronet/android/cronet_url_request_adapter.cc
+++ b/components/cronet/android/cronet_url_request_adapter.cc
@@ -4,69 +4,124 @@
 
 #include "cronet_url_request_adapter.h"
 
+#include <jni.h>

[mmenke, 2015/04/02 21:07:05]

nit:  Not needed (included in header file)

[xunjieli, 2015/04/02 21:50:13]

Done.

+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"

[mmenke, 2015/04/02 21:07:05]

nit:  These two are used in the header

[xunjieli, 2015/04/02 21:50:13]

Done.

 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/macros.h"

[mmenke, 2015/04/02 21:07:05]

nit:  in header

[xunjieli, 2015/04/02 21:50:13]

Done.

 #include "components/cronet/android/cronet_url_request_context_adapter.h"
-#include "components/cronet/android/wrapped_channel_upload_element_reader.h"
+#include "jni/CronetUrlRequest_jni.h"
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
+#include "net/base/net_errors.h"
+#include "net/base/request_priority.h"
+#include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
+#include "net/http/http_util.h"
 #include "net/url_request/redirect_info.h"
 #include "net/url_request/url_request_context.h"
 
+using base::android::ConvertUTF8ToJavaString;
+
 namespace cronet {
 
+namespace {
+
 static const int kReadBufferSize = 32768;
 
+}  // namespace

[mmenke, 2015/04/02 21:07:05]

nit:  Either need static or anonymous namespace, not both.  For just a couple
constants, static may be cleaner.

[xunjieli, 2015/04/02 21:50:12]

Done.

+
+// Explicitly register static JNI functions.
+bool CronetUrlRequestAdapterRegisterJni(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();
+
+  CronetURLRequestAdapter* adapter =
+      new CronetURLRequestAdapter(context_adapter, env, jurl_request, url,
+                                  static_cast<net::RequestPriority>(jpriority));
+
+  return reinterpret_cast<jlong>(adapter);
+}
+
 CronetURLRequestAdapter::CronetURLRequestAdapter(
     CronetURLRequestContextAdapter* context,
-    scoped_ptr<CronetURLRequestAdapterDelegate> delegate,
+    JNIEnv* env,
+    jobject jurl_request,
     const GURL& url,
     net::RequestPriority priority)
     : context_(context),
-      delegate_(delegate.Pass()),
       initial_url_(url),
       initial_priority_(priority),
       initial_method_("GET"),
       load_flags_(context->default_load_flags()) {
+  owner_.Reset(env, jurl_request);

[mmenke, 2015/04/02 21:07:05]

DCHECK(!context_->IsOnNetworkThread());?

[xunjieli, 2015/04/02 21:50:13]

Done.

 }
 
 CronetURLRequestAdapter::~CronetURLRequestAdapter() {
-  DCHECK(IsOnNetworkThread());
-}
-
-void CronetURLRequestAdapter::AddRequestHeader(const std::string& name,
-                                               const std::string& value) {
-  DCHECK(!IsOnNetworkThread());
-  initial_request_headers_.SetHeader(name, value);
-}
-
-void CronetURLRequestAdapter::DisableCache() {
-  DCHECK(!IsOnNetworkThread());
-  load_flags_ |= net::LOAD_DISABLE_CACHE;
+  DCHECK(context_->IsOnNetworkThread());
 }
 
-void CronetURLRequestAdapter::PostTaskToNetworkThread(
-    const tracked_objects::Location& from_here,
-    const base::Closure& task) {
-  DCHECK(!IsOnNetworkThread());
-  context_->PostTaskToNetworkThread(from_here, task);
+jboolean CronetURLRequestAdapter::SetHttpMethod(JNIEnv* env,
+                                                jobject jcaller,
+                                                jstring jmethod) {
+  DCHECK(!context_->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;
+  initial_method_ = method;
+  return JNI_TRUE;
 }
 
-bool CronetURLRequestAdapter::IsOnNetworkThread() const {
-  return context_->IsOnNetworkThread();
+jboolean CronetURLRequestAdapter::AddRequestHeader(JNIEnv* env,
+                                                   jobject jcaller,
+                                                   jstring jname,
+                                                   jstring jvalue) {
+  DCHECK(!context_->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;
+  }
+  initial_request_headers_.SetHeader(name, value);
+  return JNI_TRUE;
 }
 
 void CronetURLRequestAdapter::SetUpload(
     scoped_ptr<net::UploadDataStream> upload) {
-  DCHECK(!IsOnNetworkThread());
+  DCHECK(!context_->IsOnNetworkThread());
   DCHECK(!upload_);
   upload_ = upload.Pass();
 }
 
-void CronetURLRequestAdapter::Start() {
-  DCHECK(IsOnNetworkThread());
+void CronetURLRequestAdapter::Start(JNIEnv* env, jobject jcaller) {
+  DCHECK(!context_->IsOnNetworkThread());
+  context_->PostTaskToNetworkThread(
+      FROM_HERE, base::Bind(&CronetURLRequestAdapter::StartOnNetworkThread,
+                            base::Unretained(this)));
+}
+
+void CronetURLRequestAdapter::StartOnNetworkThread() {

[mmenke, 2015/04/02 21:07:05]

For this, ReadDataOnNetworkThread, and DestroyOnNetworkThread, definition order
should match declaration order in the header.

[xunjieli, 2015/04/02 21:50:12]

Done.

+  DCHECK(context_->IsOnNetworkThread());
   VLOG(1) << "Starting chromium request: "
           << initial_url_.possibly_invalid_spec().c_str()
           << " priority: " << RequestPriorityToString(initial_priority_);
@@ -81,14 +136,15 @@ void CronetURLRequestAdapter::Start() {
   url_request_->Start();
 }
 
-void CronetURLRequestAdapter::FollowDeferredRedirect() {
-  DCHECK(IsOnNetworkThread());
-
-  url_request_->FollowDeferredRedirect();
+void CronetURLRequestAdapter::ReadData(JNIEnv* env, jobject jcaller) {
+  DCHECK(!context_->IsOnNetworkThread());
+  context_->PostTaskToNetworkThread(
+      FROM_HERE, base::Bind(&CronetURLRequestAdapter::ReadDataOnNetworkThread,
+                            base::Unretained(this)));
 }
 
-void CronetURLRequestAdapter::ReadData() {
-  DCHECK(IsOnNetworkThread());
+void CronetURLRequestAdapter::ReadDataOnNetworkThread() {
+  DCHECK(context_->IsOnNetworkThread());
   if (!read_buffer_.get())
     read_buffer_ = new net::IOBufferWithSize(kReadBufferSize);
 
@@ -101,29 +157,76 @@ void CronetURLRequestAdapter::ReadData() {
   OnReadCompleted(url_request_.get(), bytes_read);
 }
 
-void CronetURLRequestAdapter::Destroy() {
-  DCHECK(IsOnNetworkThread());
+void CronetURLRequestAdapter::Destroy(JNIEnv* env, jobject jcaller) {
+  DCHECK(!context_->IsOnNetworkThread());
+  context_->PostTaskToNetworkThread(
+      FROM_HERE, base::Bind(&CronetURLRequestAdapter::DestroyOnNetworkThread,
+                            base::Unretained(this)));
+}
+
+void CronetURLRequestAdapter::DestroyOnNetworkThread() {
+  DCHECK(context_->IsOnNetworkThread());
   delete this;
 }
 
-const net::HttpResponseHeaders*
-CronetURLRequestAdapter::GetResponseHeaders() const {
-  DCHECK(IsOnNetworkThread());
-  return url_request_->response_headers();
+void CronetURLRequestAdapter::PopulateResponseHeaders(JNIEnv* env,
+                                                      jobject jurl_request,
+                                                      jobject jheaders_list) {
+  DCHECK(context_->IsOnNetworkThread());
+  const net::HttpResponseHeaders* headers = url_request_->response_headers();
+  if (headers == nullptr)
+    return;
+
+  void* iter = nullptr;
+  std::string header_name;
+  std::string header_value;
+  while (headers->EnumerateHeaderLines(&iter, &header_name, &header_value)) {
+    base::android::ScopedJavaLocalRef<jstring> name =
+        ConvertUTF8ToJavaString(env, header_name);
+    base::android::ScopedJavaLocalRef<jstring> value =
+        ConvertUTF8ToJavaString(env, header_value);
+    Java_CronetUrlRequest_onAppendResponseHeader(
+        env, jurl_request, jheaders_list, name.obj(), value.obj());
+  }
+}
+
+void CronetURLRequestAdapter::FollowDeferredRedirect(JNIEnv* env,
+                                                     jobject jcaller) {
+  DCHECK(!context_->IsOnNetworkThread());
+  context_->PostTaskToNetworkThread(
+      FROM_HERE, base::Bind(&net::URLRequest::FollowDeferredRedirect,
+                            base::Unretained(url_request_.get())));

[mmenke, 2015/04/02 21:07:05]

While this works, I think it's a little ugly and more regression prone than
posting a task that runs a CronetURLRequestAdapter method that calls
FollowDeferredRedirect on the other thread.

[xunjieli, 2015/04/02 21:50:13]

Done.

+}
+
+void CronetURLRequestAdapter::DisableCache(JNIEnv* env, jobject jcaller) {
+  DCHECK(!context_->IsOnNetworkThread());
+  load_flags_ |= net::LOAD_DISABLE_CACHE;
+}
+
+base::android::ScopedJavaLocalRef<jstring>
+CronetURLRequestAdapter::GetHttpStatusText(JNIEnv* env, jobject jcaller) const {
+  DCHECK(context_->IsOnNetworkThread());
+  const net::HttpResponseHeaders* headers = url_request_->response_headers();
+  return ConvertUTF8ToJavaString(env, headers->GetStatusText());
 }
 
-const std::string& CronetURLRequestAdapter::GetNegotiatedProtocol() const {
-  DCHECK(IsOnNetworkThread());
-  return url_request_->response_info().npn_negotiated_protocol;
+base::android::ScopedJavaLocalRef<jstring>
+CronetURLRequestAdapter::GetNegotiatedProtocol(JNIEnv* env,
+                                               jobject jcaller) const {
+  DCHECK(context_->IsOnNetworkThread());
+  return ConvertUTF8ToJavaString(
+      env, url_request_->response_info().npn_negotiated_protocol);
 }
 
-bool CronetURLRequestAdapter::GetWasCached() const {
-  DCHECK(IsOnNetworkThread());
+jboolean CronetURLRequestAdapter::GetWasCached(JNIEnv* env,
+                                               jobject jcaller) const {
+  DCHECK(context_->IsOnNetworkThread());
   return url_request_->response_info().was_cached;
 }
 
-int64 CronetURLRequestAdapter::GetTotalReceivedBytes() const {
-  DCHECK(IsOnNetworkThread());
+int64 CronetURLRequestAdapter::GetTotalReceivedBytes(JNIEnv* env,
+                                                     jobject jcaller) const {
+  DCHECK(context_->IsOnNetworkThread());
   return url_request_->GetTotalReceivedBytes();
 }
 
@@ -133,29 +236,39 @@ void CronetURLRequestAdapter::OnReceivedRedirect(
     net::URLRequest* request,
     const net::RedirectInfo& redirect_info,
     bool* defer_redirect) {
-  DCHECK(IsOnNetworkThread());
+  DCHECK(context_->IsOnNetworkThread());
   DCHECK(request->status().is_success());
-  delegate_->OnRedirect(redirect_info.new_url, redirect_info.status_code);
+  JNIEnv* env = base::android::AttachCurrentThread();
+  cronet::Java_CronetUrlRequest_onRedirect(
+      env, owner_.obj(),
+      ConvertUTF8ToJavaString(env, redirect_info.new_url.spec()).obj(),
+      redirect_info.status_code);
   *defer_redirect = true;
 }
 
 void CronetURLRequestAdapter::OnResponseStarted(net::URLRequest* request) {
-  DCHECK(IsOnNetworkThread());
+  DCHECK(context_->IsOnNetworkThread());
   if (MaybeReportError(request))
     return;
-  delegate_->OnResponseStarted(request->GetResponseCode());
+  JNIEnv* env = base::android::AttachCurrentThread();
+  cronet::Java_CronetUrlRequest_onResponseStarted(env, owner_.obj(),
+                                                  request->GetResponseCode());
 }
 
 void CronetURLRequestAdapter::OnReadCompleted(net::URLRequest* request,
                                               int bytes_read) {
-  DCHECK(IsOnNetworkThread());
+  DCHECK(context_->IsOnNetworkThread());
   if (MaybeReportError(request))
     return;
   if (bytes_read != 0) {
-    delegate_->OnBytesRead(
-        reinterpret_cast<unsigned char*>(read_buffer_->data()), bytes_read);
+    JNIEnv* env = base::android::AttachCurrentThread();
+    base::android::ScopedJavaLocalRef<jobject> java_buffer(
+        env, env->NewDirectByteBuffer(read_buffer_->data(), bytes_read));
+    cronet::Java_CronetUrlRequest_onDataReceived(env, owner_.obj(),
+                                                 java_buffer.obj());
   } else {
-    delegate_->OnRequestFinished();
+    JNIEnv* env = base::android::AttachCurrentThread();
+    cronet::Java_CronetUrlRequest_onSucceeded(env, owner_.obj());
   }
 }
 
@@ -164,9 +277,13 @@ bool CronetURLRequestAdapter::MaybeReportError(net::URLRequest* request) const {
   DCHECK_EQ(request, url_request_);
   if (url_request_->status().is_success())
     return false;
-  VLOG(1) << "Error " << url_request_->status().error()
+  int net_error = url_request_->status().error();
+  VLOG(1) << "Error " << net::ErrorToString(net_error)
           << " on chromium request: " << initial_url_.possibly_invalid_spec();
-  delegate_->OnError(url_request_->status().error());
+  JNIEnv* env = base::android::AttachCurrentThread();
+  cronet::Java_CronetUrlRequest_onError(
+      env, owner_.obj(), net_error,
+      ConvertUTF8ToJavaString(env, net::ErrorToString(net_error)).obj());
   return true;
 }