[Cronet] Remove JSON serialization of CronetEngine.Builder

components/cronet/android/cronet_url_request_context_adapter.cc

 // 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_context_adapter.h"
 
 #include <map>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
@@ skipping 14 matching lines @@
 #include "base/time/time.h"
 #include "base/values.h"
 #include "components/cronet/histogram_manager.h"
 #include "components/cronet/url_request_context_config.h"
 #include "jni/CronetUrlRequestContext_jni.h"
 #include "net/base/external_estimate_provider.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/base/network_delegate_impl.h"
+#include "net/cert/cert_verifier.h"
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_server_properties_manager.h"
 #include "net/log/write_to_file_net_log_observer.h"
 #include "net/proxy/proxy_config_service_android.h"
 #include "net/proxy/proxy_service.h"
 #include "net/sdch/sdch_owner.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
 #include "net/url_request/url_request_interceptor.h"
 
@@ skipping 332 matching lines @@
           net::AlternateProtocol::QUIC, "",
           static_cast<uint16>(quic_hint.alternate_port));
       context_->http_server_properties()->SetAlternativeService(
           quic_hint_host_port_pair, alternative_service, 1.0f,
           base::Time::Max());
     }
   }
 
   // Iterate through PKP configuration for every host.
   for (const auto& pkp : config->pkp_list) {
-    // Convert the vector of hash strings from the config to
-    // a vector of HashValue objects.
-    net::HashValueVector hash_value_vector;
-    for (const auto& hash : pkp->pin_hashes) {
-      net::HashValue hash_value;
-      bool good_hash = hash_value.FromString(*hash);
-      if (good_hash) {
-        hash_value_vector.push_back(hash_value);
-      } else {
-        LOG(WARNING) << "Unable to add hash value " << *hash;
-      }
-    }
-
     // Add the host pinning.
     context_->transport_security_state()->AddHPKP(
         pkp->host, pkp->expiration_date, pkp->include_subdomains,
-        hash_value_vector, GURL::EmptyGURL());
+        pkp->pin_hashes, GURL::EmptyGURL());
   }
 
   JNIEnv* env = base::android::AttachCurrentThread();
   jcronet_url_request_context_.Reset(env, jcronet_url_request_context.obj());
   Java_CronetUrlRequestContext_initNetworkThread(
       env, jcronet_url_request_context.obj());
 
 #if defined(DATA_REDUCTION_PROXY_SUPPORT)
   if (data_reduction_proxy_)
     data_reduction_proxy_->Init(true, GetURLRequestContext());
@@ skipping 127 matching lines @@
 void CronetURLRequestContextAdapter::OnThroughputObservation(
     int32_t throughput_kbps,
     const base::TimeTicks& timestamp,
     net::NetworkQualityEstimator::ObservationSource source) {
   Java_CronetUrlRequestContext_onThroughputObservation(
       base::android::AttachCurrentThread(), jcronet_url_request_context_.obj(),
       throughput_kbps,
       (timestamp - base::TimeTicks::UnixEpoch()).InMilliseconds(), source);
 }
 
