Include the latest binary download info in safe browsing incident reports.

chrome/browser/safe_browsing/incident_reporting_service.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/incident_reporting_service.h"
 
 #include <math.h>
 
 #include <algorithm>
 #include <vector>
@@ skipping 48 matching lines @@
     UMA_HISTOGRAM_ENUMERATION("SBIRS.Incident", type, NUM_INCIDENT_TYPES);
   } else {
     DCHECK_EQ(disposition, DROPPED);
     UMA_HISTOGRAM_ENUMERATION("SBIRS.DroppedIncident", type,
                               NUM_INCIDENT_TYPES);
   }
 }
 
 }  // namespace
 
 struct IncidentReportingService::ProfileContext {

[mattm, 2014/06/27 21:50:32]

Since the IncidentReportingService already tracks profile addition/deletion, I
wonder if it would be cleaner to make LastDownloadFinder only operate on a
single Profile, and create one instance of it per-profile.

[grt (UTC plus 2), 2014/06/30 16:53:38]

I'll explore this to see if it simplifies things. In the meantime, please
consider this change as-is for now since I'd like to get this feature into M37.
Thanks.

[grt (UTC plus 2), 2014/06/30 19:38:37]

I see pros and cons. I'm inclined to keep things as they are, but I'm open to
being convinced otherwise.

- Making the finder profile-specific would remove the dependence on
ProfileManager and the finder's need to observe profile lifecycle notifications.
This is a nice simplification.
- Profile-specific finder would mean more bookkeeping in the incident reporting
service. While the ProfileContext could hold a profile's finder, the service's
WaitingForMostRecentDownload would get more complicated since it would need to
know if any context's finder is outstanding.
- The finder's function that decides if one download is more interesting than
another (IsMoreInterestingThan) would have to be used by each finder instance to
pick the most interesting for that profile and also by the incident reporting
service to pick the most interesting among all downloads. I can think of ways of
sharing this, but I think it's nice that the finder is self-contained at the
moment.
- I'm trying to work out a way for each finder to not need a weak pointer
factory, but so far I'm coming up blank. Without one, I think there's a race if
a profile is destroyed before its finder's call to QueryDownloads returns and a
new one is created at the same memory location. Maybe this can never happen, but
it seems fragile.

I've played around with the code a bit, and so far it seems more complicated to
make the finder per-profile. Let me know what you think.

   ProfileContext();
   ~ProfileContext();
 
   // The incidents collected for this profile pending creation and/or upload.
   ScopedVector<ClientIncidentReport_IncidentData> incidents;
 
-  // False until PROFILE_CREATED notification is received.
-  bool created;
+  // False until PROFILE_ADDED notification is received.
+  bool added;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ProfileContext);
 };
 
 class IncidentReportingService::UploadContext {
  public:
   explicit UploadContext(scoped_ptr<ClientIncidentReport> report);
   ~UploadContext();
 
   // The report being uploaded.
   scoped_ptr<ClientIncidentReport> report;
 
   // The uploader in use. This is NULL until the CSD killswitch is checked.
   scoped_ptr<IncidentReportUploader> uploader;
 
   // The set of profiles from which incidents in |report| originated.
   std::vector<Profile*> profiles;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(UploadContext);
 };
 
