Implement Webviewclient.onReceivedSslError

android_webview/browser/aw_content_browser_client.cc

Index: android_webview/browser/aw_content_browser_client.cc
diff --git a/android_webview/browser/aw_content_browser_client.cc b/android_webview/browser/aw_content_browser_client.cc
index bc4918be2a9a606935bf2bef4dd0e0605406a3b4..9d5ef40d01609f46146ffff70fb7ea45139fc29d 100644
--- a/android_webview/browser/aw_content_browser_client.cc
+++ b/android_webview/browser/aw_content_browser_client.cc
@@ -5,6 +5,7 @@
 #include "android_webview/browser/aw_content_browser_client.h"
 
 #include "android_webview/browser/aw_browser_main_parts.h"
+#include "android_webview/browser/aw_certificate_error_handler_base.h"
 #include "android_webview/browser/aw_cookie_access_policy.h"
 #include "android_webview/browser/aw_quota_permission_context.h"
 #include "android_webview/browser/net_disk_cache_remover.h"
@@ -20,6 +21,7 @@
 #include "content/public/common/url_constants.h"
 #include "grit/ui_resources.h"
 #include "net/android/network_library.h"
+#include "net/base/ssl_info.h"
 #include "ui/base/resource/resource_bundle.h"
 
 namespace {
@@ -265,9 +267,15 @@ void AwContentBrowserClient::AllowCertificateError(
     bool strict_enforcement,
     const base::Callback<void(bool)>& callback,
     bool* cancel_request) {
-  // TODO(boliu): Implement this to power WebViewClient.onReceivedSslError.
-  NOTIMPLEMENTED();
-  *cancel_request = true;
+
+  AwCertificateErrorHandlerBase* error_handler =
+      AwCertificateErrorHandlerBase::FromID(render_process_id, render_view_id);

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

you're fighting your way up the layers in an awkward way here - declaring static
function in lower layer but defining it in higher one is skirting around the
issue (I think AwHttpAuthHandlerBase has the same snag, but not a reason to copy
it now we have better things to work with)

for singleton-like objects,  Bo's JNI dependency factory maybe the cool new
solution to all these problems:
https://codereview.chromium.org/12253057/diff/23008/android_webview/browser/j...

but I don't think that's the right fit here. you need the object to be
per-webview (i.e. the AwContentsClient is a per webview relation)

so I think the answer is,
1) expand AwCertificateErrorHandlerBase to be the browser/ base class for all of
AwContentsClient. (we might call it AwContentsClient and have the native/
subclass be AwContentsClientImpl
2) use WebContents::SetUserData to store a  AwContentsClient* inside the web
contents instance. (see AwContents for an example). (be carefull to handle the
AwContents::SetWebContents case too)
3) here, use
WebContents::FromId()->GetUserData(kAwContentsClient)->allowCertificateError()
(we can make a static method AwContentsClient::FromID(), and
AwContentsClient::FromWebContents, to hide some of that)


I can pair code that if it doesn't make sense.

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

I talked to Martin and got his idea about this part since I am not too familiar
with what patterns we use to deal with such inter layer access. 

I can do the change but I am not sure what you mean by "be careful to handle
AwContents::SetWebContents case too"

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

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

The static method was my suggestion - I believe at that time of our conversation
the AwHttpAuthHandlerBase pattern was the only thing we had. Sorry for
misleading you there, but it looks like Joth's plan shouldn't be a big pain to
apply.

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

I am doing the change -- Maybe I did not understand it right, but I did not like
this new solution. The UserData looks like a object registry where you register
some objects for using later. So why did not I like it that much: first it takes
the ownership of the object (unnecessary ownership concerns), second, as Joth
pointed out webcontents can be set later, creating another opportunity to create
bugs in future, third it makes me use another unnecessary reinterpret_cast
(which unfortunately takes away the typesafety). 
It is very possible I am misunderstanding how to use it by a gross margin
though. 

I am implementing this anyway...

On 2013/02/22 15:35:56, Martin Kosiba wrote:

+  if (error_handler) {
+    error_handler->AllowCertificateError(cert_error, ssl_info.cert, request_url,
+        callback, cancel_request);
+  } else {
+    *cancel_request = true;
+  }
 }
 
 WebKit::WebNotificationPresenter::Permission