Captive portal certificate list should be checked when name mismatch is the only error

chrome/browser/ssl/ssl_error_handler.cc

Index: chrome/browser/ssl/ssl_error_handler.cc
diff --git a/chrome/browser/ssl/ssl_error_handler.cc b/chrome/browser/ssl/ssl_error_handler.cc
index 0b852f403488cb5eec80bd6b263fc55242c701b2..d706a9a4381ecb4cd8e093e50f3934683184ae6f 100644
--- a/chrome/browser/ssl/ssl_error_handler.cc
+++ b/chrome/browser/ssl/ssl_error_handler.cc
@@ -547,9 +547,18 @@ void SSLErrorHandler::StartHandlingError() {
     return;
   }
 
+  const net::CertStatus non_name_mismatch_errors =
+      ssl_info_.cert_status ^ net::CERT_STATUS_COMMON_NAME_INVALID;
+  const bool only_error_is_name_mismatch =
+      cert_error_ == net::ERR_CERT_COMMON_NAME_INVALID &&
+      (!net::IsCertStatusError(non_name_mismatch_errors) ||
+       net::IsCertStatusMinorError(ssl_info_.cert_status));
+
 #if BUILDFLAG(ENABLE_CAPTIVE_PORTAL_DETECTION)
+  // Check known captive portal certificate list if the only error is
+  // name-mismatch.

[estark, 2017/02/25 01:33:18]

nit: since we keep forgetting, it would probably be helpful to elaborate here on
why this is the right thing to do. Something along the lines of: "If there are
multiple errors, it indicates that the captive portal landing page itself will
have SSL errors, and so it's not a very helpful place to direct the user to go."
(is that right? I can't remember)

[meacer, 2017/02/27 23:50:58]

I think that's right, because I can't find any other justification for this.
Added the note.

I also wrote a doc to enumerate various scenarios:
https://docs.google.com/document/d/128GcLCNHyQLtv-kYOk1-6nwU_78iAhQpQWyPkN5b6...

   if (base::FeatureList::IsEnabled(kCaptivePortalCertificateList) &&
-      cert_error_ == net::ERR_CERT_COMMON_NAME_INVALID &&
+      only_error_is_name_mismatch &&
       g_config.Pointer()->IsKnownCaptivePortalCert(ssl_info_)) {
     RecordUMA(CAPTIVE_PORTAL_CERT_FOUND);
     ShowCaptivePortalInterstitial(
@@ -567,14 +576,11 @@ void SSLErrorHandler::StartHandlingError() {
       delegate_->IsErrorOverridable() &&
       delegate_->GetSuggestedUrl(dns_names, &suggested_url)) {
     RecordUMA(WWW_MISMATCH_FOUND);
-    net::CertStatus extra_cert_errors =
-        ssl_info_.cert_status ^ net::CERT_STATUS_COMMON_NAME_INVALID;
 
     // Show the SSL intersitial if |CERT_STATUS_COMMON_NAME_INVALID| is not
     // the only error. Need not check for captive portal in this case.
     // (See the comment below).
-    if (net::IsCertStatusError(extra_cert_errors) &&
-        !net::IsCertStatusMinorError(ssl_info_.cert_status)) {
+    if (!only_error_is_name_mismatch) {
       ShowSSLInterstitial();
       return;
     }