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

This will simplify use of the API for consumers who don't know a
Profile* at the time of getting the callback.

BUG=none
R=robertshield@chromium.org

Review URL: https://codereview.chromium.org/891793002

Cr-Commit-Position: refs/heads/master@{#314328}

chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h

-// Copyright 2014 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.
-
-#ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_ADD_INCIDENT_CALLBACK_H_
-#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_ADD_INCIDENT_CALLBACK_H_
-
-#include "base/callback_forward.h"
-#include "base/memory/scoped_ptr.h"
-
-namespace safe_browsing {
-
-class Incident;
-
-// A callback used by external components to add an incident to the incident
-// reporting service.
-typedef base::Callback<void(scoped_ptr<Incident>)> AddIncidentCallback;
-
-}  // namespace safe_browsing
-
-#endif  // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_ADD_INCIDENT_CALLBACK_H_

chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/safe_browsing/binary_feature_extractor.h"
 #include "chrome/browser/safe_browsing/incident_reporting/binary_integrity_incident.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 
@@ -51,7 +52,7 @@ void RegisterBinaryIntegrityAnalysis() {
 #endif
 }
 
-void VerifyBinaryIntegrity(const AddIncidentCallback& callback) {
+void VerifyBinaryIntegrity(scoped_ptr<IncidentReceiver> incident_receiver) {
   scoped_refptr<BinaryFeatureExtractor> binary_feature_extractor(
       new BinaryFeatureExtractor());
 
@@ -78,8 +79,8 @@ void VerifyBinaryIntegrity(const AddIncidentCallback& callback) {
       incident->set_allocated_signature(signature_info.release());
 
       // Send the report.
-      callback.Run(make_scoped_ptr(
-          new BinaryIntegrityIncident(incident.Pass())));
+      incident_receiver->AddIncidentForProcess(
+          make_scoped_ptr(new BinaryIntegrityIncident(incident.Pass())));
     }
   }
 }

chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer.h

@@ -7,7 +7,7 @@
 
 #include <vector>
 
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
+#include "base/memory/scoped_ptr.h"
 
 namespace base {
 class FilePath;
@@ -15,6 +15,8 @@ class FilePath;
 
 namespace safe_browsing {
 
+class IncidentReceiver;
+
 // Registers a process-wide analysis with the incident reporting service that
 // will verify the signature of the most critical binaries used by Chrome. It
 // will send an incident report every time a signature verification fails.
@@ -22,7 +24,7 @@ void RegisterBinaryIntegrityAnalysis();
 
 // Callback to pass to the incident reporting service. The incident reporting
 // service will decide when to start the analysis.
-void VerifyBinaryIntegrity(const AddIncidentCallback& callback);
+void VerifyBinaryIntegrity(scoped_ptr<IncidentReceiver> incident_receiver);
 
 // Returns a vector containing the paths to all the binaries to verify.
 std::vector<base::FilePath> GetCriticalBinariesPath();

chrome/browser/safe_browsing/incident_reporting/binary_integrity_analyzer_win_unittest.cc

@@ -11,12 +11,16 @@
 #include "base/path_service.h"
 #include "base/test/scoped_path_override.h"
 #include "chrome/browser/safe_browsing/incident_reporting/incident.h"
+#include "chrome/browser/safe_browsing/incident_reporting/mock_incident_receiver.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "version.h"  // NOLINT
 
+using ::testing::_;
+using ::testing::StrictMock;
+
 namespace safe_browsing {
 
 namespace {
@@ -45,18 +49,13 @@ class BinaryIntegrityAnalyzerWinTest : public ::testing::Test {
  public:
   BinaryIntegrityAnalyzerWinTest();
 
-  void OnAddIncident(scoped_ptr<Incident> incident);
-
  protected:
-  bool callback_called_;
   base::FilePath test_data_dir_;
   base::ScopedTempDir temp_dir_;
   scoped_ptr<base::ScopedPathOverride> exe_dir_override_;
-  scoped_ptr<Incident> incident_;
 };
 
-BinaryIntegrityAnalyzerWinTest::BinaryIntegrityAnalyzerWinTest()
-    : callback_called_(false) {
+BinaryIntegrityAnalyzerWinTest::BinaryIntegrityAnalyzerWinTest() {
   temp_dir_.CreateUniqueTempDir();
   base::CreateDirectory(temp_dir_.path().AppendASCII(CHROME_VERSION_STRING));
 
@@ -69,16 +68,6 @@ BinaryIntegrityAnalyzerWinTest::BinaryIntegrityAnalyzerWinTest()
       new base::ScopedPathOverride(base::DIR_EXE, temp_dir_.path()));
 }
 
-// Mock the AddIncidentCallback so we can test that VerifyBinaryIntegrity
-// adds an incident callback when a signature verification fails.
-void BinaryIntegrityAnalyzerWinTest::OnAddIncident(
-    scoped_ptr<Incident> incident) {
-  callback_called_ = true;
-
-  // Take ownership of the incident so that the text fixture can inspect it.
-  incident_ = incident.Pass();
-}
-
 TEST_F(BinaryIntegrityAnalyzerWinTest, GetCriticalBinariesPath) {
   // Expected paths.
   std::vector<base::FilePath> critical_binaries_path_expected;
@@ -112,20 +101,22 @@ TEST_F(BinaryIntegrityAnalyzerWinTest, VerifyBinaryIntegrity) {
 
   ASSERT_TRUE(base::CopyFile(signed_binary_path, chrome_elf_path));
 
-  AddIncidentCallback callback = base::Bind(
-      &BinaryIntegrityAnalyzerWinTest::OnAddIncident, base::Unretained(this));
-
-  VerifyBinaryIntegrity(callback);
-  ASSERT_FALSE(callback_called_);
+  scoped_ptr<MockIncidentReceiver> mock_receiver(
+      new StrictMock<MockIncidentReceiver>());
+  VerifyBinaryIntegrity(mock_receiver.Pass());
 
   ASSERT_TRUE(EraseFileContent(chrome_elf_path));
 
-  VerifyBinaryIntegrity(callback);
-  ASSERT_TRUE(callback_called_);
+  mock_receiver.reset(new MockIncidentReceiver());
+  scoped_ptr<Incident> incident;
+  EXPECT_CALL(*mock_receiver, DoAddIncidentForProcess(_))
+      .WillOnce(TakeIncident(&incident));
+
+  VerifyBinaryIntegrity(mock_receiver.Pass());
 
   // Verify that the incident report contains the expected data.
   scoped_ptr<ClientIncidentReport_IncidentData> incident_data(
-      incident_->TakePayload());
+      incident->TakePayload());
   ASSERT_TRUE(incident_data->has_binary_integrity());
   ASSERT_TRUE(incident_data->binary_integrity().has_file_basename());
   ASSERT_EQ("chrome_elf.dll",

chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer.cc

@@ -5,7 +5,7 @@
 #include "chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer.h"
 
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 
 namespace safe_browsing {
@@ -21,7 +21,7 @@ void RegisterBlacklistLoadAnalysis() {
 }
 
 #if !defined(OS_WIN)
-void VerifyBlacklistLoadState(const AddIncidentCallback& callback) {
+void VerifyBlacklistLoadState(scoped_ptr<IncidentReceiver> incident_receiver) {
 }
 
 bool GetLoadedBlacklistedModules(std::vector<base::string16>* module_names) {

chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer.h

@@ -7,11 +7,13 @@
 
 #include <vector>
 
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
 
 namespace safe_browsing {
 
+class IncidentReceiver;
+
 // Registers a process-wide analysis with the incident reporting service that
 // will examine how effective the blacklist was.
 void RegisterBlacklistLoadAnalysis();
@@ -22,7 +24,7 @@ bool GetLoadedBlacklistedModules(std::vector<base::string16>* module_names);
 
 // Callback to pass to the incident reporting service. The incident reporting
 // service will decide when to start the analysis.
-void VerifyBlacklistLoadState(const AddIncidentCallback& callback);
+void VerifyBlacklistLoadState(scoped_ptr<IncidentReceiver> incident_receiver);
 
 }  // namespace safe_browsing
 

chrome/browser/safe_browsing/incident_reporting/blacklist_load_analyzer_win.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/install_verification/win/module_verification_common.h"
 #include "chrome/browser/safe_browsing/binary_feature_extractor.h"
 #include "chrome/browser/safe_browsing/incident_reporting/blacklist_load_incident.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
 #include "chrome/browser/safe_browsing/path_sanitizer.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
@@ -43,7 +44,7 @@ bool GetLoadedBlacklistedModules(std::vector<base::string16>* module_names) {
   return true;
 }
 
-void VerifyBlacklistLoadState(const AddIncidentCallback& callback) {
+void VerifyBlacklistLoadState(scoped_ptr<IncidentReceiver> incident_receiver) {
   std::vector<base::string16> module_names;
   if (GetLoadedBlacklistedModules(&module_names)) {
     PathSanitizer path_sanitizer;
@@ -95,7 +96,7 @@ void VerifyBlacklistLoadState(const AddIncidentCallback& callback) {
           module_path, blacklist_load->mutable_image_headers());
 
       // Send the report.
-      callback.Run(
+      incident_receiver->AddIncidentForProcess(
           make_scoped_ptr(new BlacklistLoadIncident(blacklist_load.Pass())));
     }
   }

chrome/browser/safe_browsing/incident_reporting/delayed_analysis_callback.h

@@ -6,15 +6,17 @@
 #define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_DELAYED_ANALYSIS_CALLBACK_H_
 
 #include "base/callback_forward.h"
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
+#include "base/memory/scoped_ptr.h"
 
 namespace safe_browsing {
 
+class IncidentReceiver;
+
 // A callback used by external components to register a process-wide analysis
 // step. The callback will be run after some delay following process launch in
-// the blocking pool. The argument is a callback by which the consumer can add
+// the blocking pool. The argument is a receiver by which the consumer can add
 // incidents to the incident reporting service.
-typedef base::Callback<void(const AddIncidentCallback&)>
+typedef base::Callback<void(scoped_ptr<IncidentReceiver>)>
     DelayedAnalysisCallback;
 
 }  // namespace safe_browsing

chrome/browser/safe_browsing/incident_reporting/incident_receiver.h

+// Copyright 2015 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.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_INCIDENT_RECEIVER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_INCIDENT_RECEIVER_H_
+
+#include "base/memory/scoped_ptr.h"
+
+class Profile;
+
+namespace safe_browsing {
+
+class Incident;
+
+// An interface by which incidents may be added to the incident reporting
+// service.
+class IncidentReceiver {
+ public:
+  virtual ~IncidentReceiver() {}
+
+  // Adds an incident relating to |profile|. Must be called from the UI thread.
+  virtual void AddIncidentForProfile(Profile* profile,
+                                     scoped_ptr<Incident> incident) = 0;
+
+  // Adds an incident relating to the entire browser process. May be called from
+  // any thread.
+  virtual void AddIncidentForProcess(scoped_ptr<Incident> incident) = 0;
+};
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_INCIDENT_RECEIVER_H_

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 AddIncidentOnMainThread(
+      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());
+  DCHECK(profile);
+  AddIncidentOnMainThread(service_, profile, incident.Pass());
+}
+
+void IncidentReportingService::Receiver::AddIncidentForProcess(
+    scoped_ptr<Incident> incident) {
+  if (thread_runner_->BelongsToCurrentThread()) {
+    AddIncidentOnMainThread(service_, nullptr, incident.Pass());
+  } else if (!thread_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&IncidentReportingService::Receiver::AddIncidentOnMainThread,
+                 service_, nullptr, base::Passed(&incident)))) {
+    LogIncidentDataType(DISCARDED, *incident);
+  }
+}
+
+// static
+void IncidentReportingService::Receiver::AddIncidentOnMainThread(
+    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);

chrome/browser/safe_browsing/incident_reporting/incident_reporting_service.h

@@ -19,7 +19,6 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/safe_browsing/download_protection_service.h"
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
 #include "chrome/browser/safe_browsing/incident_reporting/delayed_analysis_callback.h"
 #include "chrome/browser/safe_browsing/incident_reporting/delayed_callback_runner.h"
 #include "chrome/browser/safe_browsing/incident_reporting/download_metadata_manager.h"
@@ -54,6 +53,8 @@ class ClientIncidentReport;
 class ClientIncidentReport_DownloadDetails;
 class ClientIncidentReport_EnvironmentData;
 class ClientIncidentReport_IncidentData;
+class Incident;
+class IncidentReceiver;
 
 // A class that manages the collection of incidents and submission of incident
 // reports to the safe browsing client-side detection service. The service
@@ -77,17 +78,15 @@ class IncidentReportingService : public content::NotificationObserver {
   // dropped at destruction.
   ~IncidentReportingService() override;
 
-  // Returns a callback by which external components can add an incident to the
-  // service on behalf of |profile|. The callback may outlive the service, but
-  // will no longer have any effect after the service is deleted. The callback
-  // must not be run after |profile| has been destroyed.
-  AddIncidentCallback GetAddIncidentCallback(Profile* profile);
+  // Returns an object by which external components can add an incident to the
+  // service. The object may outlive the service, but will no longer have any
+  // effect after the service is deleted.
+  scoped_ptr<IncidentReceiver> GetIncidentReceiver();
 
   // Returns a preference validation delegate that adds incidents to the service
   // for validation failures in |profile|. The delegate may outlive the service,
   // but incidents reported by it will no longer have any effect after the
-  // service is deleted. The lifetime of the delegate should not extend beyond
-  // that of the profile it services.
+  // service is deleted.
   scoped_ptr<TrackedPreferenceValidationDelegate>
       CreatePreferenceValidationDelegate(Profile* profile);
 
@@ -143,6 +142,7 @@ class IncidentReportingService : public content::NotificationObserver {
  private:
   struct ProfileContext;
   class UploadContext;
+  class Receiver;
 
   // A mapping of profiles to contexts holding state about received incidents.
   typedef std::map<Profile*, ProfileContext*> ProfileContextCollection;
@@ -161,8 +161,7 @@ class IncidentReportingService : public content::NotificationObserver {
   // participating in extended safe browsing are preferred.
   Profile* FindEligibleProfile() const;
 
-  // 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.
+  // Adds |incident_data| relating to the optional |profile| to the service.
   void AddIncident(Profile* profile, scoped_ptr<Incident> incident);
 
   // Begins processing a report. If processing is already underway, ensures that
@@ -321,7 +320,7 @@ class IncidentReportingService : public content::NotificationObserver {
   // Non-NULL while such a search is outstanding.
   scoped_ptr<LastDownloadFinder> last_download_finder_;
 
-  // A factory for handing out weak pointers for AddIncident callbacks.
+  // A factory for handing out weak pointers for IncidentReceiver objects.
   base::WeakPtrFactory<IncidentReportingService> receiver_weak_ptr_factory_;
 
   // A factory for handing out weak pointers for internal asynchronous tasks

chrome/browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc

@@ -17,6 +17,7 @@
 #include "base/threading/thread_local.h"
 #include "chrome/browser/prefs/browser_prefs.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.h"
 #include "chrome/browser/safe_browsing/incident_reporting/last_download_finder.h"
 #include "chrome/browser/safe_browsing/incident_reporting/tracked_preference_incident.h"
@@ -246,7 +247,12 @@ class IncidentReportingServiceTest : public testing::Test {
 
   // Adds a test incident to the service.
   void AddTestIncident(Profile* profile) {
-    instance_->GetAddIncidentCallback(profile).Run(MakeTestIncident(nullptr));
+    scoped_ptr<safe_browsing::IncidentReceiver> receiver(
+        instance_->GetIncidentReceiver());
+    if (profile)
+      receiver->AddIncidentForProfile(profile, MakeTestIncident(nullptr));
+    else
+      receiver->AddIncidentForProcess(MakeTestIncident(nullptr));
   }
 
   // Registers the callback to be run for delayed analysis.
@@ -459,10 +465,10 @@ class IncidentReportingServiceTest : public testing::Test {
   void OnDownloadFinderDestroyed() { download_finder_destroyed_ = true; }
   void OnUploaderDestroyed() { uploader_destroyed_ = true; }
 
-  void OnDelayedAnalysis(const safe_browsing::AddIncidentCallback& callback) {
+  void OnDelayedAnalysis(scoped_ptr<safe_browsing::IncidentReceiver> receiver) {
     delayed_analysis_ran_ = true;
     if (on_delayed_analysis_action_ == ON_DELAYED_ANALYSIS_ADD_INCIDENT)
-      callback.Run(MakeTestIncident(nullptr));
+      receiver->AddIncidentForProcess(MakeTestIncident(nullptr));
   }
 
   // A mapping of profile name to its corresponding properties.
@@ -637,7 +643,8 @@ TEST_F(IncidentReportingServiceTest, TwoIncidentsTwoUploads) {
   ExpectTestIncidentUploaded(1);
 
   // Add a variation on the incident to the service.
-  instance_->GetAddIncidentCallback(profile).Run(MakeTestIncident("leeches"));
+  instance_->GetIncidentReceiver()->AddIncidentForProfile(
+      profile, MakeTestIncident("leeches"));
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -755,9 +762,9 @@ TEST_F(IncidentReportingServiceTest, ProcessWideTwoUploads) {
                 nullptr);
 
   // Add the test incident.
-  safe_browsing::AddIncidentCallback add_incident(
-      instance_->GetAddIncidentCallback(NULL));
-  add_incident.Run(MakeTestIncident(nullptr));
+  scoped_ptr<safe_browsing::IncidentReceiver> receiver(
+      instance_->GetIncidentReceiver());
+  receiver->AddIncidentForProcess(MakeTestIncident(nullptr));
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();
@@ -766,7 +773,7 @@ TEST_F(IncidentReportingServiceTest, ProcessWideTwoUploads) {
   ExpectTestIncidentUploaded(1);
 
   // Add a variation on the incident to the service.
-  add_incident.Run(MakeTestIncident("leeches"));
+  receiver->AddIncidentForProcess(MakeTestIncident("leeches"));
 
   // Let all tasks run.
   task_runner_->RunUntilIdle();

chrome/browser/safe_browsing/incident_reporting/mock_incident_receiver.cc

+// Copyright 2015 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/incident_reporting/mock_incident_receiver.h"
+
+#include "chrome/browser/safe_browsing/incident_reporting/incident.h"
+
+namespace safe_browsing {
+
+MockIncidentReceiver::MockIncidentReceiver() {}
+
+MockIncidentReceiver::~MockIncidentReceiver() {}
+
+}  // namespace safe_browsing

chrome/browser/safe_browsing/incident_reporting/mock_incident_receiver.h

+// Copyright 2015 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.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_MOCK_INCIDENT_RECEIVER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_MOCK_INCIDENT_RECEIVER_H_
+
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace safe_browsing {
+
+class MockIncidentReceiver : public IncidentReceiver {
+ public:
+  MockIncidentReceiver();
+  ~MockIncidentReceiver();
+
+  MOCK_METHOD2(DoAddIncidentForProfile, void(Profile*, scoped_ptr<Incident>*));
+  MOCK_METHOD1(DoAddIncidentForProcess, void(scoped_ptr<Incident>*));
+
+ protected:
+  void AddIncidentForProfile(Profile* profile,
+                             scoped_ptr<Incident> incident) override {
+    DoAddIncidentForProfile(profile, &incident);
+  }
+
+  void AddIncidentForProcess(scoped_ptr<Incident> incident) override {
+    DoAddIncidentForProcess(&incident);
+  }
+};
+
+// An action that passes ownership of the incident in |arg0| to |recipient|.
+ACTION_P(TakeIncident, recipient) {
+  *recipient = arg0->Pass();
+}
+
+// An action that passes ownership of the incident in |arg0| to the vector in
+// |incidents|.
+ACTION_P(TakeIncidentToVector, incidents) {
+  incidents->push_back(arg0->release());
+}
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_MOCK_INCIDENT_RECEIVER_H_

chrome/browser/safe_browsing/incident_reporting/off_domain_inclusion_detector.h

@@ -42,8 +42,8 @@ class OffDomainInclusionDetector {
   };
 
   // TODO(gab): Hook the OffDomainInclusionDetector to the
-  // IncidentReportingService and use an AddIncidentCallback instead of this
-  // custom callback type.
+  // IncidentReportingService and use an IncidentReceiver instead of this custom
+  // callback type.
   typedef base::Callback<void(AnalysisEvent event)> ReportAnalysisEventCallback;
 
   // Constructs an OffDomainInclusionDetector which will use |database_manager|

chrome/browser/safe_browsing/incident_reporting/preference_validation_delegate.cc

@@ -7,10 +7,10 @@
 #include <string>
 #include <vector>
 
-#include "base/callback.h"
 #include "base/json/json_writer.h"
 #include "chrome/browser/prefs/tracked/pref_hash_store_transaction.h"
 #include "chrome/browser/prefs/tracked/tracked_preference_helper.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
 #include "chrome/browser/safe_browsing/incident_reporting/tracked_preference_incident.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 
@@ -41,8 +41,10 @@ TPIncident_ValueState MapValueState(
 }  // namespace
 
 PreferenceValidationDelegate::PreferenceValidationDelegate(
-    const AddIncidentCallback& add_incident)
-    : add_incident_(add_incident) {
+    Profile* profile,
+    scoped_ptr<IncidentReceiver> incident_receiver)
+    : profile_(profile),
+      incident_receiver_(incident_receiver.Pass()) {
 }
 
 PreferenceValidationDelegate::~PreferenceValidationDelegate() {
@@ -64,8 +66,9 @@ void PreferenceValidationDelegate::OnAtomicPreferenceValidation(
       incident->clear_atomic_value();
     }
     incident->set_value_state(proto_value_state);
-    add_incident_.Run(make_scoped_ptr(
-        new TrackedPreferenceIncident(incident.Pass(), is_personal)));
+    incident_receiver_->AddIncidentForProfile(
+        profile_, make_scoped_ptr(new TrackedPreferenceIncident(incident.Pass(),
+                                                                is_personal)));
   }
 }
 
@@ -87,8 +90,9 @@ void PreferenceValidationDelegate::OnSplitPreferenceValidation(
       incident->add_split_key(*scan);
     }
     incident->set_value_state(proto_value_state);
-    add_incident_.Run(make_scoped_ptr(
-        new TrackedPreferenceIncident(incident.Pass(), is_personal)));
+    incident_receiver_->AddIncidentForProfile(
+        profile_, make_scoped_ptr(new TrackedPreferenceIncident(incident.Pass(),
+                                                                is_personal)));
   }
 }
 

chrome/browser/safe_browsing/incident_reporting/preference_validation_delegate.h

@@ -5,21 +5,25 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_PREFERENCE_VALIDATION_DELEGATE_H_
 #define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_PREFERENCE_VALIDATION_DELEGATE_H_
 
-#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "chrome/browser/prefs/tracked/tracked_preference_validation_delegate.h"
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
+
+class Profile;
 
 namespace safe_browsing {
 
+class IncidentReceiver;
+
 // A preference validation delegate that adds incidents to a given receiver
-// for preference validation failures.
+// for preference validation failures. The profile for which the delegate
+// operates must outlive the delegate itself.
 class PreferenceValidationDelegate
     : public TrackedPreferenceValidationDelegate {
  public:
-  explicit PreferenceValidationDelegate(
-      const AddIncidentCallback& add_incident);
+  PreferenceValidationDelegate(Profile* profile,
+                               scoped_ptr<IncidentReceiver> incident_receiver);
   ~PreferenceValidationDelegate() override;
 
  private:
@@ -36,7 +40,8 @@ class PreferenceValidationDelegate
       PrefHashStoreTransaction::ValueState value_state,
       bool is_personal) override;
 
-  AddIncidentCallback add_incident_;
+  Profile* profile_;
+  scoped_ptr<IncidentReceiver> incident_receiver_;
 
   DISALLOW_COPY_AND_ASSIGN(PreferenceValidationDelegate);
 };

chrome/browser/safe_browsing/incident_reporting/preference_validation_delegate_unittest.cc

@@ -12,9 +12,16 @@
 #include "base/memory/scoped_vector.h"
 #include "base/values.h"
 #include "chrome/browser/safe_browsing/incident_reporting/incident.h"
+#include "chrome/browser/safe_browsing/incident_reporting/mock_incident_receiver.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using ::testing::_;
+using ::testing::IsNull;
+using ::testing::NiceMock;
+using ::testing::WithArg;
+
 // A basic test harness that creates a delegate instance for which it stores all
 // incidents. Tests can push data to the delegate and verify that the test
 // instance was provided with the expected data.
@@ -30,13 +37,12 @@ class PreferenceValidationDelegateTest : public testing::Test {
     testing::Test::SetUp();
     invalid_keys_.push_back(std::string("one"));
     invalid_keys_.push_back(std::string("two"));
+    scoped_ptr<safe_browsing::MockIncidentReceiver> receiver(
+        new NiceMock<safe_browsing::MockIncidentReceiver>());
+    ON_CALL(*receiver, DoAddIncidentForProfile(IsNull(), _))
+        .WillByDefault(WithArg<1>(TakeIncidentToVector(&incidents_)));
     instance_.reset(new safe_browsing::PreferenceValidationDelegate(
-        base::Bind(&PreferenceValidationDelegateTest::AddIncident,
-                   base::Unretained(this))));
-  }
-
-  void AddIncident(scoped_ptr<safe_browsing::Incident> incident) {
-    incidents_.push_back(incident.release());
+        nullptr, receiver.Pass()));
   }
 
   static void ExpectValueStatesEquate(

chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.cc

@@ -10,6 +10,7 @@
 #include "base/location.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/variations/variations_service.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
 #include "chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_incident.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
@@ -20,7 +21,7 @@ namespace safe_browsing {
 namespace {
 
 void VerifyVariationsSeedSignatureOnUIThread(
-    const AddIncidentCallback& callback) {
+    scoped_ptr<IncidentReceiver> incident_receiver) {
   chrome_variations::VariationsService* variations_service =
       g_browser_process->variations_service();
   if (!variations_service)
@@ -33,7 +34,7 @@ void VerifyVariationsSeedSignatureOnUIThread(
         variations_seed_signature(
             new ClientIncidentReport_IncidentData_VariationsSeedSignatureIncident());
     variations_seed_signature->set_variations_seed_signature(invalid_signature);
-    callback.Run(make_scoped_ptr(
+    incident_receiver->AddIncidentForProcess(make_scoped_ptr(
         new VariationsSeedSignatureIncident(variations_seed_signature.Pass())));
   }
 }
@@ -48,11 +49,13 @@ void RegisterVariationsSeedSignatureAnalysis() {
       base::Bind(&VerifyVariationsSeedSignature));
 }
 
-void VerifyVariationsSeedSignature(const AddIncidentCallback& callback) {
+void VerifyVariationsSeedSignature(
+    scoped_ptr<IncidentReceiver> incident_receiver) {
   content::BrowserThread::PostTask(
       content::BrowserThread::UI,
       FROM_HERE,
-      base::Bind(&VerifyVariationsSeedSignatureOnUIThread, callback));
+      base::Bind(&VerifyVariationsSeedSignatureOnUIThread,
+                 base::Passed(&incident_receiver)));
 }
 
 }  // namespace safe_browsing

chrome/browser/safe_browsing/incident_reporting/variations_seed_signature_analyzer.h

@@ -5,18 +5,21 @@
 #ifndef CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_ANALYZER_H_
 #define CHROME_BROWSER_SAFE_BROWSING_INCIDENT_REPORTING_VARIATIONS_SEED_SIGNATURE_ANALYZER_H_
 
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
-#include "chrome/browser/safe_browsing/incident_reporting/add_incident_callback.h"
 
 namespace safe_browsing {
 
+class IncidentReceiver;
+
 // Registers a process-wide analysis with the incident reporting service that
 // will verify the variations seed signature.
 void RegisterVariationsSeedSignatureAnalysis();
 
 // Callback to pass to the incident reporting service. The incident reporting
 // service will verify if the variations seed signature is invalid.
-void VerifyVariationsSeedSignature(const AddIncidentCallback& callback);
+void VerifyVariationsSeedSignature(
+    scoped_ptr<IncidentReceiver> incident_receiver);
 
 }  // namespace safe_browsing
 

chrome/chrome_browser.gypi

@@ -2286,7 +2286,6 @@
       'browser/safe_browsing/download_feedback_service.h',
       'browser/safe_browsing/download_protection_service.cc',
       'browser/safe_browsing/download_protection_service.h',
-      'browser/safe_browsing/incident_reporting/add_incident_callback.h',
       'browser/safe_browsing/incident_reporting/binary_integrity_analyzer.cc',
       'browser/safe_browsing/incident_reporting/binary_integrity_analyzer.h',
       'browser/safe_browsing/incident_reporting/binary_integrity_analyzer_win.cc',
@@ -2310,6 +2309,7 @@
       'browser/safe_browsing/incident_reporting/incident.h',
       'browser/safe_browsing/incident_reporting/incident_handler_util.cc',
       'browser/safe_browsing/incident_reporting/incident_handler_util.h',
+      'browser/safe_browsing/incident_reporting/incident_receiver.h',
       'browser/safe_browsing/incident_reporting/incident_report_uploader.cc',
       'browser/safe_browsing/incident_reporting/incident_report_uploader.h',
       'browser/safe_browsing/incident_reporting/incident_report_uploader_impl.cc',

chrome/chrome_tests_unit.gypi

@@ -1009,6 +1009,8 @@
       'browser/safe_browsing/incident_reporting/incident_report_uploader_impl_unittest.cc',
       'browser/safe_browsing/incident_reporting/incident_reporting_service_unittest.cc',
       'browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc',
+      'browser/safe_browsing/incident_reporting/mock_incident_receiver.cc',
+      'browser/safe_browsing/incident_reporting/mock_incident_receiver.h',
       'browser/safe_browsing/incident_reporting/module_integrity_unittest_util_win.cc',
       'browser/safe_browsing/incident_reporting/module_integrity_unittest_util_win.h',
       'browser/safe_browsing/incident_reporting/module_integrity_verifier_win_unittest.cc',