Implement Webviewclient.onReceivedSslError

android_webview/native/aw_certificate_error_handler_base.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/browser/aw_certificate_error_handler_base.h"
+
+#include "android_webview/native/aw_contents.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+
+namespace android_webview {
+
+AwCertificateErrorHandlerBase::~AwCertificateErrorHandlerBase() {

[boliu, 2013/02/01 23:07:13]

This needs to be in browser/ in the matching .cc file, but see comment below.

[sgurun-gerrit only, 2013/02/02 01:01:10]

I have chatted with Martin this morning and we sketched the general design as
this one to workaround the layering problem. If we have a policy to move cc file
next to header and put the implementation of static method to some other cc
file, this is also fine and easy to do. I will let Martin/Joth or anybody who
feels strong about it speak to it then.

[sgurun-gerrit only, 2013/02/02 01:08:11]

actually thinking about this again, I think cc and header files should be
together. I will do the change.

On 2013/02/02 01:01:10, sgurun wrote:

+}
+
+//static
+AwCertificateErrorHandlerBase* AwCertificateErrorHandlerBase::FromID(

[boliu, 2013/02/01 23:07:13]

Total stylistic issue:

Normally we would put this in the class where the rest of the class
implementation is, which means there is no need for this file and this method
implementation should live in aw_contents.cc

But, I'm not sure if this is the best way to work around the layering issue, but
I can't think of anything better. So maybe wait around for joth to review before
doing this.

[sgurun-gerrit only, 2013/02/02 01:01:10]

see above.

On 2013/02/01 23:07:13, boliu wrote:

[boliu, 2013/02/02 01:10:42]

Oh, if Martin already looked over the layering problem, then yeah, move the
destructor to a cc alongside the h file, then move all of this to aw_contents.cc

[sgurun-gerrit only, 2013/02/02 03:16:10]

Done.

+    int render_process_id,
+    int render_view_id) {
+
+  const content::RenderViewHost* host =
+      content::RenderViewHost::FromID(render_process_id, render_view_id);
+  DCHECK(host);

[boliu, 2013/02/01 23:07:13]

Sorry to turn this around on you again, but now that I think about it, I wonder
if this can happen in production, like say an ssl error happens while shutdown
is happening. (DCHECKs are supposed to be for conditions that never happens)

I don't know enough to answer that question, so maybe either wait for joth to
review this, or do it more defensively and return NULL if either of these steps
fail, and handle null case appropriately from the caller (probably LOG(WARNING)
and cancel)

[sgurun-gerrit only, 2013/02/02 01:01:10]

I have seen this pattern in multiple places, and I don't think we check for
nulls/DCHECK. But I don't know enough about it. If there is a slight chance that
it can happen, let's put a check now!

On 2013/02/01 23:07:13, boliu wrote:

+  content::WebContents* web_contents =
+      content::WebContents::FromRenderViewHost(host);
+  DCHECK(web_contents);
+  return static_cast<AwCertificateErrorHandlerBase*>
+      (AwContents::FromWebContents(web_contents));
+}
+
+}  // namespace android_webview