Fix SSLClientSocketOpenSSL error-handling for Channel ID.

net/ssl/server_bound_cert_service.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/ssl/server_bound_cert_service.h"
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
@@ skipping 408 matching lines @@
 
   // See if a request for the same domain is currently in flight.
   bool create_if_missing = true;
   if (JoinToInFlightRequest(request_start, domain, private_key, cert,
                             create_if_missing, callback, out_req)) {
     return ERR_IO_PENDING;
   }
 
   int err = LookupDomainBoundCert(request_start, domain, private_key, cert,
                                   create_if_missing, callback, out_req);
   if (err == ERR_FILE_NOT_FOUND) {

[wtc, 2014/06/20 00:21:54]

We set create_if_missing to true on line 421 and pass that to
LookupDomainBoundCert(). Is this why we don't need to check (the equivalent of)
j->second->CreateIfMissing() here?

[davidben, 2014/06/20 20:05:57]

Yeah, GetOrCreateDomainBoundCert does a create_if_missing request and
GetDomainBoundCert does not. j->second->CreateIfMissing() is I think if any
request attached to the job had create_if_missing set.

     // Sync lookup did not find a valid cert.  Start generating a new one.
     workers_created_++;
     ServerBoundCertServiceWorker* worker = new ServerBoundCertServiceWorker(
         domain,
         base::Bind(&ServerBoundCertService::GeneratedServerBoundCert,
                    weak_ptr_factory_.GetWeakPtr()));
     if (!worker->Start(task_runner_)) {
       // TODO(rkn): Log to the NetLog.
       LOG(ERROR) << "ServerBoundCertServiceWorker couldn't be started.";
       RecordGetDomainBoundCertResult(WORKER_FAILURE);
@@ skipping 69 matching lines @@
   }
 
   if (err == OK) {
     // Async DB lookup found a valid cert.
     DVLOG(1) << "Cert store had valid cert for " << server_identifier;
     cert_store_hits_++;
     // ServerBoundCertServiceRequest::Post will do the histograms and stuff.
     HandleResult(OK, server_identifier, key, cert);
     return;
   }
-  // Async lookup did not find a valid cert. If no request asked to create one,
-  // return the error directly.
-  if (!j->second->CreateIfMissing()) {
+  // Async lookup failed or was missing. Return the error directly, unless the

[wtc, 2014/06/20 00:21:54]

Nit: add "the certificate" after "or".

[davidben, 2014/06/20 20:05:57]

Done.

+  // certificate was missing and a request asked to create one.
+  if (err != ERR_FILE_NOT_FOUND || !j->second->CreateIfMissing()) {
     HandleResult(err, server_identifier, key, cert);
     return;
   }
   // At least one request asked to create a cert => start generating a new one.
   workers_created_++;
   ServerBoundCertServiceWorker* worker = new ServerBoundCertServiceWorker(
       server_identifier,
       base::Bind(&ServerBoundCertService::GeneratedServerBoundCert,
                  weak_ptr_factory_.GetWeakPtr()));
   if (!worker->Start(task_runner_)) {
@@ skipping 135 matching lines @@
   }
 
   return err;
 }
 
 int ServerBoundCertService::cert_count() {
   return server_bound_cert_store_->GetCertCount();
 }
 
 }  // namespace net