Remove ENGINE indirection from Android SSLPrivateKey.

android_webview/native/aw_contents_client_bridge.cc

 // Copyright (c) 2013 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 "android_webview/native/aw_contents_client_bridge.h"
 
+#include <utility>
+
 #include "android_webview/common/devtools_instrumentation.h"
 #include "android_webview/native/aw_contents.h"
 #include "base/android/jni_android.h"
 #include "base/android/jni_array.h"
 #include "base/android/jni_string.h"
 #include "base/callback_helpers.h"
 #include "base/macros.h"
+#include "base/memory/ref_counted.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/client_certificate_delegate.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "crypto/scoped_openssl_types.h"
 #include "grit/components_strings.h"
 #include "jni/AwContentsClientBridge_jni.h"
-#include "net/android/keystore_openssl.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/openssl_client_key_store.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_client_cert_type.h"
+#include "net/ssl/ssl_platform_key_android.h"
+#include "net/ssl/ssl_private_key.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
 using base::android::AttachCurrentThread;
 using base::android::ConvertJavaStringToUTF16;
 using base::android::ConvertUTF8ToJavaString;
 using base::android::ConvertUTF16ToJavaString;
 using base::android::HasException;
 using base::android::JavaRef;
 using base::android::ScopedJavaLocalRef;
 using content::BrowserThread;
 
 namespace android_webview {
 
 namespace {
 
 // Must be called on the I/O thread to record a client certificate
 // and its private key in the OpenSSLClientKeyStore.
-void RecordClientCertificateKey(
-    const scoped_refptr<net::X509Certificate>& client_cert,
-    crypto::ScopedEVP_PKEY private_key) {
+void RecordClientCertificateKey(net::X509Certificate* client_cert,
+                                scoped_refptr<net::SSLPrivateKey> private_key) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   net::OpenSSLClientKeyStore::GetInstance()->RecordClientCertPrivateKey(
-      client_cert.get(), private_key.get());
+      client_cert, std::move(private_key));
 }
 
 }  // namespace
 
 AwContentsClientBridge::AwContentsClientBridge(JNIEnv* env, jobject obj)
     : java_ref_(env, obj) {
   DCHECK(obj);
   Java_AwContentsClientBridge_setNativeContentsClientBridge(
       env, obj, reinterpret_cast<intptr_t>(this));
 }
@@ skipping 169 matching lines @@
     encoded_chain.push_back(encoded_chain_strings[i]);
 
   // Create the X509Certificate object from the encoded chain.
   scoped_refptr<net::X509Certificate> client_cert(
       net::X509Certificate::CreateFromDERCertChain(encoded_chain));
   if (!client_cert.get()) {
     LOG(ERROR) << "Could not decode client certificate chain";
     return;
   }
 
-  // Create an EVP_PKEY wrapper for the private key JNI reference.
-  crypto::ScopedEVP_PKEY private_key(
-      net::android::GetOpenSSLPrivateKeyWrapper(private_key_ref.obj()));
-  if (!private_key.get()) {
+  // Create an SSLPrivateKey wrapper for the private key JNI reference.
+  scoped_refptr<net::SSLPrivateKey> private_key =
+      net::WrapJavaPrivateKey(private_key_ref.obj());
+  if (!private_key) {
     LOG(ERROR) << "Could not create OpenSSL wrapper for private key";
     return;
   }
 
   // Release the guard and |pending_client_cert_request_delegates_| references
   // to |delegate|.
   pending_client_cert_request_delegates_.Remove(request_id);
   ignore_result(guard.Release());
 
   // RecordClientCertificateKey() must be called on the I/O thread,
   // before the delegate is called with the selected certificate on
   // the UI thread.
   content::BrowserThread::PostTaskAndReply(
       content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&RecordClientCertificateKey, client_cert,
+      base::Bind(&RecordClientCertificateKey, base::RetainedRef(client_cert),

[boliu, 2016/09/01 20:22:30]

is RetainedRef necessary here? (/me seeing it for the first time and read the
docs for it just now..)

[davidben, 2016/09/01 20:28:01]

It's possible I'm misunderstanding, but I think it's a recent-ish change for
this stuff:
https://crbug.com/589048

The change here is:
1. I believe we no longer like const scoped_refpr<T>& and prefer either T* if
not taking ownership or plain scoped_refptr<T> if you are. Since the
RecordClientCertificateKey didn't take ownership, I switched it to T*.

Per that bug, I *think* the intent is that they explicitly have a
base:RetainedRef is the base::Bind is to take ownership and then unwrap it when
passing to a function.

But it's possible I'm wrong here. Do you agree with this interpretation?

[boliu, 2016/09/01 20:35:53]

I *think* RetainedRef just does the second part of unwrapping before the
function call. Retaining a ref to a refcounted object in callbacks is the
default behavior (ie with no wrappers).
Ahh, oops, I missed this is the *first* parameter to RecordClientCertificateKey,
not the second :p

                  base::Passed(&private_key)),
       base::Bind(&content::ClientCertificateDelegate::ContinueWithCertificate,
                  base::Owned(delegate), base::RetainedRef(client_cert)));
 }
 
 void AwContentsClientBridge::RunJavaScriptDialog(
     content::JavaScriptMessageType message_type,
     const GURL& origin_url,
     const base::string16& message_text,
     const base::string16& default_prompt_text,
@@ skipping 132 matching lines @@
   pending_client_cert_request_delegates_.Remove(request_id);
 
   delete delegate;
 }
 
 bool RegisterAwContentsClientBridge(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 }  // namespace android_webview