Support for process-wide incidents in the safe browsing incident reporting service.

BUG=399428
R=robertshield@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288028 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/safe_browsing/delayed_analysis_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_DELAYED_ANALYSIS_CALLBACK_H_
+#define CHROME_BROWSER_SAFE_BROWSING_DELAYED_ANALYSIS_CALLBACK_H_
+
+#include "base/callback_forward.h"
+#include "chrome/browser/safe_browsing/add_incident_callback.h"
+
+namespace safe_browsing {
+
+// 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
+// incidents to the incident reporting service.
+typedef base::Callback<void(const AddIncidentCallback&)>
+    DelayedAnalysisCallback;
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_DELAYED_ANALYSIS_CALLBACK_H_

chrome/browser/safe_browsing/delayed_callback_runner.cc

+// 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.
+
+#include "chrome/browser/safe_browsing/delayed_callback_runner.h"
+
+#include "base/location.h"
+
+namespace safe_browsing {
+
+DelayedCallbackRunner::DelayedCallbackRunner(
+    base::TimeDelta delay,
+    const scoped_refptr<base::TaskRunner>& task_runner)
+    : task_runner_(task_runner),
+      next_callback_(callbacks_.end()),
+      timer_(FROM_HERE, delay, this, &DelayedCallbackRunner::OnTimer) {
+}
+
+DelayedCallbackRunner::~DelayedCallbackRunner() {
+}
+
+void DelayedCallbackRunner::RegisterCallback(const base::Closure& callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  callbacks_.push_back(callback);
+}
+
+void DelayedCallbackRunner::Start() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // Nothing to do if the runner is already running or nothing has been added.
+  if (next_callback_ != callbacks_.end() || callbacks_.empty())
+    return;
+
+  // Prime the system with the first callback.
+  next_callback_ = callbacks_.begin();
+
+  // Point the starter pistol in the air and pull the trigger.
+  timer_.Reset();
+}
+
+void DelayedCallbackRunner::OnTimer() {
+  // Run the next callback on the task runner.
+  task_runner_->PostTask(FROM_HERE, *next_callback_);
+
+  // Remove this callback and get ready for the next if there is one.
+  next_callback_ = callbacks_.erase(next_callback_);
+  if (next_callback_ != callbacks_.end())
+    timer_.Reset();
+}
+
+}  // namespace safe_browsing

