Add retry information to certificate reports.

chrome/browser/safe_browsing/certificate_reporting_service_test_utils.cc

 // Copyright 2016 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 "chrome/browser/safe_browsing/certificate_reporting_service_test_utils.h"
 
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/certificate_reporting/encrypted_cert_logger.pb.h"
 #include "components/certificate_reporting/error_report.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 31 matching lines @@
                               const uint8_t* server_private_key) {
   std::string serialized_report(GetUploadData(request));
   certificate_reporting::EncryptedCertLoggerRequest encrypted_request;
   EXPECT_TRUE(encrypted_request.ParseFromString(serialized_report));
   EXPECT_EQ(kServerPublicKeyTestVersion,
             encrypted_request.server_public_key_version());
   EXPECT_EQ(certificate_reporting::EncryptedCertLoggerRequest::
                 AEAD_ECDH_AES_128_CTR_HMAC_SHA256,
             encrypted_request.algorithm());
   std::string decrypted_report;
-  certificate_reporting::ErrorReporter::DecryptErrorReport(
-      server_private_key, encrypted_request, &decrypted_report);
+  EXPECT_TRUE(certificate_reporting::ErrorReporter::DecryptErrorReport(
+      server_private_key, encrypted_request, &decrypted_report));
   return decrypted_report;
 }
 
-// Checks that the serialized reports in |observed_reports| have the same
-// hostnames as |expected_hostnames|.
-void CompareHostnames(const std::set<std::string>& expected_hostnames,
-                      const std::set<std::string>& observed_reports,
-                      const std::string& comparison_type) {
-  std::set<std::string> observed_hostnames;
-  for (const std::string& serialized_report : observed_reports) {
-    certificate_reporting::ErrorReport report;
-    ASSERT_TRUE(report.InitializeFromString(serialized_report));
-    observed_hostnames.insert(report.hostname());
-  }
-  EXPECT_EQ(expected_hostnames, observed_hostnames)
-      << "Comparison failed for " << comparison_type << " reports.";
-}
-
 void WaitReports(
     certificate_reporting_test_utils::RequestObserver* observer,
     const certificate_reporting_test_utils::ReportExpectation& expectation) {
   observer->Wait(expectation.num_reports());
-  CompareHostnames(expectation.successful_reports,
-                   observer->successful_reports(), "successful");
-  CompareHostnames(expectation.failed_reports, observer->failed_reports(),
-                   "failed");
-  CompareHostnames(expectation.delayed_reports, observer->delayed_reports(),
-                   "delayed");
+  EXPECT_EQ(expectation.successful_reports, observer->successful_reports());
+  EXPECT_EQ(expectation.failed_reports, observer->failed_reports());
+  EXPECT_EQ(expectation.delayed_reports, observer->delayed_reports());
   observer->ClearObservedReports();
 }
 
 }  // namespace
 
 namespace certificate_reporting_test_utils {
 
+ObservedReport::ObservedReport(const std::string& hostname, bool is_retried)
+    : hostname(hostname), is_retried(is_retried) {}
+
+// static
+ObservedReport ObservedReport::Retried(const std::string& hostname) {
+  return ObservedReport(hostname, true);
+}
+
+// static
+ObservedReport ObservedReport::NotRetried(const std::string& hostname) {
+  return ObservedReport(hostname, false);
+}
+
+bool ObservedReport::operator<(const ObservedReport& other) const {

[estark, 2017/01/13 23:26:22]

Shouldn't these compare is_retried as well? (If that's not necessary for some
reason, maybe leave a comment in the .h)

[meacer, 2017/01/17 22:24:27]

This was keying on the hostname, leaving is_retried out. But your comment made
me realize what we want is actually a map instead of a set, so changed std::set
to unordered map. ObservedReport struct is no longer needed.

+  return hostname < other.hostname;
+}
+
+bool ObservedReport::operator==(const ObservedReport& other) const {
+  return hostname == other.hostname;
+}
+
 RequestObserver::RequestObserver()
     : num_events_to_wait_for_(0u), num_received_events_(0u) {}
 RequestObserver::~RequestObserver() {}
 
 void RequestObserver::Wait(unsigned int num_events_to_wait_for) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!run_loop_);
   ASSERT_LE(num_received_events_, num_events_to_wait_for)
       << "Observed unexpected report";
 
   if (num_received_events_ < num_events_to_wait_for) {
     num_events_to_wait_for_ = num_events_to_wait_for;
     run_loop_.reset(new base::RunLoop());
     run_loop_->Run();
     run_loop_.reset(nullptr);
     EXPECT_EQ(0u, num_received_events_);
     EXPECT_EQ(0u, num_events_to_wait_for_);
   } else if (num_received_events_ == num_events_to_wait_for) {
     num_received_events_ = 0u;
     num_events_to_wait_for_ = 0u;
   }
 }
 
