Download Feedback Service should upload all eligible downloads

chrome/browser/safe_browsing/download_feedback_service_unittest.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 <stddef.h>
 #include <stdint.h>
 
 #include <vector>
 
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/browser/safe_browsing/download_feedback.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/mock_download_item.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using ::testing::_;
 using ::testing::Return;
+using ::testing::ReturnRef;
 using ::testing::SaveArg;
 
 namespace safe_browsing {
 
 namespace {
 
 class FakeDownloadFeedback : public DownloadFeedback {
  public:
   FakeDownloadFeedback(net::URLRequestContextGetter* request_context_getter,
                        base::TaskRunner* file_task_runner,
                        const base::FilePath& file_path,
                        const std::string& ping_request,
                        const std::string& ping_response,
                        base::Closure deletion_callback)
       : ping_request_(ping_request),
         ping_response_(ping_response),
         deletion_callback_(deletion_callback),
-        start_called_(false) {
-  }
+        start_called_(false) {}
 
   ~FakeDownloadFeedback() override { deletion_callback_.Run(); }
 
   void Start(const base::Closure& finish_callback) override {
     start_called_ = true;
     finish_callback_ = finish_callback;
   }
 
   const std::string& GetPingRequestForTesting() const override {
     return ping_request_;
@@ skipping 27 matching lines @@
  public:
   ~FakeDownloadFeedbackFactory() override {}
 
   std::unique_ptr<DownloadFeedback> CreateDownloadFeedback(
       net::URLRequestContextGetter* request_context_getter,
       base::TaskRunner* file_task_runner,
       const base::FilePath& file_path,
       const std::string& ping_request,
       const std::string& ping_response) override {
     FakeDownloadFeedback* feedback = new FakeDownloadFeedback(
-        request_context_getter,
-        file_task_runner,
-        file_path,
-        ping_request,
+        request_context_getter, file_task_runner, file_path, ping_request,
         ping_response,
-        base::Bind(&FakeDownloadFeedbackFactory::DownloadFeedbackDeleted,
-                   base::Unretained(this),
-                   feedbacks_.size()));
+        base::Bind(&FakeDownloadFeedbackFactory::DownloadFeedbackSent,
+                   base::Unretained(this), feedbacks_.size()));
     feedbacks_.push_back(feedback);
     return base::WrapUnique(feedback);
   }
 
-  void DownloadFeedbackDeleted(size_t n) { feedbacks_[n] = nullptr; }
+  void DownloadFeedbackSent(size_t n) { feedbacks_[n] = nullptr; }
 
   FakeDownloadFeedback* feedback(size_t n) const {
     return feedbacks_[n];
   }
 
   size_t num_feedbacks() const {
     return feedbacks_.size();
   }
 
  private:
@@ skipping 70 matching lines @@
 
   EXPECT_FALSE(WillStorePings(DownloadProtectionService::SAFE, bad_size));
   EXPECT_FALSE(WillStorePings(DownloadProtectionService::DANGEROUS, bad_size));
   EXPECT_FALSE(WillStorePings(DownloadProtectionService::UNCOMMON, bad_size));
   EXPECT_FALSE(
       WillStorePings(DownloadProtectionService::DANGEROUS_HOST, bad_size));
   EXPECT_FALSE(WillStorePings(DownloadProtectionService::POTENTIALLY_UNWANTED,
                               bad_size));
 }
 
-TEST_F(DownloadFeedbackServiceTest, SingleFeedbackComplete) {
+TEST_F(DownloadFeedbackServiceTest, SingleFeedbackCompleteAndDiscardDownload) {
   const base::FilePath file_path(CreateTestFile(0));
   const std::string ping_request = "ping";
   const std::string ping_response = "resp";
 
   content::DownloadItem::AcquireFileCallback download_discarded_callback;
 
   content::MockDownloadItem item;
   EXPECT_CALL(item, GetDangerType())
       .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT));
   EXPECT_CALL(item, GetReceivedBytes()).WillRepeatedly(Return(1000));
-  EXPECT_CALL(item, StealDangerousDownload(_))
-      .WillOnce(SaveArg<0>(&download_discarded_callback));
+  EXPECT_CALL(item,
+              StealDangerousDownload(true /*delete_file_after_feedback*/, _))
+      .WillOnce(SaveArg<1>(&download_discarded_callback));
 
   DownloadFeedbackService service(request_context_getter_.get(),
                                   file_task_runner_.get());
   service.MaybeStorePingsForDownload(
       DownloadProtectionService::UNCOMMON, &item, ping_request, ping_response);
   ASSERT_TRUE(DownloadFeedbackService::IsEnabledForDownload(item));
-  service.BeginFeedbackForDownload(&item);
+  service.BeginFeedbackForDownload(&item, DownloadCommands::DISCARD);
   ASSERT_FALSE(download_discarded_callback.is_null());
   EXPECT_EQ(0U, num_feedbacks());
 
+  download_discarded_callback.Run(file_path);
+  ASSERT_EQ(1U, num_feedbacks());
+  ASSERT_TRUE(feedback(0));
+  EXPECT_TRUE(feedback(0)->start_called());
+  EXPECT_EQ(ping_request, feedback(0)->GetPingRequestForTesting());
+  EXPECT_EQ(ping_response, feedback(0)->GetPingResponseForTesting());
+
+  feedback(0)->finish_callback().Run();
+  EXPECT_FALSE(feedback(0));
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(base::PathExists(file_path));
+}
+
+TEST_F(DownloadFeedbackServiceTest, SingleFeedbackCompleteAndKeepDownload) {
+  const base::FilePath file_path(CreateTestFile(0));
+  const std::string ping_request = "ping";
+  const std::string ping_response = "resp";
+
+  content::DownloadItem::AcquireFileCallback download_discarded_callback;
+
+  content::MockDownloadItem item;
+  EXPECT_CALL(item, GetDangerType())
+      .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT));
+  EXPECT_CALL(item, GetReceivedBytes()).WillRepeatedly(Return(1000));
+  EXPECT_CALL(item,
+              StealDangerousDownload(false /*delete_file_after_feedback*/, _))
+      .WillOnce(SaveArg<1>(&download_discarded_callback));
+  EXPECT_CALL(item, ValidateDangerousDownload()).Times(1);
+  GURL empty_url;
+  EXPECT_CALL(item, GetURL()).WillOnce(ReturnRef(empty_url));
+
+  DownloadFeedbackService service(request_context_getter_.get(),
+                                  file_task_runner_.get());
+  service.MaybeStorePingsForDownload(DownloadProtectionService::UNCOMMON, &item,
+                                     ping_request, ping_response);
+  ASSERT_TRUE(DownloadFeedbackService::IsEnabledForDownload(item));
+  service.BeginFeedbackForDownload(&item, DownloadCommands::KEEP);
+  ASSERT_FALSE(download_discarded_callback.is_null());
+  EXPECT_EQ(0U, num_feedbacks());
+
   download_discarded_callback.Run(file_path);
   ASSERT_EQ(1U, num_feedbacks());
   ASSERT_TRUE(feedback(0));
   EXPECT_TRUE(feedback(0)->start_called());
   EXPECT_EQ(ping_request, feedback(0)->GetPingRequestForTesting());
   EXPECT_EQ(ping_response, feedback(0)->GetPingResponseForTesting());
 
   feedback(0)->finish_callback().Run();
   EXPECT_FALSE(feedback(0));
 