-IncidentReportingService::ProfileContext::ProfileContext() : created() {
+IncidentReportingService::ProfileContext::ProfileContext() : added() {
 }
 
 IncidentReportingService::ProfileContext::~ProfileContext() {
 }
 
 IncidentReportingService::UploadContext::UploadContext(
     scoped_ptr<ClientIncidentReport> report)
     : report(report.Pass()) {
 }
 
@@ skipping 13 matching lines @@
                   base::SequencedWorkerPool::SKIP_ON_SHUTDOWN)),
       environment_collection_pending_(),
       collection_timeout_pending_(),
       upload_timer_(FROM_HERE,
                     base::TimeDelta::FromMilliseconds(kDefaultUploadDelayMs),
                     this,
                     &IncidentReportingService::OnCollectionTimeout),
       receiver_weak_ptr_factory_(this),
       weak_ptr_factory_(this) {
   notification_registrar_.Add(this,
-                              chrome::NOTIFICATION_PROFILE_CREATED,
+                              chrome::NOTIFICATION_PROFILE_ADDED,
                               content::NotificationService::AllSources());
   notification_registrar_.Add(this,
                               chrome::NOTIFICATION_PROFILE_DESTROYED,
                               content::NotificationService::AllSources());
 }
 
 IncidentReportingService::~IncidentReportingService() {
   CancelIncidentCollection();
 
   // Cancel all internal asynchronous tasks.
   weak_ptr_factory_.InvalidateWeakPtrs();
 
   CancelEnvironmentCollection();
+  CancelDownloadCollection();
   CancelAllReportUploads();
 
   STLDeleteValues(&profiles_);
 }
 
 AddIncidentCallback IncidentReportingService::GetAddIncidentCallback(
     Profile* profile) {
   // Force the context to be created so that incidents added before
-  // OnProfileCreated is called are held until the profile's preferences can be
+  // 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<TrackedPreferenceValidationDelegate>
 IncidentReportingService::CreatePreferenceValidationDelegate(Profile* profile) {
@@ skipping 13 matching lines @@
     environment_collection_task_runner_ = task_runner;
   } else {
     collect_environment_data_fn_ = &CollectEnvironmentData;
     environment_collection_task_runner_ =
         content::BrowserThread::GetBlockingPool()
             ->GetTaskRunnerWithShutdownBehavior(
                 base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
   }
 }
 
-void IncidentReportingService::OnProfileCreated(Profile* profile) {
+void IncidentReportingService::OnProfileAdded(Profile* profile) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  // Track the addition of all profiles even when no report is being assembled
+  // so that the service can determine whether or not it can evaluate a
+  // profile's preferences at the time of incident addition.
   ProfileContext* context = GetOrCreateProfileContext(profile);
-  context->created = true;
+  context->added = true;
 
-  // Drop all incidents if this profile is not participating in safe browsing.
+  // Nothing else to do if a report is not being assembled.
+  if (!report_)
+    return;
+
+  // Drop all incidents received prior to creation if the profile is not
+  // participating in safe browsing.
   if (!context->incidents.empty() &&
       !profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
     for (size_t i = 0; i < context->incidents.size(); ++i) {
       LogIncidentDataType(DROPPED, *context->incidents[i]);
     }
     context->incidents.clear();
   }
+
+  // Take another stab at finding the most recent download if a report is being
+  // assembled and one hasn't been found yet (the LastDownloadFinder operates
+  // only on profiles that have been added to the ProfileManager).
+  BeginDownloadCollection();
+}
+
+scoped_ptr<LastDownloadFinder> IncidentReportingService::CreateDownloadFinder(
+    const LastDownloadFinder::LastDownloadCallback& callback) {
+  return LastDownloadFinder::Create(callback).Pass();
 }
 
 scoped_ptr<IncidentReportUploader> IncidentReportingService::StartReportUpload(
     const IncidentReportUploader::OnResultCallback& callback,
     const scoped_refptr<net::URLRequestContextGetter>& request_context_getter,
     const ClientIncidentReport& report) {
-#if 0
   return IncidentReportUploaderImpl::UploadReport(
              callback, request_context_getter, report).Pass();
-#else
-  // TODO(grt): Remove this temporary suppression of all uploads.
-  return scoped_ptr<IncidentReportUploader>();
-#endif
 }
 
 IncidentReportingService::ProfileContext*
 IncidentReportingService::GetOrCreateProfileContext(Profile* profile) {
   ProfileContextCollection::iterator it =
       profiles_.insert(ProfileContextCollection::value_type(profile, NULL))
           .first;
   if (!it->second)
     it->second = new ProfileContext();
   return it->second;
@@ skipping 37 matching lines @@
     scoped_ptr<ClientIncidentReport_IncidentData> incident_data) {
   DCHECK(thread_checker_.CalledOnValidThread());
   // Incidents outside the context of a profile are not supported at the moment.
   DCHECK(profile);
 
   ProfileContext* context = GetProfileContext(profile);
   // It is forbidden to call this function with a destroyed profile.
   DCHECK(context);
 
   // Drop the incident immediately if profile creation has completed and the
-  // profile is not participating in safe browsing.
-  if (context->created &&
+  // profile is not participating in safe browsing. Preference evaluation is
+  // deferred until OnProfileAdded() if profile creation has not yet
+  // completed.
+  if (context->added &&
       !profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
     LogIncidentDataType(DROPPED, *incident_data);
     return;
   }
 
-  // Create a new report if this is the first incident ever or first since last
-  // upload.
+  // Start assembling a new report if this is the first incident ever or the
+  // first since the last upload.
   if (!report_) {
     report_.reset(new ClientIncidentReport());
     first_incident_time_ = base::Time::Now();
   }
 
-  // Provide time to the new incident if the caller didn't provide it.
+  // Provide time to the new incident if the caller didn't do so.
   if (!incident_data->has_incident_time_msec())
     incident_data->set_incident_time_msec(base::Time::Now().ToJavaTime());
 
   // Take ownership of the incident.
   context->incidents.push_back(incident_data.release());
 
   if (!last_incident_time_.is_null()) {
     UMA_HISTOGRAM_TIMES("SBIRS.InterIncidentTime",
                         base::TimeTicks::Now() - last_incident_time_);
   }
   last_incident_time_ = base::TimeTicks::Now();
 
   // Persist the incident data.
 
   // Restart the delay timer to send the report upon expiration.
   collection_timeout_pending_ = true;
   upload_timer_.Reset();
 
   BeginEnvironmentCollection();
+  BeginDownloadCollection();
 }
 
 void IncidentReportingService::BeginEnvironmentCollection() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(report_);
+  // Nothing to do if environment collection is pending or has already
+  // completed.
   if (environment_collection_pending_ || report_->has_environment())
     return;
 
   environment_collection_begin_ = base::TimeTicks::Now();
   ClientIncidentReport_EnvironmentData* environment_data =
       new ClientIncidentReport_EnvironmentData();
   environment_collection_pending_ =
       environment_collection_task_runner_->PostTaskAndReply(
           FROM_HERE,
           base::Bind(collect_environment_data_fn_, environment_data),
           base::Bind(&IncidentReportingService::OnEnvironmentDataCollected,
                      weak_ptr_factory_.GetWeakPtr(),
                      base::Passed(make_scoped_ptr(environment_data))));
 
   // Posting the task will fail if the runner has been shut down. This should
   // never happen since the blocking pool is shut down after this service.
   DCHECK(environment_collection_pending_);
 }
 
+bool IncidentReportingService::WaitingForEnvironmentCollection() {
+  return environment_collection_pending_;
+}
+
 void IncidentReportingService::CancelEnvironmentCollection() {
   environment_collection_begin_ = base::TimeTicks();
   environment_collection_pending_ = false;
   if (report_)
     report_->clear_environment();
 }
 
 void IncidentReportingService::OnEnvironmentDataCollected(
     scoped_ptr<ClientIncidentReport_EnvironmentData> environment_data) {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 11 matching lines @@
 
   report_->set_allocated_environment(environment_data.release());
 
   UMA_HISTOGRAM_TIMES("SBIRS.EnvCollectionTime",
                       base::TimeTicks::Now() - environment_collection_begin_);
   environment_collection_begin_ = base::TimeTicks();
 
   UploadIfCollectionComplete();
 }
 
+bool IncidentReportingService::WaitingToCollateIncidents() {
+  return collection_timeout_pending_;
+}
+
 void IncidentReportingService::CancelIncidentCollection() {
   collection_timeout_pending_ = false;
   last_incident_time_ = base::TimeTicks();
   report_.reset();
 }
 
 void IncidentReportingService::OnCollectionTimeout() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Exit early if collection was cancelled.
   if (!collection_timeout_pending_)
     return;
 
   // Wait another round if incidents have come in from a profile that has yet to
   // complete creation.
   for (ProfileContextCollection::iterator scan = profiles_.begin();
        scan != profiles_.end();
        ++scan) {
-    if (!scan->second->created && !scan->second->incidents.empty()) {
+    if (!scan->second->added && !scan->second->incidents.empty()) {
       upload_timer_.Reset();
       return;
     }
   }
 
   collection_timeout_pending_ = false;
 
   UploadIfCollectionComplete();
 }
 
-void IncidentReportingService::CollectDownloadDetails(
-    ClientIncidentReport_DownloadDetails* download_details) {
+void IncidentReportingService::BeginDownloadCollection() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // TODO(grt): collect download info; http://crbug.com/383042.
+  DCHECK(report_);
+  // Nothing to do if a search for the most recent download is already pending
+  // or if one has already been found.
+  if (last_download_finder_ || report_->has_download())
+    return;
+
+  last_download_begin_ = base::TimeTicks::Now();
+  last_download_finder_ = CreateDownloadFinder(
+      base::Bind(&IncidentReportingService::OnLastDownloadFound,
+                 weak_ptr_factory_.GetWeakPtr()));
+  // No instance is returned if there are no eligible loaded profiles. Another
+  // search will be attempted in OnProfileAdded() if another profile appears on
+  // the scene.
+  if (!last_download_finder_)
+    last_download_begin_ = base::TimeTicks();
+}
+
+bool IncidentReportingService::WaitingForMostRecentDownload() {
+  DCHECK(report_);  // Only call this when a report is being assembled.
+  // The easy case: not waiting if a download has already been found.
+  if (report_->has_download())
+    return false;
+  // The next easy case: waiting if the finder is operating.
+  if (last_download_finder_)
+    return true;
+  // The harder case: waiting if a profile has not yet been added.
+  for (ProfileContextCollection::const_iterator scan = profiles_.begin();
+       scan != profiles_.end();
+       ++scan) {
+    if (!scan->second->added)
+      return true;
+  }
+  // There is no most recent download and there's nothing more to wait for.
+  return false;
+}
+
+void IncidentReportingService::CancelDownloadCollection() {
+  last_download_finder_.reset();
+  last_download_begin_ = base::TimeTicks();
+  if (report_)
+    report_->clear_download();
+}
+
+void IncidentReportingService::OnLastDownloadFound(
+    scoped_ptr<ClientIncidentReport_DownloadDetails> last_download) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK(report_);
+
+  UMA_HISTOGRAM_TIMES("SBIRS.DownloadCollectionTime",
+                      base::TimeTicks::Now() - last_download_begin_);
+  last_download_begin_ = base::TimeTicks();
+
+  // Harvest the finder.
+  last_download_finder_.reset();
+
+  if (last_download)
+    report_->set_allocated_download(last_download.release());
+
+  UploadIfCollectionComplete();
 }
 
 void IncidentReportingService::UploadIfCollectionComplete() {
   DCHECK(report_);
-  // Bail out if there are still outstanding collection tasks.
-  if (environment_collection_pending_ || collection_timeout_pending_)
+  // Bail out if there are still outstanding collection tasks. Completion of any
+  // of these will start another upload attempt.
+  if (WaitingForEnvironmentCollection() ||
+      WaitingToCollateIncidents() ||
+      WaitingForMostRecentDownload()) {
     return;
+  }
 
   // Take ownership of the report and clear things for future reports.
   scoped_ptr<ClientIncidentReport> report(report_.Pass());
   last_incident_time_ = base::TimeTicks();
 
+  // Drop the report if no executable download was found.
+  if (!report->has_download()) {
+    UMA_HISTOGRAM_ENUMERATION("SBIRS.UploadResult",
+                              IncidentReportUploader::UPLOAD_NO_DOWNLOAD,
+                              IncidentReportUploader::NUM_UPLOAD_RESULTS);
+    return;
+  }
+
   ClientIncidentReport_EnvironmentData_Process* process =
       report->mutable_environment()->mutable_process();
 
   // Not all platforms have a metrics reporting preference.
   if (g_browser_process->local_state()->FindPreference(
           prefs::kMetricsReportingEnabled)) {
     process->set_metrics_consent(g_browser_process->local_state()->GetBoolean(
         prefs::kMetricsReportingEnabled));
   }
 
@@ skipping 140 matching lines @@
   if (result == IncidentReportUploader::UPLOAD_SUCCESS)
     HandleResponse(*upload);
   // else retry?
 }
 
 void IncidentReportingService::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
-    case chrome::NOTIFICATION_PROFILE_CREATED: {
+    case chrome::NOTIFICATION_PROFILE_ADDED: {
       Profile* profile = content::Source<Profile>(source).ptr();
       if (!profile->IsOffTheRecord())
-        OnProfileCreated(profile);
+        OnProfileAdded(profile);
       break;
     }
     case chrome::NOTIFICATION_PROFILE_DESTROYED: {
       Profile* profile = content::Source<Profile>(source).ptr();
       if (!profile->IsOffTheRecord())
         OnProfileDestroyed(profile);
       break;
     }
     default:
       break;
   }
 }
 
 }  // namespace safe_browsing