Implement Webviewclient.onReceivedSslError

android_webview/browser/aw_content_browser_client.cc

 // Copyright (c) 2012 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/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"
 #include "android_webview/browser/renderer_host/aw_resource_dispatcher_host_delegate.h"
 #include "android_webview/common/url_constants.h"
 #include "base/android/locale_utils.h"
 #include "base/base_paths_android.h"
 #include "base/path_service.h"
 #include "content/public/browser/access_token_store.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #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 {
 
 class AwAccessTokenStore : public content::AccessTokenStore {
  public:
   AwAccessTokenStore() { }
 
   // content::AccessTokenStore implementation
   virtual void LoadAccessTokens(
@@ skipping 225 matching lines @@
 void AwContentBrowserClient::AllowCertificateError(
     int render_process_id,
     int render_view_id,
     int cert_error,
     const net::SSLInfo& ssl_info,
     const GURL& request_url,
     bool overridable,
     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
     AwContentBrowserClient::CheckDesktopNotificationPermission(
         const GURL& source_url,
         content::ResourceContext* context,
         int render_process_id) {
   // Android WebView does not support notifications, so return Denied here.
   return WebKit::WebNotificationPresenter::PermissionDenied;
 }
@@ skipping 104 matching lines @@
 
 bool AwContentBrowserClient::AllowPepperSocketAPI(
     content::BrowserContext* browser_context,
     const GURL& url,
     const content::SocketPermissionRequest& params) {
   NOTREACHED() << "Android WebView does not support plugins";
   return false;
 }
 
 }  // namespace android_webview