Add client cert support to android_webview

android_webview/native/aw_contents_client_bridge_unittest.cc

Index: android_webview/native/aw_contents_client_bridge_unittest.cc
diff --git a/android_webview/native/aw_contents_client_bridge_unittest.cc b/android_webview/native/aw_contents_client_bridge_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b7fbc011918bf99ef4669a6b29145b3da45c325b
--- /dev/null
+++ b/android_webview/native/aw_contents_client_bridge_unittest.cc
@@ -0,0 +1,170 @@
+// 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 "android_webview/native/aw_contents_client_bridge.h"
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_array.h"
+#include "base/android/scoped_java_ref.h"
+#include "base/bind.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "jni/MockAwContentsClientBridge_jni.h"
+#include "net/android/net_jni_registrar.h"
+#include "net/ssl/ssl_cert_request_info.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+

[boliu, 2014/04/16 02:57:46]

unittests are usually put into its own anonymous namespace

[sgurun-gerrit only, 2014/04/17 15:10:51]

Done.

[boliu, 2014/04/18 17:10:12]

Actually wrap the whole file in android_webview, then anonymous namespace. We
are not being very consistent here, so feel free to fix all our other unit test
files in a separate CL.

[sgurun-gerrit only, 2014/04/19 01:28:49]

That's something very loosely applied it seems -just checked 8 random unittest
files and only two of them has it. anyway, it makes sense to have the unit test
in the namespace, don't know why we are more strict about it.

On 2014/04/18 17:10:12, boliu wrote:

