Safe browsing incident reporting service improvements.

chrome/browser/safe_browsing/incident_reporting_service.h

Index: chrome/browser/safe_browsing/incident_reporting_service.h
diff --git a/chrome/browser/safe_browsing/incident_reporting_service.h b/chrome/browser/safe_browsing/incident_reporting_service.h
index e8260070786c4265d40c1a4dfd9063477b80af2b..f7287d96137f7480b1fd3c5d6c80fd8c3d8db0df 100644
--- a/chrome/browser/safe_browsing/incident_reporting_service.h
+++ b/chrome/browser/safe_browsing/incident_reporting_service.h
@@ -7,6 +7,8 @@
 
 #include <stdint.h>
 
+#include <map>
+
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
@@ -18,7 +20,10 @@
 #include "base/timer/timer.h"
 #include "chrome/browser/safe_browsing/add_incident_callback.h"
 #include "chrome/browser/safe_browsing/incident_report_uploader.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 
+class Profile;
 class SafeBrowsingDatabaseManager;
 class SafeBrowsingService;
 class TrackedPreferenceValidationDelegate;
@@ -27,6 +32,11 @@ namespace base {
 class TaskRunner;
 }
 
+namespace content {
+class NotificationDetails;
+class NotificationSource;
+}
+
 namespace net {
 class URLRequestContextGetter;
 }
@@ -41,29 +51,31 @@ class ClientIncidentReport_IncidentData;
 // A class that manages the collection of incidents and submission of incident
 // reports to the safe browsing client-side detection service. The service
 // begins operation when an incident is reported via the AddIncident method.
-// Following this, the service collects environmental data and waits a bit.
+// Incidents reported from a profile that is loading are held until the profile
+// is fully created. Incidents originating from profiles that do not participate
+// in safe browsing are dropped. Following the addition of an incident that is
+// not dropped, the service collects environmental data and waits a bit.
 // Additional incidents that arrive during this time are collated with the
 // initial incident. Finally, already-reported incidents are pruned and any
 // remaining are uploaded in an incident report.
-class IncidentReportingService {
+class IncidentReportingService : public content::NotificationObserver {
  public:
   IncidentReportingService(SafeBrowsingService* safe_browsing_service,
                            const scoped_refptr<net::URLRequestContextGetter>&
                                request_context_getter);
-  virtual ~IncidentReportingService();
 
-  // Enables or disables the service. When disabling, incident or data
-  // collection in progress is dropped.
-  void SetEnabled(bool enabled);
+  // All incident collection, data collection, and uploads in progress are
+  // dropped at destruction.
+  virtual ~IncidentReportingService();
 
   // Returns a callback by which external components can add an incident to the
-  // service.
-  AddIncidentCallback GetAddIncidentCallback();
+  // service on behalf of |profile|.

[mattm, 2014/06/18 21:07:18]

comment should mention expectations regarding using the callback and the
lifetime of profile.

[grt (UTC plus 2), 2014/06/19 02:24:39]

Done.

+  AddIncidentCallback GetAddIncidentCallback(Profile* profile);
 
   // Returns a preference validation delegate that adds incidents to the service
-  // for validation failures.
+  // for validation failures in |profile|.

[mattm, 2014/06/18 21:07:18]

same

[grt (UTC plus 2), 2014/06/19 02:24:39]

Done.

   scoped_ptr<TrackedPreferenceValidationDelegate>
-      CreatePreferenceValidationDelegate();
+      CreatePreferenceValidationDelegate(Profile* profile);
 
  protected:
   // A pointer to a function that populates a protobuf with environment data.
@@ -84,11 +96,31 @@ class IncidentReportingService {
       const ClientIncidentReport& report);
 
  private:
+  struct ProfileContext;
   class UploadContext;
 
+  // A mapping of profiles to contexts holding state about received incidents.
+  typedef std::map<Profile*, ProfileContext*> ProfileContextCollection;
+
+  // Returns the context for |profile|, creating it if it does not exist.
+  ProfileContext* GetOrCreateProfileContext(Profile* profile);
+
+  // Returns the context for |profile|, or NULL if it is unknown.
+  ProfileContext* GetProfileContext(Profile* profile);
+
+  // Handles the creation of a new profile. Creates a new context for |profile|
+  // if one does not exist, and drops any received incidents for the profile if
+  // the profile is not participating in safe browsing.
+  void OnProfileCreated(Profile* profile);
+
+  // Handles the destruction of a profile. Incidents reported for the profile
+  // but not yet uploaded are dropped.
+  void OnProfileDestroyed(Profile* profile);
+
   // Adds |incident_data| to the service. The incident_time_msec field is
   // populated with the current time if the caller has not already done so.
-  void AddIncident(scoped_ptr<ClientIncidentReport_IncidentData> incident_data);
+  void AddIncident(Profile* profile,
+                   scoped_ptr<ClientIncidentReport_IncidentData> incident_data);
 
   // Starts a task to collect environment data in the blocking pool.
   void BeginEnvironmentCollection();
@@ -122,7 +154,9 @@ class IncidentReportingService {
   // Prunes incidents that have previously been reported.
   void PruneReportedIncidents(ClientIncidentReport* report);
 
-  // Uploads an incident report if all data collection is complete.
+  // Uploads an incident report if all data collection is complete. Incidents
+  // originating from profiles that do not participate in safe browsing are
+  // dropped.
   void UploadIfCollectionComplete();
 
   // Cancels all uploads, discarding all reports and responses in progress.
@@ -140,6 +174,11 @@ class IncidentReportingService {
                             IncidentReportUploader::Result result,
                             scoped_ptr<ClientIncidentResponse> response);
 
+  // content::NotificationObserver methods.
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
   base::ThreadChecker thread_checker_;
 
   // The safe browsing database manager, through which the whitelist killswitch
@@ -160,8 +199,8 @@ class IncidentReportingService {
   // collection task at shutdown if it has not yet started.
   scoped_refptr<base::TaskRunner> environment_collection_task_runner_;
 
-  // True when the service has been enabled.
-  bool enabled_;
+  // Registrar for observing profile lifecycle notifications.
+  content::NotificationRegistrar notification_registrar_;
 
   // True when the asynchronous environment collection task has been fired off
   // but has not yet completed.
@@ -189,6 +228,9 @@ class IncidentReportingService {
   // The time at which environmental data collection was initiated.
   base::TimeTicks environment_collection_begin_;
 
+  // Context data for all on-the-record profiles.
+  ProfileContextCollection profiles_;
+
   // The collection of uploads in progress.
   ScopedVector<UploadContext> uploads_;