-  // File should still exist since our FakeDownloadFeedback does not delete it.
   base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(base::PathExists(file_path));
 }
 
 TEST_F(DownloadFeedbackServiceTest, MultiplePendingFeedbackComplete) {
   const std::string ping_request = "ping";
   const std::string ping_response = "resp";
   const size_t kNumDownloads = 3;
 
   content::DownloadItem::AcquireFileCallback
       download_discarded_callback[kNumDownloads];
 
   base::FilePath file_path[kNumDownloads];
   content::MockDownloadItem item[kNumDownloads];
   for (size_t i = 0; i < kNumDownloads; ++i) {
     file_path[i] = CreateTestFile(i);
     EXPECT_CALL(item[i], GetDangerType())
         .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT));
     EXPECT_CALL(item[i], GetReceivedBytes()).WillRepeatedly(Return(1000));
-    EXPECT_CALL(item[i], StealDangerousDownload(_))
-        .WillOnce(SaveArg<0>(&download_discarded_callback[i]));
+    EXPECT_CALL(item[i], StealDangerousDownload(true, _))
+        .WillOnce(SaveArg<1>(&download_discarded_callback[i]));
     DownloadFeedbackService::MaybeStorePingsForDownload(
         DownloadProtectionService::UNCOMMON, &item[i], ping_request,
         ping_response);
     ASSERT_TRUE(DownloadFeedbackService::IsEnabledForDownload(item[i]));
   }
 
   {
     DownloadFeedbackService service(request_context_getter_.get(),
                                     file_task_runner_.get());
     for (size_t i = 0; i < kNumDownloads; ++i) {
       SCOPED_TRACE(i);
-      service.BeginFeedbackForDownload(&item[i]);
+      service.BeginFeedbackForDownload(&item[i], DownloadCommands::DISCARD);
       ASSERT_FALSE(download_discarded_callback[i].is_null());
     }
     EXPECT_EQ(0U, num_feedbacks());
 
     for (size_t i = 0; i < kNumDownloads; ++i) {
       download_discarded_callback[i].Run(file_path[i]);
     }
 
     ASSERT_EQ(3U, num_feedbacks());
     EXPECT_TRUE(feedback(0)->start_called());
@@ skipping 35 matching lines @@
   content::DownloadItem::AcquireFileCallback
       download_discarded_callback[kNumDownloads];
 
   base::FilePath file_path[kNumDownloads];
   content::MockDownloadItem item[kNumDownloads];
   for (size_t i = 0; i < kNumDownloads; ++i) {
     file_path[i] = CreateTestFile(i);
     EXPECT_CALL(item[i], GetDangerType())
         .WillRepeatedly(Return(content::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT));
     EXPECT_CALL(item[i], GetReceivedBytes()).WillRepeatedly(Return(1000));
-    EXPECT_CALL(item[i], StealDangerousDownload(_))
-        .WillOnce(SaveArg<0>(&download_discarded_callback[i]));
+    EXPECT_CALL(item[i], StealDangerousDownload(true, _))
+        .WillOnce(SaveArg<1>(&download_discarded_callback[i]));
     DownloadFeedbackService::MaybeStorePingsForDownload(
         DownloadProtectionService::UNCOMMON, &item[i], ping_request,
         ping_response);
     ASSERT_TRUE(DownloadFeedbackService::IsEnabledForDownload(item[i]));
   }
 
   {
     DownloadFeedbackService service(request_context_getter_.get(),
                                     file_task_runner_.get());
     for (size_t i = 0; i < kNumDownloads; ++i) {
       SCOPED_TRACE(i);
-      service.BeginFeedbackForDownload(&item[i]);
+      service.BeginFeedbackForDownload(&item[i], DownloadCommands::DISCARD);
       ASSERT_FALSE(download_discarded_callback[i].is_null());
     }
     EXPECT_EQ(0U, num_feedbacks());
 
     download_discarded_callback[0].Run(file_path[0]);
     ASSERT_EQ(1U, num_feedbacks());
     ASSERT_TRUE(feedback(0));
     EXPECT_TRUE(feedback(0)->start_called());
 
     download_discarded_callback[1].Run(file_path[1]);
@@ skipping 25 matching lines @@
   // was deleted.
   EXPECT_FALSE(base::PathExists(file_path[2]));
 
   // These files should still exist since the FakeDownloadFeedback does not
   // delete them.
   EXPECT_TRUE(base::PathExists(file_path[0]));
   EXPECT_TRUE(base::PathExists(file_path[1]));
 }
 
 }  // namespace safe_browsing