Take a Profile when adding an incident to the incident reporting service.

chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc

Index: chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
diff --git a/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc b/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
index f5bafc5eee4fcd780873baaa5c2bcdadf1ca0065..8c5c80d6e3e0bd407f1e437caa1474e27a5770dd 100644
--- a/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
+++ b/chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.cc
@@ -26,6 +26,7 @@
 #include "chrome/browser/safe_browsing/database_manager.h"
 #include "chrome/browser/safe_browsing/incident_reporting/environment_data_collection.h"
 #include "chrome/browser/safe_browsing/incident_reporting/incident.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
 #include "chrome/browser/safe_browsing/incident_reporting/incident_report_uploader_impl.h"
 #include "chrome/browser/safe_browsing/incident_reporting/preference_validation_delegate.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
@@ -166,19 +167,6 @@ void CleanLegacyPruneState(Profile* profile) {
     pref_update.Get()->RemoveWithoutPathExpansion(incident_type, NULL);
 }
 
-// Runs |callback| on the thread to which |thread_runner| belongs. The callback
-// is run immediately if this function is called on |thread_runner|'s thread.
-void AddIncidentOnOriginThread(
-    const AddIncidentCallback& callback,
-    scoped_refptr<base::SingleThreadTaskRunner> thread_runner,
-    scoped_ptr<Incident> incident) {
-  if (thread_runner->BelongsToCurrentThread())
-    callback.Run(incident.Pass());
-  else
-    thread_runner->PostTask(FROM_HERE,
-                            base::Bind(callback, base::Passed(&incident)));
-}
-
 }  // namespace
 
 struct IncidentReportingService::ProfileContext {
@@ -217,6 +205,70 @@ class IncidentReportingService::UploadContext {
   DISALLOW_COPY_AND_ASSIGN(UploadContext);
 };
 
+// An IncidentReceiver that is weakly-bound to the service and transparently
+// bounces process-wide incidents back to the main thread for handling.
+class IncidentReportingService::Receiver : public IncidentReceiver {
+ public:
+  explicit Receiver(const base::WeakPtr<IncidentReportingService>& service);
+  ~Receiver() override;
+
+  // IncidentReceiver methods:
+  void AddIncidentForProfile(Profile* profile,
+                             scoped_ptr<Incident> incident) override;
+  void AddIncidentForProcess(scoped_ptr<Incident> incident) override;
+
+ private:
+  static void AddIncidentOnUIThread(
+      const base::WeakPtr<IncidentReportingService>& service,
+      Profile* profile,
+      scoped_ptr<Incident> incident);
+
+  base::WeakPtr<IncidentReportingService> service_;
+  scoped_refptr<base::SingleThreadTaskRunner> thread_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(Receiver);
+};
+
+IncidentReportingService::Receiver::Receiver(
+    const base::WeakPtr<IncidentReportingService>& service)
+    : service_(service),
+      thread_runner_(base::ThreadTaskRunnerHandle::Get()) {
+}
+
+IncidentReportingService::Receiver::~Receiver() {
+}
+
+void IncidentReportingService::Receiver::AddIncidentForProfile(
+    Profile* profile,
+    scoped_ptr<Incident> incident) {
+  DCHECK(thread_runner_->BelongsToCurrentThread());

[robertshield, 2015/02/03 02:54:42]

For my own education: this DCHECK in practice checks that this is called on the
UI thread, so is there a reason not to check for being on the UI thread
explicitly?  It seems like not doing so (as you've done here) is nicer from a
not-as-coupled-to-specific-threading-model point of view, and yet the static
method is called AddIncident*OnUIThread* which is kind of specific. 

[grt (UTC plus 2), 2015/02/03 03:27:59]

I wish I didn't have to use BrowserThread anywhere in the impl since it leads to
tighter coupling and more weight in unit tests. I just noticed that the comment
for the class (line 208) says "main thread" rather than "UI thread," so I guess
the answer is that sometimes I'm inconsistent. I'll rename the function on line
262 so that it matches the comment and is therefore less jarring.

+  DCHECK(profile);
+  AddIncidentOnUIThread(service_, profile, incident.Pass());
+}
+
+void IncidentReportingService::Receiver::AddIncidentForProcess(
+    scoped_ptr<Incident> incident) {
+  if (thread_runner_->BelongsToCurrentThread()) {
+    AddIncidentOnUIThread(service_, nullptr, incident.Pass());
+  } else if (!thread_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&IncidentReportingService::Receiver::AddIncidentOnUIThread,
+                 service_, nullptr, base::Passed(&incident)))) {
+    LogIncidentDataType(DISCARDED, *incident);
+  }
+}
+
+// static
+void IncidentReportingService::Receiver::AddIncidentOnUIThread(
+    const base::WeakPtr<IncidentReportingService>& service,
+    Profile* profile,
+    scoped_ptr<Incident> incident) {
+  if (service)
+    service->AddIncident(profile, incident.Pass());
+  else
+    LogIncidentDataType(DISCARDED, *incident);
+}
+
 IncidentReportingService::ProfileContext::ProfileContext() : added() {
 }
 