-void RequestObserver::OnRequest(const std::string& serialized_report,
+void RequestObserver::OnRequest(const ObservedReport& observed_report,
                                 ReportSendingResult report_type) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   switch (report_type) {
     case REPORTS_SUCCESSFUL:
-      successful_reports_.insert(serialized_report);
+      successful_reports_.insert(observed_report);
       break;
     case REPORTS_FAIL:
-      failed_reports_.insert(serialized_report);
+      failed_reports_.insert(observed_report);
       break;
     case REPORTS_DELAY:
-      delayed_reports_.insert(serialized_report);
+      delayed_reports_.insert(observed_report);
       break;
   }
 
   num_received_events_++;
   if (!run_loop_) {
     return;
   }
   ASSERT_LE(num_received_events_, num_events_to_wait_for_)
       << "Observed unexpected report";
 
   if (num_received_events_ == num_events_to_wait_for_) {
     num_events_to_wait_for_ = 0u;
     num_received_events_ = 0u;
     run_loop_->Quit();
   }
 }
 
-const std::set<std::string>& RequestObserver::successful_reports() const {
+const std::set<ObservedReport>& RequestObserver::successful_reports() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return successful_reports_;
 }
 
-const std::set<std::string>& RequestObserver::failed_reports() const {
+const std::set<ObservedReport>& RequestObserver::failed_reports() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return failed_reports_;
 }
 
-const std::set<std::string>& RequestObserver::delayed_reports() const {
+const std::set<ObservedReport>& RequestObserver::delayed_reports() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return delayed_reports_;
 }
 
 void RequestObserver::ClearObservedReports() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   successful_reports_.clear();
   failed_reports_.clear();
   delayed_reports_.clear();
 }
@@ skipping 73 matching lines @@
       server_private_key_(server_private_key),
       weak_factory_(this) {}
 
 CertReportJobInterceptor::~CertReportJobInterceptor() {}
 
 net::URLRequestJob* CertReportJobInterceptor::MaybeInterceptRequest(
     net::URLRequest* request,
     net::NetworkDelegate* network_delegate) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
 
