Forget SSL error exceptions when good certs seen for regular requests.

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 610f741dd98f65e6dd77777a0f63a98029536393..29feda37a84c4625eadbd1418993c4a49b8a7628 100644
--- a/content/browser/ssl/ssl_policy.cc
+++ b/content/browser/ssl/ssl_policy.cc
@@ -8,6 +8,7 @@
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/memory/singleton.h"
+#include "base/metrics/histogram.h"

[Mark P, 2015/04/17 16:20:36]

histogram_macros is more appropriate

[jww, 2015/04/17 18:14:33]

Done.

 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
@@ -105,13 +106,30 @@ void SSLPolicy::DidRunInsecureContent(NavigationEntryImpl* entry,
                                    site_instance->GetProcess()->GetID());
 }
 
+enum SSLGoodCertSeenEvent {

[Mark P, 2015/04/17 16:20:36]

1. This is an odd place for this enum.  This is style-acceptable?
2. Please put the useful standard warning label about enums that a recorded in
UMA (don't deleted or reorder or renumber; add new entry only at end)
3. Please explicitly do =0, =1, etc.

[jww, 2015/04/17 18:14:33]

You're right, it probably should be at the top in an anonymous namespace. Done.
Done.
Done.

+  NO_PREVIOUS_EXCEPTION,
+  HAD_PREVIOUS_EXCEPTION,
+  END_OF_SSL_GOOD_CERT_SEEN_EVENT
+};
+
 void SSLPolicy::OnRequestStarted(SSLRequestInfo* info) {
   // TODO(abarth): This mechanism is wrong.  What we should be doing is sending
   // this information back through WebKit and out some FrameLoaderClient
   // methods.
 
-  if (net::IsCertStatusError(info->ssl_cert_status()))
+  if (net::IsCertStatusError(info->ssl_cert_status())) {
     backend_->HostRanInsecureContent(info->url().host(), info->child_id());
+  } else {
+    SSLGoodCertSeenEvent event = NO_PREVIOUS_EXCEPTION;
+    if (backend_->HasAllowException(info->url().host()))
+      event = HAD_PREVIOUS_EXCEPTION;
+    UMA_HISTOGRAM_ENUMERATION("interstitial.ssl.good_cert_seen", event,

[Mark P, 2015/04/17 16:20:36]

This sounds more like a BOOLEAN histogram.  Why are you using an enum here?  Do
you have the expectation that it needs to expand in the future?  If so, how?

[jww, 2015/04/17 18:14:33]

In the past, when I've had boolean uma measurements, I've been asked to make an
enum anyway for clarity in the reporting (see SSLCertificateDecisionsDidRevoke
for example:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...).

Personally, I like that approach as well, but if you feel strongly that it
should be a straight boolean instead, let me know.

[Mark P, 2015/04/17 20:26:19]

Acknowledged.

+                              END_OF_SSL_GOOD_CERT_SEEN_EVENT);
+
+    // If there's no certificate error, a good certificate has been seen, so
+    // clear out any exceptions that were made by the user for bad certificates.
+    backend_->RevokeUserAllowExceptions(info->url().host());
+  }
 }
 
 void SSLPolicy::UpdateEntry(NavigationEntryImpl* entry,