Handle all non-overridable SSL errors in the same place

net/url_request/url_request_http_job.cc

 // Copyright (c) 2012 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 "net/url_request/url_request_http_job.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 872 matching lines @@
                                        NetLog::StringCallback("source",
                                                               &source));
           NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, error));
         }
         return;
       }
     }
 
     SaveCookiesAndNotifyHeadersComplete(net::OK);
   } else if (IsCertificateError(result)) {
-    // We encountered an SSL certificate error.  Ask our delegate to decide
-    // what we should do.
-
-    TransportSecurityState::DomainState domain_state;
-    const URLRequestContext* context = request_->context();
-    const bool fatal = context->transport_security_state() &&
-        context->transport_security_state()->GetDomainState(
-            request_info_.url.host(),
-            SSLConfigService::IsSNIAvailable(context->ssl_config_service()),
-            &domain_state) &&
-        domain_state.ShouldSSLErrorsBeFatal();
-    NotifySSLCertificateError(transaction_->GetResponseInfo()->ssl_info, fatal);
+    // We encountered an SSL certificate error.
+    if (result == ERR_SSL_WEAK_SERVER_EPHEMERAL_DH_KEY ||
+        result == ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN) {
+      // These are hard failures. They're handled separately and don't have
+      // the correct cert status, so we set it here.

[Ryan Sleevi, 2013/09/10 20:51:27]

comment nit: Can you drop the "we", leaving the comment more concise.

// so set it here.

[felt, 2013/09/10 21:03:21]

Done.

+      SSLInfo info(transaction_->GetResponseInfo()->ssl_info);
+      info.cert_status = MapNetErrorToCertStatus(result);
+      NotifySSLCertificateError(info, true);
+    } else {
+      // Maybe overridable, maybe not.
+      // Ask our delegate to decide what we should do.

[Ryan Sleevi, 2013/09/10 20:51:27]

comment nit:
// The error may be overridable. Ask the delegate to decide.

avoids the pronouns, and avoids the "maybe, maybe not" construct.

[felt, 2013/09/10 21:03:21]

Done.

+      TransportSecurityState::DomainState domain_state;
+      const URLRequestContext* context = request_->context();
+      const bool fatal = context->transport_security_state() &&
+          context->transport_security_state()->GetDomainState(
+              request_info_.url.host(),
+              SSLConfigService::IsSNIAvailable(context->ssl_config_service()),
+              &domain_state) &&
+          domain_state.ShouldSSLErrorsBeFatal();
+      NotifySSLCertificateError(
+          transaction_->GetResponseInfo()->ssl_info, fatal);
+    }
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     NotifyCertificateRequested(
         transaction_->GetResponseInfo()->cert_request_info.get());
   } else {
     NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, result));
   }
 }
 
 void URLRequestHttpJob::OnHeadersReceivedCallback(int result) {
   SetUnblockedOnDelegate();
@@ skipping 589 matching lines @@
 
 void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 }
 
 void URLRequestHttpJob::OnDetachRequest() {
   http_transaction_delegate_->OnDetachRequest();
 }
 
 }  // namespace net