Download Feedback Service should upload all eligible downloads

chrome/browser/safe_browsing/download_feedback_service.cc

Index: chrome/browser/safe_browsing/download_feedback_service.cc
diff --git a/chrome/browser/safe_browsing/download_feedback_service.cc b/chrome/browser/safe_browsing/download_feedback_service.cc
index 0b78aa79c112625dc79eafcd1795674cc6c6033d..4d108542b273ff1fb7dd351419ef273a4e4702ac 100644
--- a/chrome/browser/safe_browsing/download_feedback_service.cc
+++ b/chrome/browser/safe_browsing/download_feedback_service.cc
@@ -140,11 +140,10 @@ void DownloadFeedbackService::RecordEligibleDownloadShown(
                             content::DOWNLOAD_DANGER_TYPE_MAX);
 }
 
-
 void DownloadFeedbackService::BeginFeedbackForDownload(
-    content::DownloadItem* download) {
+    content::DownloadItem* download,
+    DownloadCommands::Command download_command) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
   UMA_HISTOGRAM_ENUMERATION("SBDownloadFeedback.Activations",
                             download->GetDangerType(),
                             content::DOWNLOAD_DANGER_TYPE_MAX);
@@ -153,11 +152,14 @@ void DownloadFeedbackService::BeginFeedbackForDownload(
   DCHECK(pings);
 
   download->StealDangerousDownload(
+      download_command == DownloadCommands::DISCARD,
       base::Bind(&DownloadFeedbackService::BeginFeedbackOrDeleteFile,
-                 file_task_runner_,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 pings->ping_request(),
-                 pings->ping_response()));
+                 file_task_runner_, weak_ptr_factory_.GetWeakPtr(),
+                 pings->ping_request(), pings->ping_response()));
+  if (download_command == DownloadCommands::KEEP) {
+    DownloadCommands command(download);
+    command.ExecuteCommand(download_command);
+  }
 }
 
 // static
@@ -167,6 +169,9 @@ void DownloadFeedbackService::BeginFeedbackOrDeleteFile(
     const std::string& ping_request,
     const std::string& ping_response,
     const base::FilePath& path) {
+  if (path.empty())

[asanka, 2016/11/01 21:09:23]

We should probably measure this. Not a big deal if you don't, but if the failure
rate is high, then we'd need to revisit our strategy.

[Jialiu Lin, 2016/11/03 20:16:50]

Done.

+    return;
+
   if (service) {
     service->BeginFeedback(ping_request, ping_response, path);
   } else {
@@ -183,10 +188,9 @@ void DownloadFeedbackService::StartPendingFeedback() {
       &DownloadFeedbackService::FeedbackComplete, base::Unretained(this)));
 }
 
-void DownloadFeedbackService::BeginFeedback(
-    const std::string& ping_request,
-    const std::string& ping_response,
-    const base::FilePath& path) {
+void DownloadFeedbackService::BeginFeedback(const std::string& ping_request,
+                                            const std::string& ping_response,
+                                            const base::FilePath& path) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   std::unique_ptr<DownloadFeedback> feedback(DownloadFeedback::Create(
       request_context_getter_.get(), file_task_runner_.get(), path,