chrome/browser/safe_browsing/delayed_callback_runner.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_DELAYED_CALLBACK_RUNNER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_DELAYED_CALLBACK_RUNNER_H_
+
+#include <list>
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/task_runner.h"
+#include "base/threading/thread_checker.h"
+#include "base/time/time.h"
+#include "base/timer/timer.h"
+
+namespace safe_browsing {
+
+// Runs callbacks on a given task runner, waiting a certain amount of time
+// between each. The delay also applies to running the first callback (i.e.,
+// the first callback will be run some time after Start() is invoked). Callbacks
+// are deleted after they are run. Start() is idempotent: calling it while the
+// runner is doing its job has no effect.
+class DelayedCallbackRunner {
+ public:
+  // Constructs an instance that runs tasks on |callback_runner|, waiting for
+  // |delay| time to pass before and between each callback.
+  DelayedCallbackRunner(base::TimeDelta delay,
+                        const scoped_refptr<base::TaskRunner>& task_runner);
+  ~DelayedCallbackRunner();
+
+  // Registers |callback| with the runner. A copy of |callback| is held until it
+  // is run.
+  void RegisterCallback(const base::Closure& callback);
+
+  // Starts running the callbacks after the delay.
+  void Start();
+
+ private:
+  typedef std::list<base::Closure> CallbackList;
+
+  // A callback invoked by the timer to run the next callback. The timer is
+  // restarted to process the next callback if there is one.
+  void OnTimer();
+
+  base::ThreadChecker thread_checker_;
+
+  // The runner on which callbacks are to be run.
+  scoped_refptr<base::TaskRunner> task_runner_;
+
+  // The list of callbacks to run. Callbacks are removed when run.
+  CallbackList callbacks_;
+
+  // callbacks_.end() when no work is being done. Any other value otherwise.
+  CallbackList::iterator next_callback_;
+
+  // A timer upon the firing of which the next callback will be run.
+  base::DelayTimer<DelayedCallbackRunner> timer_;
+
+  DISALLOW_COPY_AND_ASSIGN(DelayedCallbackRunner);
+};
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_DELAYED_CALLBACK_RUNNER_H_

chrome/browser/safe_browsing/delayed_callback_runner_unittest.cc

+// 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.
+
+#include "chrome/browser/safe_browsing/delayed_callback_runner.h"
+
+#include <map>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "base/test/test_simple_task_runner.h"
+#include "base/thread_task_runner_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+// A class of objects that invoke a callback upon destruction. This is used as
+// an owned argument on callbacks given to a DelayedCallbackRunner under test.
+class CallbackArgument {
+ public:
+  explicit CallbackArgument(const base::Closure& on_delete)
+      : on_delete_(on_delete) {}
+  ~CallbackArgument() { on_delete_.Run(); }
+
+ private:
+  base::Closure on_delete_;
+
+  DISALLOW_COPY_AND_ASSIGN(CallbackArgument);
+};
+
+}  // namespace
+
+// A test fixture that prepares a DelayedCallbackRunner instance for use and
+// tracks the lifecycle of callbacks sent to it.
+class DelayedCallbackRunnerTest : public testing::Test {
+ public:
+  // Registers a callback that will record its running and destruction to the
+  // test fixture under the given name.
+  void RegisterTestCallback(const std::string& name) {
+    callbacks_[name] = CallbackState();
+    instance_->RegisterCallback(MakeCallback(name));
+  }
+
+ protected:
+  DelayedCallbackRunnerTest()
+      : task_runner_(new base::TestSimpleTaskRunner),
+        thread_task_runner_handle_(task_runner_) {}
+
+  virtual void SetUp() OVERRIDE {
+    instance_.reset(new safe_browsing::DelayedCallbackRunner(
+        base::TimeDelta::FromMilliseconds(1),  // ignored by simple runner.
+        task_runner_));
+  }
+
+  virtual void TearDown() OVERRIDE { instance_.reset(); }
+
+  void OnRun(const std::string& name, CallbackArgument* arg) {
+    EXPECT_FALSE(callbacks_[name].run);
+    callbacks_[name].run = true;
+  }
+
+  void OnDelete(const std::string& name) {
+    EXPECT_FALSE(callbacks_[name].deleted);
+    callbacks_[name].deleted = true;
+  }
+
+  // Returns a callback argument that calls the test fixture's OnDelete method
+  // on behalf of the given callback name.
+  scoped_ptr<CallbackArgument> MakeCallbackArgument(const std::string& name) {
+    return make_scoped_ptr(new CallbackArgument(base::Bind(
+        &DelayedCallbackRunnerTest::OnDelete, base::Unretained(this), name)));
+  }
+
+  // Returns a closure that calls |OnRun| when run and |OnDelete| when deleted
+  // on behalf of the given callback name.
+  base::Closure MakeCallback(const std::string& name) {
+    return base::Bind(&DelayedCallbackRunnerTest::OnRun,
+                      base::Unretained(this),
+                      name,
+                      base::Owned(MakeCallbackArgument(name).release()));
+  }
+
+  bool CallbackWasRun(const std::string& name) { return callbacks_[name].run; }
+
+  bool CallbackWasDeleted(const std::string& name) {
+    return callbacks_[name].deleted;
+  }
+
+  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
+  base::ThreadTaskRunnerHandle thread_task_runner_handle_;
+  scoped_ptr<safe_browsing::DelayedCallbackRunner> instance_;
+
+ private:
+  struct CallbackState {
+    CallbackState() : run(), deleted() {}
+    bool run;
+    bool deleted;
+  };
+
+  std::map<std::string, CallbackState> callbacks_;
+};
+
+// Tests that a callback is deleted when not run before the runner is destroyed.
+TEST_F(DelayedCallbackRunnerTest, NotRunDeleted) {
+  const std::string name("one");
+  RegisterTestCallback(name);
+  instance_.reset();
+  EXPECT_FALSE(CallbackWasRun(name));
+  EXPECT_TRUE(CallbackWasDeleted(name));
+}
+
+// Tests that a callback is run and deleted while the runner is alive.
+TEST_F(DelayedCallbackRunnerTest, RunDeleted) {
+  const std::string name("one");
+  RegisterTestCallback(name);
+  instance_->Start();
+  task_runner_->RunUntilIdle();
+  EXPECT_TRUE(CallbackWasRun(name));
+  EXPECT_TRUE(CallbackWasDeleted(name));
+}
+
+// Tests that a callback registered after Start() is called is also run and
+// deleted.
+TEST_F(DelayedCallbackRunnerTest, AddWhileRunningRun) {
+  const std::string name("one");
+  const std::string name2("two");
+
+  // Post a task to register a new callback after Start() is called.
+  task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&DelayedCallbackRunnerTest::RegisterTestCallback,
+                 base::Unretained(this),
+                 name2));
+
+  RegisterTestCallback(name);
+  instance_->Start();
+  task_runner_->RunUntilIdle();
+  EXPECT_TRUE(CallbackWasRun(name));
+  EXPECT_TRUE(CallbackWasDeleted(name));
+  EXPECT_TRUE(CallbackWasRun(name2));
+  EXPECT_TRUE(CallbackWasDeleted(name2));
+}
+
+TEST_F(DelayedCallbackRunnerTest, MultipleRuns) {
+  const std::string name("one");
+  const std::string name2("two");
+
+  RegisterTestCallback(name);
+  instance_->Start();
+  task_runner_->RunUntilIdle();
+  EXPECT_TRUE(CallbackWasRun(name));
+  EXPECT_TRUE(CallbackWasDeleted(name));
+
+  RegisterTestCallback(name2);
+  instance_->Start();
+  task_runner_->RunUntilIdle();
+  EXPECT_TRUE(CallbackWasRun(name2));
+  EXPECT_TRUE(CallbackWasDeleted(name2));
+}