-  const std::string uploaded_report =
+  const std::string serialized_report =
       GetReportContents(request, server_private_key_);
   content::BrowserThread::PostTask(
       content::BrowserThread::UI, FROM_HERE,
       base::Bind(&CertReportJobInterceptor::RequestCreated,
-                 weak_factory_.GetWeakPtr(), uploaded_report,
+                 weak_factory_.GetWeakPtr(), serialized_report,
                  expected_report_result_));
 
   if (expected_report_result_ == REPORTS_FAIL) {
     return new DelayableCertReportURLRequestJob(
         false, true, request, network_delegate,
         base::Bind(&CertReportJobInterceptor::RequestDestructed,
-                   base::Unretained(this), uploaded_report,
+                   base::Unretained(this), serialized_report,
                    expected_report_result_));
 
   } else if (expected_report_result_ == REPORTS_DELAY) {
     DCHECK(!delayed_request_) << "Supports only one delayed request at a time";
     DelayableCertReportURLRequestJob* job =
         new DelayableCertReportURLRequestJob(
             true, false, request, network_delegate,
             base::Bind(&CertReportJobInterceptor::RequestDestructed,
-                       base::Unretained(this), uploaded_report,
+                       base::Unretained(this), serialized_report,
                        expected_report_result_));
     delayed_request_ = job->GetWeakPtr();
     return job;
   }
   // Successful url request job.
   return new DelayableCertReportURLRequestJob(
       false, false, request, network_delegate,
       base::Bind(&CertReportJobInterceptor::RequestDestructed,
-                 base::Unretained(this), uploaded_report,
+                 base::Unretained(this), serialized_report,
                  expected_report_result_));
 }
 
 void CertReportJobInterceptor::SetFailureMode(
     ReportSendingResult expected_report_result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&CertReportJobInterceptor::SetFailureModeOnIOThread,
                  weak_factory_.GetWeakPtr(), expected_report_result));
@@ skipping 24 matching lines @@
 }
 
 void CertReportJobInterceptor::ResumeOnIOThread() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   EXPECT_EQ(REPORTS_DELAY, expected_report_result_);
   if (delayed_request_)
     delayed_request_->Resume();
 }
 
 void CertReportJobInterceptor::RequestCreated(
-    const std::string& uploaded_report,
+    const std::string& serialized_report,
     ReportSendingResult expected_report_result) const {
-  request_created_observer_.OnRequest(uploaded_report, expected_report_result);
+  certificate_reporting::ErrorReport error_report;
+  EXPECT_TRUE(error_report.InitializeFromString(serialized_report));
+  request_created_observer_.OnRequest(
+      ObservedReport(error_report.hostname(), error_report.is_retry_upload()),
+      expected_report_result);
 }
 
 void CertReportJobInterceptor::RequestDestructed(
-    const std::string& uploaded_report,
+    const std::string& serialized_report,
     ReportSendingResult expected_report_result) const {
-  request_destroyed_observer_.OnRequest(uploaded_report,
-                                        expected_report_result);
+  certificate_reporting::ErrorReport error_report;
+  EXPECT_TRUE(error_report.InitializeFromString(serialized_report));
+  request_destroyed_observer_.OnRequest(
+      ObservedReport(error_report.hostname(), error_report.is_retry_upload()),
+      expected_report_result);
 }
 
 ReportExpectation::ReportExpectation() {}
 
 ReportExpectation::ReportExpectation(const ReportExpectation& other) = default;
 
 ReportExpectation::~ReportExpectation() {}
 
 // static
 ReportExpectation ReportExpectation::Successful(
-    const std::set<std::string>& reports) {
+    const std::set<ObservedReport>& reports) {
   ReportExpectation expectation;
   expectation.successful_reports = reports;
   return expectation;
 }
 
 // static
 ReportExpectation ReportExpectation::Failed(
-    const std::set<std::string>& reports) {
+    const std::set<ObservedReport>& reports) {
   ReportExpectation expectation;
   expectation.failed_reports = reports;
   return expectation;
 }
 
 // static
 ReportExpectation ReportExpectation::Delayed(
-    const std::set<std::string>& reports) {
+    const std::set<ObservedReport>& reports) {
   ReportExpectation expectation;
   expectation.delayed_reports = reports;
   return expectation;
 }
 
 int ReportExpectation::num_reports() const {
   return successful_reports.size() + failed_reports.size() +
          delayed_reports.size();
 }
 
@@ skipping 84 matching lines @@
 
   if (service->GetReporterForTesting()) {
     // Reporter can be null if reporting is disabled.
     EXPECT_EQ(
         0u,
         service->GetReporterForTesting()->inflight_report_count_for_testing());
   }
 }
 
 }  // namespace certificate_reporting_test_utils