Optimize CT & OCSP handling code

net/cert/multi_log_ct_verifier_unittest.cc

Index: net/cert/multi_log_ct_verifier_unittest.cc
diff --git a/net/cert/multi_log_ct_verifier_unittest.cc b/net/cert/multi_log_ct_verifier_unittest.cc
index 3111b5e451e3f5716edef55b52686a589dfe9d0e..cf20c1ba27265d611133d52082116ff40dc3957f 100644
--- a/net/cert/multi_log_ct_verifier_unittest.cc
+++ b/net/cert/multi_log_ct_verifier_unittest.cc
@@ -111,18 +111,11 @@ class MultiLogCTVerifierTest : public ::testing::Test {
     return true;
   }
 
-  bool VerifySinglePrecertificateChain(
-      scoped_refptr<X509Certificate> chain,
-      const NetLogWithSource& net_log,
-      SignedCertificateTimestampAndStatusList* output_scts) {
-    return verifier_->Verify(chain.get(), std::string(), std::string(),
-                             output_scts, net_log) == OK;
-  }
-
   bool VerifySinglePrecertificateChain(scoped_refptr<X509Certificate> chain) {

[eroman, 2016/12/27 22:00:41]

Can you document what the return value of this function represents? (The new
implementation doesn't match what my expectation would be).

     SignedCertificateTimestampAndStatusList scts;
-    return verifier_->Verify(chain.get(), std::string(), std::string(), &scts,
-                             NetLogWithSource()) == OK;
+    verifier_->Verify(chain.get(), base::StringPiece(), base::StringPiece(),
+                      &scts, NetLogWithSource());
+    return !scts.empty();

[eroman, 2016/12/27 22:00:41]

Not sure what the goal of the function is, but I would have expected this to be
returning true only if there is a success:

return !SCTsMatchingStatus(scts, SCT_STATUS_OK).empty()

   }
 
   bool CheckPrecertificateVerification(scoped_refptr<X509Certificate> chain) {
@@ -130,11 +123,12 @@ class MultiLogCTVerifierTest : public ::testing::Test {
     TestNetLog test_net_log;
     NetLogWithSource net_log = NetLogWithSource::Make(
         &test_net_log, NetLogSourceType::SSL_CONNECT_JOB);
-    return (VerifySinglePrecertificateChain(chain, net_log, &scts) &&
-            ct::CheckForSingleVerifiedSCTInResult(scts, kLogDescription) &&
-            ct::CheckForSCTOrigin(
-                scts, ct::SignedCertificateTimestamp::SCT_EMBEDDED) &&
-            CheckForEmbeddedSCTInNetLog(test_net_log));
+    verifier_->Verify(chain.get(), base::StringPiece(), base::StringPiece(),
+                      &scts, net_log);
+    return ct::CheckForSingleVerifiedSCTInResult(scts, kLogDescription) &&
+           ct::CheckForSCTOrigin(
+               scts, ct::SignedCertificateTimestamp::SCT_EMBEDDED) &&
+           CheckForEmbeddedSCTInNetLog(test_net_log);
   }
 
   // Histogram-related helper methods
@@ -208,8 +202,8 @@ TEST_F(MultiLogCTVerifierTest, VerifiesSCTOverX509Cert) {
   std::string sct_list = ct::GetSCTListForTesting();
 
   SignedCertificateTimestampAndStatusList scts;
-  EXPECT_EQ(OK, verifier_->Verify(chain_.get(), std::string(), sct_list, &scts,
-                                  NetLogWithSource()));
+  verifier_->Verify(chain_.get(), base::StringPiece(), sct_list, &scts,
+                    NetLogWithSource());
   ASSERT_TRUE(ct::CheckForSingleVerifiedSCTInResult(scts, kLogDescription));
   ASSERT_TRUE(ct::CheckForSCTOrigin(
       scts, ct::SignedCertificateTimestamp::SCT_FROM_TLS_EXTENSION));
@@ -219,8 +213,8 @@ TEST_F(MultiLogCTVerifierTest, IdentifiesSCTFromUnknownLog) {
   std::string sct_list = ct::GetSCTListWithInvalidSCT();
   SignedCertificateTimestampAndStatusList scts;
 
-  EXPECT_NE(OK, verifier_->Verify(chain_.get(), std::string(), sct_list, &scts,
-                                  NetLogWithSource()));
+  verifier_->Verify(chain_.get(), base::StringPiece(), sct_list, &scts,
+                    NetLogWithSource());
   EXPECT_EQ(1U, scts.size());
   EXPECT_EQ("", scts[0].sct->log_description);
   EXPECT_EQ(ct::SCT_STATUS_LOG_UNKNOWN, scts[0].status);
@@ -242,8 +236,8 @@ TEST_F(MultiLogCTVerifierTest, CountsInvalidSCTsInStatusHistogram) {
   int num_invalid_scts = GetValueFromHistogram(
       "Net.CertificateTransparency.SCTStatus", ct::SCT_STATUS_LOG_UNKNOWN);
 
-  EXPECT_NE(OK, verifier_->Verify(chain_.get(), std::string(), sct_list, &scts,
-                                  NetLogWithSource()));
+  verifier_->Verify(chain_.get(), base::StringPiece(), sct_list, &scts,
+                    NetLogWithSource());
 
   ASSERT_EQ(num_valid_scts, NumValidSCTsInStatusHistogram());
   ASSERT_EQ(num_invalid_scts + 1,