+using android_webview::AwContentsClientBridge;
+using base::android::AttachCurrentThread;
+using base::android::ScopedJavaLocalRef;
+using net::SSLCertRequestInfo;
+using net::SSLClientCertType;
+using net::X509Certificate;
+using testing::NotNull;
+using testing::Test;
+
+// Tests the android_webview contents client bridge.
+class AwContentsClientBridgeTest : public Test {
+ public:
+  typedef AwContentsClientBridge::SelectCertificateCallback
+      SelectCertificateCallback;
+
+  AwContentsClientBridgeTest() { }
+
+  // Callback method called when a cert is selected.
+  void CertSelected(X509Certificate* cert);
+ protected:
+  virtual void SetUp();
+  void TestCertType(SSLClientCertType type, const std::string& expected_name);
+  // Create the TestBrowserThreads. Just instantiate the member variable.
+  content::TestBrowserThreadBundle thread_bundle_;
+  base::android::ScopedJavaGlobalRef<jobject> jbridge_;
+  scoped_ptr<AwContentsClientBridge> bridge_;
+  scoped_refptr<SSLCertRequestInfo> cert_request_info_;
+  X509Certificate* selected_cert_;
+  int cert_selected_callbacks_;
+  JNIEnv* env_;
+};
+
+void AwContentsClientBridgeTest::SetUp() {
+  env_ = AttachCurrentThread();
+  ASSERT_THAT(env_, NotNull());
+  ASSERT_TRUE(android_webview::RegisterAwContentsClientBridge(env_));
+  ASSERT_TRUE(RegisterNativesImpl(env_));
+  ASSERT_TRUE(net::android::RegisterJni(env_));
+  jbridge_.Reset(env_,
+      Java_MockAwContentsClientBridge_getAwContentsClientBridge(env_).obj());
+  bridge_.reset(new AwContentsClientBridge(env_, jbridge_.obj()));
+  selected_cert_ = NULL;
+  cert_selected_callbacks_ = 0;
+  cert_request_info_ = new net::SSLCertRequestInfo;
+}
+
+void AwContentsClientBridgeTest::CertSelected(X509Certificate* cert) {
+  selected_cert_ = cert;
+  cert_selected_callbacks_++;
+}
+
+TEST_F(AwContentsClientBridgeTest,
+    TestClientCertCallbackCalledWhenJavaDoesNotTakeOwnership) {
+  Java_MockAwContentsClientBridge_setEarlyOutSelectClientCertificate(
+      env_,
+      jbridge_.obj(),
+      true);
+  bridge_->SelectClientCertificate(
+      cert_request_info_.get(),
+      base::Bind(
+          &AwContentsClientBridgeTest::CertSelected,
+          base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(NULL, selected_cert_);
+  EXPECT_EQ(1, cert_selected_callbacks_);
+}
+
+// Verify that if java takes ownership, SelectClientCertificate() does not
+// call the callback.
+TEST_F(AwContentsClientBridgeTest,
+    TestClientCertCallbackNotCalledWhenJavaTakesOwnership) {
+  bridge_->SelectClientCertificate(
+      cert_request_info_.get(),
+      base::Bind(
+          &AwContentsClientBridgeTest::CertSelected,
+          base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(NULL, selected_cert_);
+  EXPECT_EQ(0, cert_selected_callbacks_);

[boliu, 2014/04/16 02:57:46]

Does the native object leak here?

Maybe the mock can keep going and actually call the callback.

[sgurun-gerrit only, 2014/04/18 01:44:55]

This does not apply anymore.
On 2014/04/16 02:57:46, boliu wrote:

+}
+
+TEST_F(AwContentsClientBridgeTest, TestClientCertKeyTypesCorrectlyEncoded) {
+  SSLClientCertType cert_types[3] = {net::CLIENT_CERT_RSA_SIGN,
+      net::CLIENT_CERT_DSS_SIGN, net::CLIENT_CERT_ECDSA_SIGN};
+  std::string expected_names[3] = {"RSA", "DSA" ,"ECDSA"};
+
+  for(int i = 0; i<3; i++) {

[boliu, 2014/04/16 02:57:46]

nit: space add space around "<"

[sgurun-gerrit only, 2014/04/17 15:10:51]

Done.

+    TestCertType(cert_types[i], expected_names[i]);
+  }
+}
+
+void AwContentsClientBridgeTest::TestCertType(SSLClientCertType type,
+      const std::string& expected_name) {
+  cert_request_info_->cert_key_types.clear();
+  cert_request_info_->cert_key_types.push_back(type);
+  bridge_->SelectClientCertificate(
+      cert_request_info_.get(),
+      base::Bind(
+          &AwContentsClientBridgeTest::CertSelected,
+          base::Unretained(static_cast<AwContentsClientBridgeTest*>(this))));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(0, cert_selected_callbacks_);
+  ScopedJavaLocalRef<jobjectArray> key_types =
+      Java_MockAwContentsClientBridge_getKeyTypes(env_, jbridge_.obj());
+  std::vector<std::string> vec;
+  base::android::AppendJavaStringArrayToStringVector(env_,
+                                                     key_types.obj(),
+                                                     &vec);
+  EXPECT_EQ(1u, vec.size());
+  EXPECT_EQ(expected_name, vec[0]);
+}
+
+// Verify that ProvideClientCertificateResponse calls the callback with NULL
+// parameters when encodedchain is not provided.
+TEST_F(AwContentsClientBridgeTest,
+    TestProvideClientCertificateResponseCallsCallbackOnNullKey) {
+  SelectCertificateCallback callback = base::Bind(
+      &AwContentsClientBridgeTest::CertSelected,
+      base::Unretained(static_cast<AwContentsClientBridgeTest*>(this)));
+  // Create a copy of the callback on the heap. We will test if
+  // ProvideClientCertificateResponse manages ownership properly.
+  SelectCertificateCallback* temp = new SelectCertificateCallback(callback);
+  jlong request_id = reinterpret_cast<intptr_t>(temp);
+  jclass byteClass = env_->FindClass("java/lang/Byte");
+  jobjectArray array = env_->NewObjectArray(1, byteClass, 0);

[boliu, 2014/04/16 02:57:46]

If mock uses ClientCertificateRequestCallback directly, can we continue the test
above to cover these two cases (instead of doing crazy jni here)

[sgurun-gerrit only, 2014/04/18 01:44:55]

not doing crazy jni anymore.
On 2014/04/16 02:57:46, boliu wrote:

+  bridge_->ProvideClientCertificateResponse(env_, jbridge_.obj(), request_id,
+    array, NULL);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(NULL, selected_cert_);
+  EXPECT_EQ(1, cert_selected_callbacks_);
+}
+
+// Verify that ProvideClientCertificateResponse calls the callback with
+// NULL parameters when private key is not provided.
+TEST_F(AwContentsClientBridgeTest,
+    TestProvideClientCertificateResponseCallsCallbackOnNullChain) {
+  SelectCertificateCallback callback = base::Bind(
+      &AwContentsClientBridgeTest::CertSelected,
+      base::Unretained(static_cast<AwContentsClientBridgeTest*>(this)));
+  // Create a copy of the callback on the heap. We will test if
+  // ProvideClientCertificateResponse manages ownership properly.
+  SelectCertificateCallback* temp = new SelectCertificateCallback(callback);
+  jlong request_id = reinterpret_cast<intptr_t>(temp);
+  ScopedJavaLocalRef<jobject> privateKey =
+      Java_MockAwContentsClientBridge_getTestPrivateKey(env_, jbridge_.obj());
+  bridge_->ProvideClientCertificateResponse(env_, jbridge_.obj(), request_id,
+    NULL, privateKey.obj());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(NULL, selected_cert_);
+  EXPECT_EQ(1, cert_selected_callbacks_);
+}