Include high-fidelity metadata about a download in incident reports.

chrome/browser/safe_browsing/incident_reporting/last_download_finder.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/last_download_finder.h"
 
 #include <algorithm>
 #include <functional>
 #include <utility>
 
@@ skipping 10 matching lines @@
 #include "chrome/common/safe_browsing/download_protection_util.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 
 namespace safe_browsing {
 
 namespace {
 
+// Overloaded function to access members of either a DownloadRow or a
+// ClientIncidentReport_DownloadDetails instance.

[mattm, 2014/10/24 02:11:10]

what function?

+
+// history::DownloadRow.

[mattm, 2014/10/24 02:11:10]

what?

+
+// Returns the end time of a download.
+int64 GetEndTime(const history::DownloadRow& row) {
+  return row.end_time.ToJavaTime();
+}
+
+// Returns true if a download was of a binary file.
+bool IsBinaryDownload(const history::DownloadRow& row) {
+  // TODO(grt): Peek into archives to see if they contain binaries;
+  // http://crbug.com/386915.
+  return (download_protection_util::IsBinaryFile(row.target_path) &&
+          !download_protection_util::IsArchiveFile(row.target_path));
+}
+
+// Returns true if a download has been opened.
+bool HasBeenOpened(const history::DownloadRow& row) {
+  return row.opened;
+}
+
+// ClientIncidentReport_DownloadDetails.

[mattm, 2014/10/24 02:11:10]

what?.. oh.  These comments aren't really clear..

[grt (UTC plus 2), 2014/10/30 19:07:35]

Modified comments and re-ordered the functions for clarity. Let me know what you
think.

+
+// Returns the end time of a download.
+int64 GetEndTime(const ClientIncidentReport_DownloadDetails& details) {
+  return details.download_time_msec();
+}
+
+// Returns true if a download was of a binary file.
+bool IsBinaryDownload(const ClientIncidentReport_DownloadDetails& details) {
+  return true;
+}
+
+// Returns true if a download has been opened.
+bool HasBeenOpened(const ClientIncidentReport_DownloadDetails& details) {
+  return details.has_open_time_msec() && details.open_time_msec();
+}
+
 // Returns true if |first| is more recent than |second|, preferring opened over
 // non-opened for downloads that completed at the same time (extraordinarily
 // unlikely). Only files that look like some kind of executable are considered.
-bool IsMoreInterestingThan(const history::DownloadRow& first,
-                           const history::DownloadRow& second) {
-  if (first.end_time < second.end_time)
+template <class A, class B>
+bool IsMoreInterestingThan(const A& first, const B& second) {
+  if (GetEndTime(first) < GetEndTime(second) || !IsBinaryDownload(first))
     return false;
-  // TODO(grt): Peek into archives to see if they contain binaries;
-  // http://crbug.com/386915.
-  if (!download_protection_util::IsBinaryFile(first.target_path) ||
-      download_protection_util::IsArchiveFile(first.target_path)) {
-    return false;
-  }
-  return (first.end_time != second.end_time ||
-          (first.opened && !second.opened));
+  return (GetEndTime(first) != GetEndTime(second) ||
+          (HasBeenOpened(first) && !HasBeenOpened(second)));
 }
 
 // Returns a pointer to the most interesting completed download in |downloads|.
 const history::DownloadRow* FindMostInteresting(
     const std::vector<history::DownloadRow>& downloads) {
   const history::DownloadRow* most_recent_row = NULL;
   for (size_t i = 0; i < downloads.size(); ++i) {
     const history::DownloadRow& row = downloads[i];
     // Ignore incomplete downloads.
     if (row.state != content::DownloadItem::COMPLETE)
       continue;
     if (!most_recent_row || IsMoreInterestingThan(row, *most_recent_row))
       most_recent_row = &row;
   }
   return most_recent_row;
 }
 
+// Returns true if |candidate| is more interesting than whichever of |details|
+// or |best_row| is present.
+template <class D>
+bool IsMostInteresting(const D& candidate,
+                       const ClientIncidentReport_DownloadDetails* details,
+                       const history::DownloadRow& best_row) {
+  return details ?
+      IsMoreInterestingThan(candidate, *details) :
+      IsMoreInterestingThan(candidate, best_row);
+}
+
 // Populates the |details| protobuf with information pertaining to |download|.
-void PopulateDetails(const history::DownloadRow& download,
-                     ClientIncidentReport_DownloadDetails* details) {
+void PopulateDetailsFromRow(const history::DownloadRow& download,
+                            ClientIncidentReport_DownloadDetails* details) {
   ClientDownloadRequest* download_request = details->mutable_download();
   download_request->set_url(download.url_chain.back().spec());
   // digests is a required field, so force it to exist.
   // TODO(grt): Include digests in reports; http://crbug.com/389123.
   ignore_result(download_request->mutable_digests());
   download_request->set_length(download.received_bytes);
   for (size_t i = 0; i < download.url_chain.size(); ++i) {
     const GURL& url = download.url_chain[i];
     ClientDownloadRequest_Resource* resource =
         download_request->add_resources();
@@ skipping 20 matching lines @@
     details->set_open_time_msec(download.end_time.ToJavaTime());
 }
 
 }  // namespace
 
 LastDownloadFinder::~LastDownloadFinder() {
 }
 
 // static
 scoped_ptr<LastDownloadFinder> LastDownloadFinder::Create(
+    const DownloadDetailsGetter& download_details_getter,
     const LastDownloadCallback& callback) {
   scoped_ptr<LastDownloadFinder> finder(make_scoped_ptr(new LastDownloadFinder(
-      g_browser_process->profile_manager()->GetLoadedProfiles(), callback)));
+      download_details_getter,
+      g_browser_process->profile_manager()->GetLoadedProfiles(),
+      callback)));
   // Return NULL if there is no work to do.
   if (finder->profiles_.empty())
     return scoped_ptr<LastDownloadFinder>();
   return finder.Pass();
 }
 
 LastDownloadFinder::LastDownloadFinder() : weak_ptr_factory_(this) {
 }
 
-LastDownloadFinder::LastDownloadFinder(const std::vector<Profile*>& profiles,
-                                       const LastDownloadCallback& callback)
-    : callback_(callback), weak_ptr_factory_(this) {
+LastDownloadFinder::LastDownloadFinder(
+    const DownloadDetailsGetter& download_details_getter,
+    const std::vector<Profile*>& profiles,
+    const LastDownloadCallback& callback)
+    : download_details_getter_(download_details_getter),
+      callback_(callback),
+      weak_ptr_factory_(this) {
   // Observe profile lifecycle events so that the finder can begin or abandon
   // the search in profiles while it is running.
   notification_registrar_.Add(this,
                               chrome::NOTIFICATION_PROFILE_ADDED,
                               content::NotificationService::AllSources());
   notification_registrar_.Add(this,
                               chrome::NOTIFICATION_HISTORY_LOADED,
                               content::NotificationService::AllSources());
   notification_registrar_.Add(this,
                               chrome::NOTIFICATION_PROFILE_DESTROYED,
@@ skipping 10 matching lines @@
   // Do not look in OTR profiles or in profiles that do not participate in
   // safe browsing.
   if (profile->IsOffTheRecord() ||
       !profile->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
     return;
   }
 
   // Exit early if already processing this profile. This could happen if, for
   // example, NOTIFICATION_PROFILE_ADDED arrives after construction while
   // waiting for NOTIFICATION_HISTORY_LOADED.
-  if (std::find(profiles_.begin(), profiles_.end(), profile) !=
-      profiles_.end()) {
-    return;
-  }
-
-  HistoryService* history_service =
-      HistoryServiceFactory::GetForProfile(profile, Profile::IMPLICIT_ACCESS);
-  // No history service is returned for profiles that do not save history.
-  if (!history_service)
+  if (profiles_.count(profile))
     return;
 
-  profiles_.push_back(profile);
-  if (history_service->BackendLoaded()) {
-    history_service->QueryDownloads(
-        base::Bind(&LastDownloadFinder::OnDownloadQuery,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   profile));
-  }  // else wait until history is loaded.
+  // Initiate a metadata search.
+  profiles_[profile] = WAITING_FOR_METADATA;

[mattm, 2014/10/24 02:11:10]

this is confusing now.. maybe the variable should be named profile_states_ ?

[grt (UTC plus 2), 2014/10/30 19:07:35]

Done.

+  download_details_getter_.Run(profile,
+                               base::Bind(&LastDownloadFinder::OnMetadataQuery,
+                                          weak_ptr_factory_.GetWeakPtr(),
+                                          profile));
+}
+
+void LastDownloadFinder::OnMetadataQuery(
+    Profile* profile,
+    scoped_ptr<ClientIncidentReport_DownloadDetails> details) {
+  auto iter = profiles_.find(profile);
+  // Early-exit if the search for this profile was abandoned.
+  if (iter == profiles_.end())
+    return;
+
+  if (details) {
+    if (IsMostInteresting(*details, details_.get(), most_recent_row_)) {
+      details_ = details.Pass();
+      most_recent_row_.end_time = base::Time();
+    }
+
+    RemoveProfileAndReportIfDone(iter);
+  } else {
+    // Search history since no metadata was found.
+    iter->second = WAITING_FOR_HISTORY;
+    HistoryService* history_service =
+        HistoryServiceFactory::GetForProfile(profile, Profile::IMPLICIT_ACCESS);
+    // No history service is returned for profiles that do not save history.
+    if (!history_service) {
+      RemoveProfileAndReportIfDone(iter);
+      return;
+    }
+    if (history_service->BackendLoaded()) {
+      history_service->QueryDownloads(
+          base::Bind(&LastDownloadFinder::OnDownloadQuery,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     profile));
+    }  // else wait until history is loaded.
+  }
 }
 
 void LastDownloadFinder::OnProfileHistoryLoaded(
     Profile* profile,
     HistoryService* history_service) {
-  if (std::find(profiles_.begin(), profiles_.end(), profile) !=
-      profiles_.end()) {
+  auto iter = profiles_.find(profile);
+  if (iter == profiles_.end())
+    return;
+
+  // Start the query in the history service if the finder was waiting for the
+  // service to load.
+  if (iter->second == WAITING_FOR_HISTORY) {
     history_service->QueryDownloads(
         base::Bind(&LastDownloadFinder::OnDownloadQuery,
                    weak_ptr_factory_.GetWeakPtr(),
                    profile));
   }
 }
 
 void LastDownloadFinder::AbandonSearchInProfile(Profile* profile) {
   // |profile| may not be present in the set of profiles.
-  std::vector<Profile*>::iterator it =
-      std::find(profiles_.begin(), profiles_.end(), profile);
-  if (it != profiles_.end())
-    RemoveProfileAndReportIfDone(it);
+  auto iter = profiles_.find(profile);
+  if (iter != profiles_.end())
+    RemoveProfileAndReportIfDone(iter);
 }
 
 void LastDownloadFinder::OnDownloadQuery(
     Profile* profile,
     scoped_ptr<std::vector<history::DownloadRow> > downloads) {
   // Early-exit if the history search for this profile was abandoned.
-  std::vector<Profile*>::iterator it =
-      std::find(profiles_.begin(), profiles_.end(), profile);
-  if (it == profiles_.end())
+  auto iter = profiles_.find(profile);
+  if (iter == profiles_.end())
     return;
 
   // Find the most recent from this profile and use it if it's better than
   // anything else found so far.
   const history::DownloadRow* profile_best = FindMostInteresting(*downloads);
-  if (profile_best && IsMoreInterestingThan(*profile_best, most_recent_row_))
+  if (profile_best &&
+      IsMostInteresting(*profile_best, details_.get(), most_recent_row_)) {
+    details_.reset();
     most_recent_row_ = *profile_best;
+  }
 
-  RemoveProfileAndReportIfDone(it);
+  RemoveProfileAndReportIfDone(iter);
 }
 
 void LastDownloadFinder::RemoveProfileAndReportIfDone(
-    std::vector<Profile*>::iterator it) {
-  DCHECK(it != profiles_.end());
-
-  *it = profiles_.back();
-  profiles_.resize(profiles_.size() - 1);
+    std::map<Profile*, ProfileWaitState>::iterator iter) {
+  DCHECK(iter != profiles_.end());
+  profiles_.erase(iter);
 
   // Finish processing if all results are in.
   if (profiles_.empty())
     ReportResults();
-  // Do not touch this instance after reporting results.
+  // Do not touch this LastDownloadFinder after reporting results.
 }
 
 void LastDownloadFinder::ReportResults() {
   DCHECK(profiles_.empty());
-  if (most_recent_row_.end_time.is_null()) {
-    callback_.Run(scoped_ptr<ClientIncidentReport_DownloadDetails>());
-    // Do not touch this instance after running the callback, since it may have
-    // been deleted.
-  } else {
+  if (details_) {
+    callback_.Run(make_scoped_ptr(new ClientIncidentReport_DownloadDetails(
+                                      *details_)).Pass());
+    // Do not touch this LastDownloadFinder after running the callback, since it
+    // may have been deleted.
+  } else if (!most_recent_row_.end_time.is_null()) {
     scoped_ptr<ClientIncidentReport_DownloadDetails> details(
         new ClientIncidentReport_DownloadDetails());
-    PopulateDetails(most_recent_row_, details.get());
+    PopulateDetailsFromRow(most_recent_row_, details.get());
     callback_.Run(details.Pass());
-    // Do not touch this instance after running the callback, since it may have
-    // been deleted.
+    // Do not touch this LastDownloadFinder after running the callback, since it
+    // may have been deleted.
+  } else {
+    callback_.Run(scoped_ptr<ClientIncidentReport_DownloadDetails>());
+    // Do not touch this LastDownloadFinder after running the callback, since it
+    // may have been deleted.
   }
 }
 
 void LastDownloadFinder::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_PROFILE_ADDED:
       SearchInProfile(content::Source<Profile>(source).ptr());
       break;
     case chrome::NOTIFICATION_HISTORY_LOADED:
       OnProfileHistoryLoaded(content::Source<Profile>(source).ptr(),
                              content::Details<HistoryService>(details).ptr());
       break;
     case chrome::NOTIFICATION_PROFILE_DESTROYED:
       AbandonSearchInProfile(content::Source<Profile>(source).ptr());
       break;
     default:
       break;
   }
 }
 
 }  // namespace safe_browsing