Download Feedback Service should upload all eligible downloads

chrome/browser/safe_browsing/download_feedback_service.cc

 // Copyright 2013 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/download_feedback_service.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ skipping 122 matching lines @@
 }
 
 // static
 void DownloadFeedbackService::RecordEligibleDownloadShown(
     content::DownloadDangerType danger_type) {
   UMA_HISTOGRAM_ENUMERATION("SBDownloadFeedback.Eligible",
                             danger_type,
                             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);
 
   DownloadFeedbackPings* pings = DownloadFeedbackPings::FromDownload(*download);
   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
 void DownloadFeedbackService::BeginFeedbackOrDeleteFile(
     const scoped_refptr<base::TaskRunner>& file_task_runner,
     const base::WeakPtr<DownloadFeedbackService>& service,
     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 {
     base::FileUtilProxy::DeleteFile(file_task_runner.get(),
                                     path,
                                     false,
                                     base::FileUtilProxy::StatusCallback());
   }
 }
 
 void DownloadFeedbackService::StartPendingFeedback() {
   DCHECK(!active_feedback_.empty());
   active_feedback_.front()->Start(base::Bind(
       &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,
       ping_request, ping_response));
   active_feedback_.push(std::move(feedback));
   UMA_HISTOGRAM_COUNTS_100("SBDownloadFeedback.ActiveFeedbacks",
                            active_feedback_.size());
 
   if (active_feedback_.size() == 1)
     StartPendingFeedback();
 }
 
 void DownloadFeedbackService::FeedbackComplete() {
   DVLOG(1) << __func__;
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!active_feedback_.empty());
   active_feedback_.pop();
   if (!active_feedback_.empty())
     StartPendingFeedback();
 }
 
 }  // namespace safe_browsing