@@ -293,16 +345,8 @@ IncidentReportingService::~IncidentReportingService() {
   STLDeleteValues(&profiles_);
 }
 
-AddIncidentCallback IncidentReportingService::GetAddIncidentCallback(
-    Profile* profile) {
-  // Force the context to be created so that incidents added before
-  // OnProfileAdded is called are held until the profile's preferences can be
-  // queried.
-  ignore_result(GetOrCreateProfileContext(profile));
-
-  return base::Bind(&IncidentReportingService::AddIncident,
-                    receiver_weak_ptr_factory_.GetWeakPtr(),
-                    profile);
+scoped_ptr<IncidentReceiver> IncidentReportingService::GetIncidentReceiver() {
+  return make_scoped_ptr(new Receiver(receiver_weak_ptr_factory_.GetWeakPtr()));
 }
 
 scoped_ptr<TrackedPreferenceValidationDelegate>
@@ -312,21 +356,17 @@ IncidentReportingService::CreatePreferenceValidationDelegate(Profile* profile) {
   if (profile->IsOffTheRecord())
     return scoped_ptr<TrackedPreferenceValidationDelegate>();
   return scoped_ptr<TrackedPreferenceValidationDelegate>(
-      new PreferenceValidationDelegate(GetAddIncidentCallback(profile)));
+      new PreferenceValidationDelegate(profile, GetIncidentReceiver()));
 }
 
 void IncidentReportingService::RegisterDelayedAnalysisCallback(
     const DelayedAnalysisCallback& callback) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // |callback| will be run on the blocking pool, so it will likely run the
-  // AddIncidentCallback there as well. Bounce the run of that callback back to
-  // the current thread via AddIncidentOnOriginThread.
+  // |callback| will be run on the blocking pool. The receiver will bounce back
+  // to the origin thread if needed.
   delayed_analysis_callbacks_.RegisterCallback(
-      base::Bind(callback,
-                 base::Bind(&AddIncidentOnOriginThread,
-                            GetAddIncidentCallback(NULL),
-                            base::ThreadTaskRunnerHandle::Get())));
+      base::Bind(callback, base::Passed(GetIncidentReceiver())));
 
   // Start running the callbacks if any profiles are participating in safe
   // browsing. If none are now, running will commence if/when a participaing
@@ -518,9 +558,11 @@ void IncidentReportingService::AddIncident(Profile* profile,
                                            scoped_ptr<Incident> incident) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  ProfileContext* context = GetProfileContext(profile);
-  // It is forbidden to call this function with a destroyed profile.
-  DCHECK(context);
+  // Ignore incidents from off-the-record profiles.
+  if (profile && profile->IsOffTheRecord())
+    return;
+
+  ProfileContext* context = GetOrCreateProfileContext(profile);
   // If this is a process-wide incident, the context must not indicate that the
   // profile (which is NULL) has been added to the profile manager.
   DCHECK(profile || !context->added);