Cleanup of SSLHostStateDelegate and related code

content/public/browser/ssl_host_state_delegate.h

Index: content/public/browser/ssl_host_state_delegate.h
diff --git a/content/public/browser/ssl_host_state_delegate.h b/content/public/browser/ssl_host_state_delegate.h
index bcacd7fdef1aa08b47496abf9344090809df2dd5..9f564a194b1a42b8cb7d4f32bfb16c06cfc8514c 100644
--- a/content/public/browser/ssl_host_state_delegate.h
+++ b/content/public/browser/ssl_host_state_delegate.h
@@ -27,13 +27,13 @@ class SSLHostStateDelegate {
   // Records that |cert| is not permitted to be used for |host| in the future,
   // for a specified |error| type.
   virtual void DenyCert(const std::string& host,
-                        net::X509Certificate* cert,
+                        const net::X509Certificate& cert,

[Ryan Sleevi, 2014/09/04 21:46:24]

So, the trade-off here is that net::X509Certificate is designed to be
ref-counted.

If anyone wanted to be able to hold on to |cert| (e.g. as part of implementing
QueryPolicy), they can no longer do so with this API. Instead, they'd have to
create a new net::X509Certificate via the X509Certificate::Create methods
(which, because of an in memory cache, would return a pointer to the same object
after reparsing the data q_q).

This is fine because none of the (Chrome) implementations actually store on to
|cert|. But I don't know if that's long-term where you want the API to go, or if
that's the contract you make.

The benefit is you avoid the need for NULL-checking. The downside is you force
more work on the implementers of this interface from being able to implement
certain patterns.

I'm fine with either, because I don't want to get lost in a see of "what if",
but I wanted to make sure you understood the API tradeoff being done here.

[jww, 2014/09/05 00:16:31]

I am happy with the API choice here, so I'm going to go ahead and commit. I
prefer the non-NULL guarantee over the potential API choices in the future, and
worst case scenario, we can modify the API if that comes up.

                         net::CertStatus error) = 0;
 
   // Records that |cert| is permitted to be used for |host| in the future, for
   // a specified |error| type.
   virtual void AllowCert(const std::string&,
-                         net::X509Certificate* cert,
+                         const net::X509Certificate& cert,
                          net::CertStatus error) = 0;
 
   // Clear all allow/deny preferences.
@@ -44,7 +44,7 @@ class SSLHostStateDelegate {
   // immediately prior to this query, otherwise false.
   virtual net::CertPolicy::Judgment QueryPolicy(
       const std::string& host,
-      net::X509Certificate* cert,
+      const net::X509Certificate& cert,
       net::CertStatus error,
       bool* expired_previous_decision) = 0;