Chromecast buildfix: ClientCertificateDelegate changes.

chromecast/browser/cast_content_browser_client.cc

Index: chromecast/browser/cast_content_browser_client.cc
diff --git a/chromecast/browser/cast_content_browser_client.cc b/chromecast/browser/cast_content_browser_client.cc
index 9793e68b6f0885a7d5025d6f78afd7a83e6cbe08..98ad74d46d391e3362d0245d20d33fcc82b3d279 100644
--- a/chromecast/browser/cast_content_browser_client.cc
+++ b/chromecast/browser/cast_content_browser_client.cc
@@ -32,6 +32,7 @@
 #include "content/public/browser/client_certificate_delegate.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_dispatcher_host.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/content_descriptors.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
@@ -195,7 +196,7 @@ void CastContentBrowserClient::AllowCertificateError(
 }
 
 void CastContentBrowserClient::SelectClientCertificate(
-    WebContents* web_contents,
+    content::WebContents* web_contents,
     net::SSLCertRequestInfo* cert_request_info,
     scoped_ptr<content::ClientCertificateDelegate> delegate) {
   GURL requesting_url("https://" + cert_request_info->host_and_port.ToString());
@@ -203,7 +204,6 @@ void CastContentBrowserClient::SelectClientCertificate(
   if (!requesting_url.is_valid()) {
     LOG(ERROR) << "Invalid URL string: "
                << requesting_url.possibly_invalid_spec();
-    delegate->SelectClientCertificate(nullptr);

[davidben, 2015/03/11 23:26:38]

NOTE: This is a behavior change from before my CL. Although it's probably what
you want? Selecting null is to affirmatively continue without a certificate.
Letting the dtor run is to abort the request. Before, only the former was
possible. I intentionally left the behavior as-is because I wasn't sure what
this code was supposed to do[*]. (Though it should never get to this point
anyway.)

[*] I also still need to meet with you folks about it. It's unlikely to do what
you want because client auth decisions in one request apply from then on into
the entire BrowsingContext.

[davidben, 2015/03/11 23:28:41]

Er, to clarify, that's not a result of my change or anything. This has been true
forever. If one request selects a certificate, it applies to all requests in the
BrowserContext from then on. If one requests selects no certificate (rather than
abort), it *also* applies to all requests in the BrowserContext from then on.

[gunsch, 2015/03/11 23:32:13]

Hmm, is that really the case? Before your CL, this read "callback.run(NULL)",
and the other cases I see seem to have changed to just let
ClientCertificateDelegate get destroyed. E.g. (from the diff):

https://codereview.chromium.org/download/issue859213006_720001.diff

ContentBrowserClient::SelectClientCertificate
and
AwContentBrowserClient::SelectClientCertificate

Were those locations a behavior change in your CL, or the new code path for
maintaining the same behavior?

[davidben, 2015/03/11 23:35:23]

callback.run(NULL) is equivalent to SelectClientCertificate(NULL). There was no
way to abort a client certificate request at all before. (That was the point of
the CL.)
Those were a behavior change. (A reviewer asked me to sort those out there. I
probably should have done it in a follow-up. Oh well. Those places were less
unsound, so I was more comfortable changing them.)

[gunsch, 2015/03/11 23:59:53]

Got it. I'll go with ContinueWithCertificate(nullptr) for now and talk more with
Doug about the intended behavior. Thanks for the information.

     return;
   }
 
@@ -223,7 +223,7 @@ void CastContentBrowserClient::SelectClientCertificate(
                  base::Unretained(this), requesting_url,
                  web_contents->GetRenderProcessHost()->GetID()),
       base::Bind(&content::ClientCertificateDelegate::ContinueWithCertificate,
-                 delegate.Pass()));
+                 base::Owned(delegate.release())));
 }
 
 net::X509Certificate*