Implement Webviewclient.onReceivedSslError

android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java

Index: android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
diff --git a/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java b/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
new file mode 100644
index 0000000000000000000000000000000000000000..df72102261439a799ae3a28b92e70819c407291f
--- /dev/null
+++ b/android_webview/java/src/org/chromium/android_webview/AwContentsClientBridge.java
@@ -0,0 +1,120 @@
+// 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.
+
+package org.chromium.android_webview;
+
+import android.net.http.SslCertificate;
+import android.net.http.SslError;
+import android.util.Log;
+import android.webkit.SslErrorHandler;
+
+import org.chromium.base.CalledByNative;
+import org.chromium.base.JNINamespace;
+import org.chromium.base.ThreadUtils;
+import org.chromium.net.X509Util;
+
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+
+/**
+ * This class bridges the native and the Java AwContentsClient classes. It routes the
+ * calls in between and it also manages the callbacks.
+ */
+@JNINamespace("android_webview")
+public class AwContentsClientBridge {

[joth, 2013/02/22 01:15:45]

I see this is fairly logic heavy, so maybe useful to have its own class, but our
normal approach would be to have AwContentsClient.java talk direct to
aw_contents_client.cc no need for another Bridge class in between.

Marcin - is this what you were thinking?


The logic below maybe able to extract into some SslCertificateHelper class...
which will avoid the need to have AwContents call method on AwContentsClient for
the der->cert conversion utility (which feels a bit of a random interconnection)

[sgurun-gerrit only, 2013/02/22 01:48:07]

I started as part of AwContensClient.java but then found it already large enough
so that I did not want to crowd it, had a quick chat with Marcin and decided to
go this way. We could move ssl stuff to its own class too.  However, I don't
think offloading the JNI communication part to a bridge class is a terribly bad
idea.


Agreed with the final comment.

On 2013/02/22 01:15:45, joth wrote:

+    private static final String TAG = AwContentsClientBridge.class.getSimpleName();
+
+    private AwContentsClient mClient;
+    // The native side of this object.
+    private int mNativeAwContentsClientBridge;
+
+    public AwContentsClientBridge(AwContentsClient client, int nativeAwContents) {
+        assert nativeAwContents != 0;
+        mNativeAwContentsClientBridge = nativeInit(nativeAwContents);

[joth, 2013/02/22 02:39:44]

suggest doing the native construction in AwContents (.cc) to keep it symmetric
(i.e. create own and destroy all from same class, use scoped_ptr)

[sgurun-gerrit only, 2013/02/22 19:31:31]

Done.

+        mClient = client;
+        assert mNativeAwContentsClientBridge != 0;
+    }
+
+    public void destroy() {
+        mNativeAwContentsClientBridge = 0;

[joth, 2013/02/22 01:15:45]

I'd expect to see nativeDestroy(mNativeAwContentsClientBridge) here... the java
object owns the native side?

[sgurun-gerrit only, 2013/02/22 01:48:07]

It is owned by AwContents, since we utilize it to access this class. We can come
up with a different way if you want to.

On 2013/02/22 01:15:45, joth wrote:

[joth, 2013/02/22 02:39:44]

Gotcha. So lets initiate the mNativeAwContentsClientBridge = 0; from a
CalledByNative method, invoked from the native side's destructor. That way it's
sure to be zeroed however the native side dies.

[sgurun-gerrit only, 2013/02/22 19:31:31]

Done.

+        mClient = null;
+    }
+
+    // If returns false, the request is immediately canceled, and any call to proceedSslError
+    // has no effect. If returns true, the request should be canceled or proceeded using
+    // proceedSslError().
+    // Unlike the webview classic, we do not keep keep a database of certificates that
+    // are allowed by the user, because this functionality is already handled via
+    // ssl_policy in native layers.
+    @CalledByNative
+    private boolean allowCertificateError(int certError, byte[] derBytes, final String url,
+            final int id) {
+        final SslCertificate cert = getCertificateFromDerBytes(derBytes);
+        if (cert == null) {
+            // if the certificate is null, cancel the request
+            return false;
+        }
+        final SslError sslError = SslError.SslErrorFromChromiumErrorCode(certError, cert, url);
+        SslErrorHandler handler = new SslErrorHandler() {
+            @Override
+            public void proceed() {
+                post(new Runnable() {
+                        @Override
+                        public void run() {
+                            proceedSslError(true, id);
+                        }
+                    });
+            }
+            @Override
+            public void cancel() {
+                post(new Runnable() {
+                        @Override
+                        public void run() {
+                            proceedSslError(false, id);
+                        }
+                    });
+            }
+        };
+        if (mClient != null) {

[mkosiba (inactive), 2013/02/22 15:35:56]

if the client is null then shouldn't we return false?

[sgurun-gerrit only, 2013/02/22 19:31:31]

good catch. This is an unnecessary check. mClient can't be null actually since
we asserting in the constructor and then keeping a ref.
On 2013/02/22 15:35:56, Martin Kosiba wrote:

+            mClient.onReceivedSslError(handler, sslError);
+        }
+        return true;
+    }
+
+    private void proceedSslError(boolean proceed, int id) {
+        if (mNativeAwContentsClientBridge == 0) return;
+        nativeProceedSslError(mNativeAwContentsClientBridge, proceed, id);
+    }
+
+    public static SslCertificate getCertificateFromDerBytes(byte[] derBytes) {
+        if (derBytes == null) {
+            return null;
+        }
+
+        try {
+            X509Certificate x509Certificate =
+                    X509Util.createCertificateFromBytes(derBytes);
+            return new SslCertificate(x509Certificate);
+        } catch (CertificateException e) {
+            // A SSL related exception must have occured.  This shouldn't happen.
+            Log.w(TAG, "Could not read certificate: " + e);
+        } catch (KeyStoreException e) {
+            // A SSL related exception must have occured.  This shouldn't happen.
+            Log.w(TAG, "Could not read certificate: " + e);
+        } catch (NoSuchAlgorithmException e) {
+            // A SSL related exception must have occured.  This shouldn't happen.
+            Log.w(TAG, "Could not read certificate: " + e);
+        }
+        return null;
+    }
+
+    //--------------------------------------------------------------------------------------------
+    //  Native methods
+    //--------------------------------------------------------------------------------------------
+    private native int nativeInit(int awContentsPtr);
+    private native void nativeProceedSslError(int nativeAwContentsClientBridge, boolean proceed,
+            int id);
+}