Chromecast buildfix: ClientCertificateDelegate changes.

chromecast/browser/cast_content_browser_client.cc

 // Copyright 2014 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 "chromecast/browser/cast_content_browser_client.h"
 
 #include <string>
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
@@ skipping 14 matching lines @@
 #include "chromecast/common/chromecast_switches.h"
 #include "chromecast/common/global_descriptors.h"
 #include "components/crash/app/breakpad_linux.h"
 #include "components/crash/browser/crash_handler_host_linux.h"
 #include "components/network_hints/browser/network_hints_message_filter.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/certificate_request_result_type.h"
 #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"
 #include "content/public/common/web_preferences.h"
 #include "net/ssl/ssl_cert_request_info.h"
 
 #if defined(OS_ANDROID)
 #include "chromecast/browser/android/external_video_surface_container_impl.h"
 #endif  // defined(OS_ANDROID)
 
@@ skipping 143 matching lines @@
     bool expired_previous_decision,
     const base::Callback<void(bool)>& callback,
     content::CertificateRequestResultType* result) {
   // Allow developers to override certificate errors.
   // Otherwise, any fatal certificate errors will cause an abort.
   *result = content::CERTIFICATE_REQUEST_RESULT_TYPE_CANCEL;
   return;
 }
 
 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());
 
   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;
   }
 
   // In our case there are no relevant certs in the cert_request_info. The cert
   // we need to return (if permitted) is the Cast device cert, which we can
   // access directly through the ClientAuthSigner instance. However, we need to
   // be on the IO thread to determine whether the app is whitelisted to return
   // it, because CastNetworkDelegate is bound to the IO thread.
   // Subsequently, the callback must then itself be performed back here
   // on the UI thread.
   //
   // TODO(davidben): Stop using child ID to identify an app.
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&CastContentBrowserClient::SelectClientCertificateOnIOThread,
                  base::Unretained(this), requesting_url,
                  web_contents->GetRenderProcessHost()->GetID()),
       base::Bind(&content::ClientCertificateDelegate::ContinueWithCertificate,
-                 delegate.Pass()));
+                 base::Owned(delegate.release())));
 }
 
 net::X509Certificate*
 CastContentBrowserClient::SelectClientCertificateOnIOThread(
     GURL requesting_url,
     int render_process_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   CastNetworkDelegate* network_delegate =
       url_request_context_factory_->app_network_delegate();
   if (network_delegate->IsWhitelisted(requesting_url,
@@ skipping 120 matching lines @@
         process_type, dumps_path, false /* upload */);
   // StartUploaderThread() even though upload is diferred.
   // Breakpad-related memory is freed in the uploader thread.
   crash_handler->StartUploaderThread();
   return crash_handler;
 }
 #endif  // !defined(OS_ANDROID)
 
 }  // namespace shell
 }  // namespace chromecast