Implement main CertificateReportingService code and add unit tests.

chrome/browser/safe_browsing/certificate_reporting_service.cc

Index: chrome/browser/safe_browsing/certificate_reporting_service.cc
diff --git a/chrome/browser/safe_browsing/certificate_reporting_service.cc b/chrome/browser/safe_browsing/certificate_reporting_service.cc
index 52a20c7501cdc040239c5295ddf3d94269295877..bbe31f85cf4c977afb40742ac2993c77820800a1 100644
--- a/chrome/browser/safe_browsing/certificate_reporting_service.cc
+++ b/chrome/browser/safe_browsing/certificate_reporting_service.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "base/bind_helpers.h"
 #include "base/time/clock.h"
 #include "base/time/default_clock.h"
 #include "chrome/browser/safe_browsing/certificate_reporting_service.h"
@@ -17,6 +18,10 @@ bool ReportCompareFunc(const CertificateReportingService::Report& item1,
 
 }  // namespace
 
+// static
+const char CertificateReportingService::kExtendedReportingUploadUrlInsecure[] =
+    "http://safebrowsing.googleusercontent.com/safebrowsing/clientreport/";
+
 CertificateReportingService::BoundedReportList::BoundedReportList(
     size_t max_size)
     : max_size_(max_size) {
@@ -57,11 +62,15 @@ CertificateReportingService::Reporter::Reporter(
     std::unique_ptr<certificate_reporting::ErrorReporter> error_reporter,
     std::unique_ptr<BoundedReportList> retry_list,
     base::Clock* clock,
-    base::TimeDelta report_ttl)
+    base::TimeDelta report_ttl,
+    EventObserver* event_observer,
+    bool retries_enabled)
     : error_reporter_(std::move(error_reporter)),
       retry_list_(std::move(retry_list)),
       test_clock_(clock),
       report_ttl_(report_ttl),
+      event_observer_(event_observer),
+      retries_enabled_(retries_enabled),
       current_report_id_(0),
       weak_factory_(this) {}
 
@@ -77,6 +86,9 @@ void CertificateReportingService::Reporter::Send(
 
 void CertificateReportingService::Reporter::SendPending() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  if (!retries_enabled_) {
+    return;
+  }
   base::Time now =
       test_clock_ ? test_clock_->Now() : base::Time::NowFromSystemTime();
   // Copy pending reports and clear the retry list.
@@ -102,6 +114,11 @@ CertificateReportingService::Reporter::GetQueueForTesting() const {
   return retry_list_.get();
 }
 
+void CertificateReportingService::Reporter::SetEventObserverForTesting(
+    EventObserver* observer) {
+  event_observer_ = observer;
+}
+
 void CertificateReportingService::Reporter::SendInternal(
     const CertificateReportingService::Report& report) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
@@ -118,13 +135,185 @@ void CertificateReportingService::Reporter::ErrorCallback(int report_id,
                                                           const GURL& url,
                                                           int error) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  auto it = inflight_reports_.find(report_id);
-  DCHECK(it != inflight_reports_.end());
-  retry_list_->Add(it->second);
+  if (retries_enabled_) {
+    auto it = inflight_reports_.find(report_id);
+    DCHECK(it != inflight_reports_.end());
+    retry_list_->Add(it->second);
+  }
   CHECK_GT(inflight_reports_.erase(report_id), 0u);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&CertificateReportingService::EventObserver::OnSendComplete,
+                 base::Unretained(event_observer_), report_id, false));
 }
 
 void CertificateReportingService::Reporter::SuccessCallback(int report_id) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   CHECK_GT(inflight_reports_.erase(report_id), 0u);
+  content::BrowserThread::PostTask(
+      content::BrowserThread::UI, FROM_HERE,
+      base::Bind(&CertificateReportingService::EventObserver::OnSendComplete,
+                 base::Unretained(event_observer_), report_id, true));
+}
+
+CertificateReportingService::CertificateReportingService(
+    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
+    std::unique_ptr<EventObserver> event_observer,
+    size_t max_queued_report_count,
+    base::TimeDelta max_report_age,
+    base::Clock* test_clock)
+    : enabled_(true),
+      url_request_context_(nullptr),
+      event_observer_(std::move(event_observer)),
+      max_queued_report_count_(max_queued_report_count),
+      max_report_age_(max_report_age),
+      test_clock_(test_clock),
+      made_send_attempt_(false) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CertificateReportingService::InitializeOnIOThread,
+                 base::Unretained(this), enabled_, url_request_context_getter,
+                 max_queued_report_count_, max_report_age_, test_clock_,
+                 nullptr /* error_reporter */),

[Jialiu Lin, 2016/11/30 22:18:06]

Maybe add a TODO here to indicate that "nullptr" need to be changed to wire the
actual reporter when everything is ready.  

[meacer, 2016/11/30 23:39:40]

Looks like the parameter was just wrong, it should be event_observer_ instead.
The tests still pass though, because:

