Wire up CertificateReportingService to handle report uploads

chrome/browser/safe_browsing/certificate_reporting_service.h

Index: chrome/browser/safe_browsing/certificate_reporting_service.h
diff --git a/chrome/browser/safe_browsing/certificate_reporting_service.h b/chrome/browser/safe_browsing/certificate_reporting_service.h
index 6c45e7edcbd824aac15229b6d14e4f654e76f401..4a8b3f4497b6cee27f32573d2d0441625fa6c67a 100644
--- a/chrome/browser/safe_browsing/certificate_reporting_service.h
+++ b/chrome/browser/safe_browsing/certificate_reporting_service.h
@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/callback_list.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
@@ -18,6 +19,8 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "net/url_request/url_request_context_getter.h"
 
+class Profile;
+
 namespace base {
 class Clock;
 }
@@ -26,6 +29,10 @@ namespace net {
 class URLRequestContextGetter;
 }
 
+namespace safe_browsing {
+class SafeBrowsingService;
+}
+
 // This service initiates uploads of invalid certificate reports and retries any
 // failed uploads. Each report is retried until it's older than a certain time
 // to live (TTL). Reports older than this TTL are dropped and no more retried,
@@ -34,10 +41,17 @@ class URLRequestContextGetter;
 // Lifetime and dependencies:
 //
 // CertificateReportingService uses the url request context from SafeBrowsing
-// service. SafeBrowsing service is created before CertificateReportingService,
+// service. SafeBrowsingService is created before CertificateReportingService,

[estark, 2016/12/16 23:39:04]

Who/what enforces this?

[meacer, 2016/12/19 23:16:45]

SafeBrowsing is created on first use by the browser main loop through
ChromeContentBrowserClient::ResourceDispatcherHostCreated.

CertificateReportingService also gets created on first use and that happens when
the metrics service uploads a report, or the user sees an SSL error. Both of
them happen after ResourceDispatcherHostCreated gets called.

 // but is also shut down before any KeyedService is shut down. This means that
 // CertificateReportingService cannot depend on SafeBrowsing's url request being
-// available at all times, and it should know when SafeBrowsing shuts down.
+// available at all times, and it should know when SafeBrowsing shuts down. It
+// does this by subscribing to SafeBrowsingService shut downs when it's
+// created. When SafeBrowsingService shuts down, CertificateReportingService
+// also shuts down.
+//
+// This class also observes SafeBrowsing preference changes to enable/disable
+// reporting. It does this by creating a PreferenceObserver that notifies
+// this service of changes in SafeBrowsing and extended reporting preferences.
 class CertificateReportingService : public KeyedService {
  public:
   // Represents a report to be sent.
@@ -129,13 +143,24 @@ class CertificateReportingService : public KeyedService {
     DISALLOW_COPY_AND_ASSIGN(Reporter);
   };
 
+  // Observes SafeBrowsing preference changes.
+  class PreferenceObserver {
+   public:
+    virtual ~PreferenceObserver() {}
+
+    // Called when SafeBrowsing enabled state changes.
+    virtual void OnPreferenceChanged() = 0;
+  };
+
   CertificateReportingService(
+      safe_browsing::SafeBrowsingService* safe_browsing_service,
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
+      Profile* profile,
       uint8_t server_public_key[/* 32 */],
       uint32_t server_public_key_version,
       size_t max_queued_report_count,
       base::TimeDelta max_report_age,
-      std::unique_ptr<base::Clock> clock);
+      base::Clock* clock);
 
   ~CertificateReportingService() override;
 
@@ -154,12 +179,8 @@ class CertificateReportingService : public KeyedService {
   // once the service is initialized.
   void SetEnabled(bool enabled);
 
-  // Getters and setters for testing.
+  // Getters for testing.
   Reporter* GetReporterForTesting() const;
-  void SetMaxQueuedReportCountForTesting(size_t max_report_count);
-  void SetClockForTesting(std::unique_ptr<base::Clock> clock);
-  void SetMaxReportAgeForTesting(base::TimeDelta max_report_age);
-
   static GURL GetReportingURLForTesting();
 
  private:
@@ -194,6 +215,16 @@ class CertificateReportingService : public KeyedService {
   net::URLRequestContext* url_request_context_;
   std::unique_ptr<Reporter> reporter_;
 
+  // Observes SafeBrowsing preference changes (SB is enabled/disabled, extended
+  // reporting is enabled/disabled).
+  std::unique_ptr<PreferenceObserver> preference_observer_;
+
+  // Subscription for url request context shutdowns. When this subscription is
+  // notified, it means SafeBrowsingService is shutting down, and this service
+  // must also shut down.
+  std::unique_ptr<base::CallbackList<void(void)>::Subscription>
+      safe_browsing_service_shutdown_subscription_;
+
   // Maximum number of reports to be queued for retry.
   size_t max_queued_report_count_;
 
@@ -202,11 +233,7 @@ class CertificateReportingService : public KeyedService {
   // this age is ignored and is not re-uploaded.
   base::TimeDelta max_report_age_;
 
-  std::unique_ptr<base::Clock> clock_;
-
-  // Whether a send has ever been made. Used to verify that test setters are
-  // only called after initialization.
-  bool made_send_attempt_;
+  base::Clock* clock_;
 
   // Encryption parameters.
   uint8_t* server_public_key_;