Remove DenyCertForHost from SSLHostStateDelegate API.

content/browser/ssl/ssl_policy.cc

Index: content/browser/ssl/ssl_policy.cc
diff --git a/content/browser/ssl/ssl_policy.cc b/content/browser/ssl/ssl_policy.cc
index 18fdde4dfe2bf9823e716ae01e48db73f1641fb8..f2bdd2145cf2bc912468ff5d6d1554a36474aed4 100644
--- a/content/browser/ssl/ssl_policy.cc
+++ b/content/browser/ssl/ssl_policy.cc
@@ -45,10 +45,9 @@ void SSLPolicy::OnCertError(SSLCertErrorHandler* handler) {
     return;
   }
 
-  // The judgment is either DENIED or UNKNOWN.
-  // For now we handle the DENIED as the UNKNOWN, which means a blocking
-  // page is shown to the user every time he comes back to the page.
-
+  // The judgment must be UNKNOWN because QueryPolicy guarantees that it will
+  // never return DENIED. For these hosts, a blocking page is shown to the user
+  // every time he comes back to the page.
   int options_mask = 0;
   switch (handler->cert_error()) {
     case net::ERR_CERT_COMMON_NAME_INVALID:
@@ -180,13 +179,6 @@ void SSLPolicy::OnAllowCertificate(scoped_refptr<SSLCertErrorHandler> handler,
     handler->ContinueRequest();
   } else {
     // Default behavior for rejecting a certificate.
-    //
-    // While DenyCertForHost() executes synchronously on this thread,
-    // CancelRequest() gets posted to a different thread. Calling
-    // DenyCertForHost() first ensures deterministic ordering.
-    backend_->DenyCertForHost(handler->ssl_info().cert.get(),

[Ryan Sleevi, 2014/08/23 01:33:20]

You said this code isn't called, but this calls DenyCert, so it seems it was.

[jww, 2014/08/23 14:14:42]

In the description, I actually wrote that it's never "functionally used." A fine
line, and I'll try to clarify the comment because it is, indeed, confusing.

Yes, this calls DenyCert which, in the storage strategy, stores the decision as
"DENIED". However, the only place QueryPolicy is ever called (which is the only
way to retrieve this information) is right here in ssl_policy.cc through
ssl_policy_backend, where there's an explicit comment that DENIED will be
treated as UNKNOWN (see line 48 above). Hence the desire to get rid of this
confusing portion of the SSLHostStateDelegate API that implies that DENIED is
meaningful, but in practice, isn't.

[Ryan Sleevi, 2014/08/25 06:17:24]

So then shouldn't you also be removing that enum value from the CertPolicy?

And if you do that, does it still make sense as an enum when it's just boolean
(UNKNOWN, aka "!ALLOWED", and ALLOWED). I'm happy to leave it as an enum, but
that means updating the comment.

[jww, 2014/09/03 21:15:56]

I'll up you one. CertPolicy is dead. It's completely unused at this point. I
still like having an enum to make it clear and explicit about what's going on,
but I've move it to ssl_host_state_delegate.h, and I've removed net::CertPolicy
completely.

-                              handler->request_url().host(),
-                              handler->cert_error());
     handler->CancelRequest();
   }
 }