- The unittest creates a CertificateReportingService by passing it an observer.
- This observer waits for a single Reset event when the service is created. It
doesn't wait for Send events.
- Since the Reset event is posted by the reply callback below (line 179),
passing nullptr to InitializeOnIOThread here doesn't cause any problems (at
least for the current test setup) because this parameter is passed down to
Reporter (which handles Send events).
- Finally, individual test cases that observe Send events set their own event
observers, overwriting this one.

I'm now passing the proper value.

+      base::Bind(&EventObserver::OnReset,
+                 base::Unretained(event_observer_.get())));
+}
+
+CertificateReportingService::~CertificateReportingService() {
+  DCHECK(!reporter_);
+}
+
+void CertificateReportingService::Shutdown() {
+  // Shutdown will be called twice: Once after SafeBrowsing shuts down, and once
+  // when all KeyedServices shut down. All calls after the first one is a no-op.
+  enabled_ = false;
+
+  Reset(base::Bind(&EventObserver::OnReset,
+                   base::Unretained(event_observer_.get())));
+}
+
+void CertificateReportingService::Send(const std::string& serialized_report) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  made_send_attempt_ = true;
+  if (!reporter_) {
+    DidAttemptSend(false);
+    return;
+  }
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CertificateReportingService::Reporter::Send,
+                 base::Unretained(reporter_.get()), serialized_report),
+      base::Bind(&CertificateReportingService::DidAttemptSend,
+                 base::Unretained(this), true));
+}
+
+void CertificateReportingService::SendPending() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  made_send_attempt_ = true;
+  if (!reporter_) {
+    DidAttemptSend(false);
+    return;
+  }
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CertificateReportingService::Reporter::SendPending,
+                 base::Unretained(reporter_.get())),
+      base::Bind(&CertificateReportingService::DidAttemptSend,
+                 base::Unretained(this), true));
+}
+
+void CertificateReportingService::InitializeOnIOThread(
+    bool enabled,
+    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
+    size_t max_queued_report_count,
+    base::TimeDelta max_report_age,
+    base::Clock* test_clock,
+    EventObserver* event_observer) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  DCHECK(!url_request_context_);
+  url_request_context_ = url_request_context_getter->GetURLRequestContext();
+  ResetOnIOThread(enabled, url_request_context_, max_queued_report_count,
+                  max_report_age, test_clock, event_observer);
+}
+
+void CertificateReportingService::DidAttemptSend(bool sent) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  event_observer_->OnSendAttempt(sent);
+}
+
+void CertificateReportingService::SetEnabled(bool enabled) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  enabled_ = enabled;
+  Reset(base::Bind(&EventObserver::OnReset,
+                   base::Unretained(event_observer_.get())));
+}
+
+CertificateReportingService::Reporter*
+CertificateReportingService::get_reporter_for_testing() const {
+  return reporter_.get();
+}
+
+void CertificateReportingService::SetEventObserverForTesting(
+    std::unique_ptr<CertificateReportingService::EventObserver> observer) {
+  event_observer_ = std::move(observer);
+  // reporter_ can be null if reporting is disabled.
+  if (reporter_) {
+    reporter_->SetEventObserverForTesting(event_observer_.get());
+  }
+}
+
+void CertificateReportingService::SetMaxQueuedReportCountForTesting(
+    size_t count) {
+  DCHECK(!made_send_attempt_);
+  max_queued_report_count_ = count;
+  Reset(base::Bind(base::DoNothing));
+}
+
+void CertificateReportingService::SetClockForTesting(base::Clock* clock) {
+  DCHECK(!made_send_attempt_);
+  test_clock_ = clock;
+  Reset(base::Bind(base::DoNothing));
+}
+
+void CertificateReportingService::SetMaxReportAgeForTesting(
+    base::TimeDelta max_report_age) {
+  DCHECK(!made_send_attempt_);
+  max_report_age_ = max_report_age;
+  Reset(base::Bind(base::DoNothing));
+}
+
+void CertificateReportingService::Reset(
+    const base::Callback<void()>& callback) {
+  content::BrowserThread::PostTaskAndReply(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CertificateReportingService::ResetOnIOThread,
+                 base::Unretained(this), enabled_, url_request_context_,
+                 max_queued_report_count_, max_report_age_, test_clock_,
+                 event_observer_.get()),
+      callback);
+}
+
+void CertificateReportingService::ResetOnIOThread(
+    bool enabled,
+    net::URLRequestContext* url_request_context,
+    size_t max_queued_report_count,
+    base::TimeDelta max_report_age,
+    base::Clock* test_clock,
+    EventObserver* event_observer) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
+  // url_request_context_ is null during shutdown.
+  if (!enabled || !url_request_context) {
+    reporter_.reset(nullptr);
+    return;
+  }
+  reporter_.reset(new Reporter(
+      std::unique_ptr<certificate_reporting::ErrorReporter>(
+          new certificate_reporting::ErrorReporter(
+              url_request_context, GURL(kExtendedReportingUploadUrlInsecure),
+              net::ReportSender::DO_NOT_SEND_COOKIES)),
+      std::unique_ptr<BoundedReportList>(
+          new BoundedReportList(max_queued_report_count)),
+      test_clock, max_report_age, event_observer, true /* retries_enabled */));
 }