+// Create a URLRequestContextConfig from the given parameters.
+static jlong CreateRequestContextConfig(
+    JNIEnv* env,
+    const JavaParamRef<jclass>& jcaller,
+    const JavaParamRef<jstring>& juser_agent,
+    const JavaParamRef<jstring>& jstorage_path,
+    jboolean jquic_enabled,
+    jboolean jhttp2_enabled,
+    jboolean jsdch_enabled,
+    const JavaParamRef<jstring>& jdata_reduction_proxy_key,
+    const JavaParamRef<jstring>& jdata_reduction_proxy_primary_proxy,
+    const JavaParamRef<jstring>& jdata_reduction_proxy_fallback_proxy,
+    const JavaParamRef<jstring>& jdata_reduction_proxy_secure_proxy_check_url,
+    jboolean jdisable_cache,
+    jint jhttp_cache_mode,
+    jlong jhttp_cache_max_size,
+    const JavaParamRef<jstring>& jexperimental_quic_connection_options,
+    jlong jmock_cert_verifier) {
+  return reinterpret_cast<jlong>(new URLRequestContextConfig(
+      jquic_enabled, jhttp2_enabled, jsdch_enabled,
+      static_cast<URLRequestContextConfig::HttpCacheType>(jhttp_cache_mode),
+      jhttp_cache_max_size, jdisable_cache,
+      base::android::ConvertJavaStringToUTF8(env, jstorage_path),
+      base::android::ConvertJavaStringToUTF8(env, juser_agent),
+      base::android::ConvertJavaStringToUTF8(
+          env, jexperimental_quic_connection_options),
+      base::android::ConvertJavaStringToUTF8(env, jdata_reduction_proxy_key),
+      base::android::ConvertJavaStringToUTF8(
+          env, jdata_reduction_proxy_primary_proxy),
+      base::android::ConvertJavaStringToUTF8(
+          env, jdata_reduction_proxy_fallback_proxy),
+      base::android::ConvertJavaStringToUTF8(
+          env, jdata_reduction_proxy_secure_proxy_check_url),
+      make_scoped_ptr(
+          reinterpret_cast<net::CertVerifier*>(jmock_cert_verifier))));
+}
+
+// Add a QUIC hint to a URLRequestContextConfig.
+static void AddQuicHint(JNIEnv* env,
+                        const JavaParamRef<jclass>& jcaller,
+                        jlong jurl_request_context_config,
+                        const JavaParamRef<jstring>& jhost,
+                        jint jport,
+                        jint jalternate_port) {
+  URLRequestContextConfig* config =
+      reinterpret_cast<URLRequestContextConfig*>(jurl_request_context_config);
+  config->quic_hints.push_back(
+      make_scoped_ptr(new URLRequestContextConfig::QuicHint(
+          base::android::ConvertJavaStringToUTF8(env, jhost), jport,
+          jalternate_port)));
+}
+
+// Add a public key ping to URLRequestContextConfig.
+static void AddPkp(JNIEnv* env,
+                   const JavaParamRef<jclass>& jcaller,
+                   jlong jurl_request_context_config,
+                   const JavaParamRef<jstring>& jhost,
+                   const JavaParamRef<jobjectArray>& jhashes,
+                   jboolean jinclude_subdomains,
+                   jlong jexpiration_time) {

[xunjieli, 2015/12/04 19:19:13]

Maybe add a comment here documenting what |jexpiration_time| is. Something like
number of milliseconds since the epoch.

[pauljensen, 2015/12/07 13:53:34]

Done.

+  URLRequestContextConfig* config =
+      reinterpret_cast<URLRequestContextConfig*>(jurl_request_context_config);
+  scoped_ptr<URLRequestContextConfig::Pkp> pkp(new URLRequestContextConfig::Pkp(
+      base::android::ConvertJavaStringToUTF8(env, jhost), jinclude_subdomains,
+      base::Time::UnixEpoch() +
+          base::TimeDelta::FromMilliseconds(jexpiration_time)));
+  size_t hash_count = env->GetArrayLength(jhashes);
+  for (size_t i = 0; i < hash_count; ++i) {
+    ScopedJavaLocalRef<jbyteArray> bytes_array(
+        env, static_cast<jbyteArray>(env->GetObjectArrayElement(jhashes, i)));
+    if (env->GetArrayLength(bytes_array.obj()) != sizeof(net::SHA256HashValue))

[xunjieli, 2015/12/04 19:19:13]

Does this actually work? The size of the struct includes all of its members
right?

Should it be:
net::SHA256HashValue hash_value;

if (env->GetArrayLength(bytes_array.obj()) != sizeof(hash_value.data))

[pauljensen, 2015/12/07 13:53:34]

Yes, this works.  In fact SHA256HashValue exists to be a POD representation of
the hash value, see paragraph two here:
https://code.google.com/p/chromium/issues/detail?id=551067#c2
I've added a static_assert to enforce SHA256HashValue is POD and contains no
overhead.

+      continue;
+    jbyte* bytes = env->GetByteArrayElements(bytes_array.obj(), nullptr);
+    net::HashValue hash(*reinterpret_cast<net::SHA256HashValue*>(bytes));

[xunjieli, 2015/12/04 19:19:13]

SHA256HashValue is a struct. Does the reinterpret_cast actually work?

Should we allocate a SHA256HashValue on the stack, and do a memcpy of the bytes
as unint_8/unsigned char to the SHA256HashValue's data field?

[pauljensen, 2015/12/07 13:53:34]

Yes, this works.  I really want to avoid memcpy as it's very type-unsafe and add
a useless copy.  I've added a static_assert to enforce SHA256HashValue is POD
and contains no overhead.

+    pkp->pin_hashes.push_back(hash);
+    env->ReleaseByteArrayElements(bytes_array.obj(), bytes, JNI_ABORT);
+  }
+  config->pkp_list.push_back(std::move(pkp));
+}
+
 // Creates RequestContextAdater if config is valid URLRequestContextConfig,
 // returns 0 otherwise.
 static jlong CreateRequestContextAdapter(JNIEnv* env,
                                          const JavaParamRef<jclass>& jcaller,
-                                         const JavaParamRef<jstring>& jconfig) {
-  std::string config_string =
-      base::android::ConvertJavaStringToUTF8(env, jconfig);
+                                         jlong jconfig) {
   scoped_ptr<URLRequestContextConfig> context_config(
-      new URLRequestContextConfig());
-  if (!context_config->LoadFromJSON(config_string))
-    return 0;
+      reinterpret_cast<URLRequestContextConfig*>(jconfig));
 
   CronetURLRequestContextAdapter* context_adapter =
       new CronetURLRequestContextAdapter(context_config.Pass());
   return reinterpret_cast<jlong>(context_adapter);
 }
 
 static jint SetMinLogLevel(JNIEnv* env,
                            const JavaParamRef<jclass>& jcaller,
                            jint jlog_level) {
   jint old_log_level = static_cast<jint>(logging::GetMinLogLevel());
   // MinLogLevel is global, shared by all URLRequestContexts.
   logging::SetMinLogLevel(static_cast<int>(jlog_level));
   return old_log_level;
 }
 
 static ScopedJavaLocalRef<jbyteArray> GetHistogramDeltas(
     JNIEnv* env,
     const JavaParamRef<jclass>& jcaller) {
   base::StatisticsRecorder::Initialize();
   std::vector<uint8> data;
   if (!HistogramManager::GetInstance()->GetDeltas(&data))
     return ScopedJavaLocalRef<jbyteArray>();
   return base::android::ToJavaByteArray(env, &data[0], data.size());
 }
 
 }  // namespace cronet