Download Feedback Service should upload all eligible downloads

chrome/browser/download/download_browsertest.cc

Index: chrome/browser/download/download_browsertest.cc
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index 6612712e29c1d7238d04672651571883a0a097d1..be6dd27306896eb446a66b70294ee126ea01b248 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -3311,7 +3311,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, SafeSupportedFile) {
   download->Cancel(true);
 }
 
-IN_PROC_BROWSER_TEST_F(DownloadTest, FeedbackService) {
+IN_PROC_BROWSER_TEST_F(DownloadTest, FeedbackServiceDiscardDownload) {
   // Make a dangerous file.
   GURL download_url(net::URLRequestMockHTTPJob::GetMockUrl(
       "downloads/dangerous/dangerous.swf"));
@@ -3355,15 +3355,65 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, FeedbackService) {
 
   // Begin feedback and check that the file is "stolen".
   download_protection_service->feedback_service()->BeginFeedbackForDownload(
-      downloads[0]);
+      downloads[0], DownloadCommands::DISCARD);
   std::vector<DownloadItem*> updated_downloads;
   GetDownloads(browser(), &updated_downloads);
   ASSERT_TRUE(updated_downloads.empty());
 }
 
+IN_PROC_BROWSER_TEST_F(DownloadTest, FeedbackServiceKeepDownload) {
+  // Make a dangerous file.
+  GURL download_url(net::URLRequestMockHTTPJob::GetMockUrl(
+      "downloads/dangerous/dangerous.swf"));
+  std::unique_ptr<content::DownloadTestObserverInterrupted> observer(
+      new content::DownloadTestObserverInterrupted(
+          DownloadManagerForBrowser(browser()), 1,
+          content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_QUIT));
+  ui_test_utils::NavigateToURLWithDisposition(
+      browser(), GURL(download_url), WindowOpenDisposition::NEW_BACKGROUND_TAB,
+      ui_test_utils::BROWSER_TEST_NONE);
+  observer->WaitForFinished();
+
+  // Get the download from the DownloadManager.
+  std::vector<DownloadItem*> downloads;
+  DownloadManagerForBrowser(browser())->GetAllDownloads(&downloads);
+  ASSERT_EQ(1u, downloads.size());
+  EXPECT_TRUE(downloads[0]->IsDangerous());
+
+  // Save fake pings for the download.
+  safe_browsing::ClientDownloadReport fake_metadata;
+  fake_metadata.mutable_download_request()->set_url("http://test");
+  fake_metadata.mutable_download_request()->set_length(1);
+  fake_metadata.mutable_download_request()->mutable_digests()->set_sha1("hi");
+  fake_metadata.mutable_download_response()->set_verdict(
+      safe_browsing::ClientDownloadResponse::UNCOMMON);
+  std::string ping_request(
+      fake_metadata.download_request().SerializeAsString());
+  std::string ping_response(
+      fake_metadata.download_response().SerializeAsString());
+  safe_browsing::SafeBrowsingService* sb_service =
+      g_browser_process->safe_browsing_service();
+  safe_browsing::DownloadProtectionService* download_protection_service =
+      sb_service->download_protection_service();
+  download_protection_service->feedback_service()->MaybeStorePingsForDownload(
+      safe_browsing::DownloadProtectionService::UNCOMMON, downloads[0],
+      ping_request, ping_response);
+  ASSERT_TRUE(safe_browsing::DownloadFeedbackService::IsEnabledForDownload(
+      *(downloads[0])));
+
+  // Begin feedback and check that file is still there.
+  download_protection_service->feedback_service()->BeginFeedbackForDownload(
+      downloads[0], DownloadCommands::KEEP);
+  std::vector<DownloadItem*> updated_downloads;
+  GetDownloads(browser(), &updated_downloads);
+  ASSERT_FALSE(updated_downloads.empty());
+  for (auto download : updated_downloads)
+    download->Cancel(true);

[asanka, 2016/11/01 20:58:58]

Should also test that the target file exists. You can accept the download and
wait for it to complete. At that point, the download file should still exist.

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

Done.

+}
+
 IN_PROC_BROWSER_TEST_F(
-     DownloadTestWithFakeSafeBrowsing,
-     SendUncommonDownloadReportIfUserProceed) {
+    DownloadTestWithFakeSafeBrowsing,
+    SendUncommonDownloadReportIfUserProceed) {
   browser()->profile()->GetPrefs()->SetBoolean(prefs::kSafeBrowsingEnabled,
                                                true);
   // Make a dangerous file.