chrome/browser/safe_browsing/incident_reporting_service.h

@@ -19,6 +19,8 @@
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/safe_browsing/add_incident_callback.h"
+#include "chrome/browser/safe_browsing/delayed_analysis_callback.h"
+#include "chrome/browser/safe_browsing/delayed_callback_runner.h"
 #include "chrome/browser/safe_browsing/incident_report_uploader.h"
 #include "chrome/browser/safe_browsing/last_download_finder.h"
 #include "content/public/browser/notification_observer.h"
@@ -54,11 +56,13 @@ class ClientIncidentReport_IncidentData;
 // begins operation when an incident is reported via the AddIncident method.
 // 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, finds the most recent
-// binary download, 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.
+// in safe browsing are dropped. Process-wide incidents are affiliated with a
+// profile that participates in safe browsing when one becomes available.
+// Following the addition of an incident that is not dropped, the service
+// collects environmental data, finds the most recent binary download, 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 : public content::NotificationObserver {
  public:
   IncidentReportingService(SafeBrowsingService* safe_browsing_service,
@@ -83,11 +87,22 @@ class IncidentReportingService : public content::NotificationObserver {
   scoped_ptr<TrackedPreferenceValidationDelegate>
       CreatePreferenceValidationDelegate(Profile* profile);
 
+  // Registers |callback| to be run after some delay following process launch.
+  void RegisterDelayedAnalysisCallback(const DelayedAnalysisCallback& callback);
+
  protected:
   // A pointer to a function that populates a protobuf with environment data.
   typedef void (*CollectEnvironmentDataFn)(
       ClientIncidentReport_EnvironmentData*);
 
+  // For testing so that the TaskRunner used for delayed analysis callbacks can
+  // be specified.
+  IncidentReportingService(
+      SafeBrowsingService* safe_browsing_service,
+      const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
+      base::TimeDelta delayed_task_interval,
+      const scoped_refptr<base::TaskRunner>& delayed_task_runner);
+
   // Sets the function called by the service to collect environment data and the
   // task runner on which it is called. Used by unit tests to provide a fake
   // environment data collector.
@@ -131,11 +146,23 @@ class IncidentReportingService : public content::NotificationObserver {
   // but not yet uploaded are dropped.
   void OnProfileDestroyed(Profile* profile);
 
+  // Returns an initialized profile that participates in safe browsing. Profiles
+  // 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.
   void AddIncident(Profile* profile,
                    scoped_ptr<ClientIncidentReport_IncidentData> incident_data);
 
+  // Begins processing a report. If processing is already underway, ensures that
+  // collection tasks have completed or are running.
+  void BeginReportProcessing();
+
+  // Begins the process of collating incidents by waiting for incidents to
+  // arrive. This function is idempotent.
+  void BeginIncidentCollation();
+
   // Starts a task to collect environment data in the blocking pool.
   void BeginEnvironmentCollection();
 
@@ -159,10 +186,11 @@ class IncidentReportingService : public content::NotificationObserver {
   // Cancels the collection timeout.
   void CancelIncidentCollection();
 
-  // A callback invoked on the UI thread after which incident collection has
+  // A callback invoked on the UI thread after which incident collation has
   // completed. Incident report processing continues, either by waiting for
-  // environment data to arrive or by sending an incident report.
-  void OnCollectionTimeout();
+  // environment data or the most recent download to arrive or by sending an
+  // incident report.
+  void OnCollationTimeout();
 
   // Starts the asynchronous process of finding the most recent executable
   // download if one is not currently being search for and/or has not already
@@ -234,12 +262,12 @@ class IncidentReportingService : public content::NotificationObserver {
   bool environment_collection_pending_;
 
   // True when an incident has been received and the service is waiting for the
-  // upload_timer_ to fire.
-  bool collection_timeout_pending_;
+  // collation_timer_ to fire.
+  bool collation_timeout_pending_;
 
   // A timer upon the firing of which the service will report received
   // incidents.
-  base::DelayTimer<IncidentReportingService> upload_timer_;
+  base::DelayTimer<IncidentReportingService> collation_timer_;
 
   // The report currently being assembled. This becomes non-NULL when an initial
   // incident is reported, and returns to NULL when the report is sent for
@@ -258,9 +286,13 @@ class IncidentReportingService : public content::NotificationObserver {
   // The time at which download collection was initiated.
   base::TimeTicks last_download_begin_;
 
-  // Context data for all on-the-record profiles.
+  // Context data for all on-the-record profiles plus the process-wide (NULL)
+  // context.
   ProfileContextCollection profiles_;
 
+  // Callbacks registered for performing delayed analysis.
+  DelayedCallbackRunner delayed_analysis_callbacks_;
+
   // The collection of uploads in progress.
   ScopedVector<UploadContext> uploads_;
 

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -332,6 +332,14 @@ SafeBrowsingService::CreatePreferenceValidationDelegate(
   return scoped_ptr<TrackedPreferenceValidationDelegate>();
 }
 
+void SafeBrowsingService::RegisterDelayedAnalysisCallback(
+    const safe_browsing::DelayedAnalysisCallback& callback) {
+#if defined(FULL_SAFE_BROWSING)
+  if (incident_service_)
+    incident_service_->RegisterDelayedAnalysisCallback(callback);
+#endif
+}
+
 SafeBrowsingUIManager* SafeBrowsingService::CreateUIManager() {
   return new SafeBrowsingUIManager(this);
 }

chrome/browser/safe_browsing/safe_browsing_service.h

@@ -17,6 +17,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/sequenced_task_runner_helpers.h"
+#include "chrome/browser/safe_browsing/delayed_analysis_callback.h"
 #include "chrome/browser/safe_browsing/safe_browsing_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
@@ -124,6 +125,12 @@ class SafeBrowsingService
   scoped_ptr<TrackedPreferenceValidationDelegate>
       CreatePreferenceValidationDelegate(Profile* profile) const;
 
+  // Registers |callback| to be run after some delay following process launch.
+  // |callback| will be dropped if the service is not applicable for the
+  // process.
+  void RegisterDelayedAnalysisCallback(
+      const safe_browsing::DelayedAnalysisCallback& callback);
+
  protected:
   // Creates the safe browsing service.  Need to initialize before using.
   SafeBrowsingService();

chrome/chrome_browser.gypi

@@ -2653,6 +2653,9 @@
       'browser/safe_browsing/client_side_detection_service.h',
       'browser/safe_browsing/database_manager.cc',
       'browser/safe_browsing/database_manager.h',
+      'browser/safe_browsing/delayed_analysis_callback.h',
+      'browser/safe_browsing/delayed_callback_runner.cc',
+      'browser/safe_browsing/delayed_callback_runner.h',
       'browser/safe_browsing/download_feedback.cc',
       'browser/safe_browsing/download_feedback.h',
       'browser/safe_browsing/download_feedback_service.cc',

chrome/chrome_tests_unit.gypi

@@ -1205,6 +1205,7 @@
         'browser/safe_browsing/chunk_range_unittest.cc',
         'browser/safe_browsing/client_side_detection_host_unittest.cc',
         'browser/safe_browsing/client_side_detection_service_unittest.cc',
+        'browser/safe_browsing/delayed_callback_runner_unittest.cc',
         'browser/safe_browsing/database_manager_unittest.cc',
         'browser/safe_browsing/download_feedback_service_unittest.cc',
         'browser/safe_browsing/download_feedback_unittest.cc',