Track CTR of uncommon download warning.

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 331fb0e600faee41c2a49711bb6055fbf19927d7..8bbeacfe6cd156868677bd1696056f6028efe532 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -32,6 +32,7 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/download/chrome_download_manager_delegate.h"
+#include "chrome/browser/download/download_commands.h"
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_item_model.h"
@@ -75,6 +76,7 @@
 #include "components/infobars/core/confirm_infobar_delegate.h"
 #include "components/infobars/core/infobar.h"
 #include "components/prefs/pref_service.h"
+#include "content/public/browser/download_danger_type.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 #include "content/public/browser/download_item.h"
 #include "content/public/browser/download_manager.h"
@@ -248,6 +250,57 @@ class DownloadTestObserverResumable : public content::DownloadTestObserver {
   DISALLOW_COPY_AND_ASSIGN(DownloadTestObserverResumable);
 };
 
+// DownloadTestObserverTerminal subclass for testing.
+class DownloadTestObserverAcceptUncommon

[asanka, 2016/03/11 22:05:32]

Rather than going this route, can you create a DownloadTestObserverTerminal with
a danger action of ON_DANGEROUS_DOWNLOAD_QUIT? Then your observer will quit when
it sees the dangerous download and then the test can invoke
DownloadCommands::ExecuteCommand().

This way you don't need to expose the test observer guts.

[Jialiu Lin, 2016/03/12 00:15:27]

This is brilliant! 

+    : public content::DownloadTestObserverTerminal {
+ public:
+  DownloadTestObserverAcceptUncommon(
+      DownloadManager* download_manager,
+      size_t wait_count,
+      content::DownloadTestObserver::DangerousDownloadAction
+          dangerous_download_action)
+      : content::DownloadTestObserverTerminal(download_manager,
+                                              wait_count,
+                                              dangerous_download_action),
+        weak_factory_(this) {}
+  ~DownloadTestObserverAcceptUncommon() override {}
+
+  void OnDownloadUpdated(DownloadItem* download) override {
+    // Real UI code gets the user's response after returning from the observer.
+    if (download->IsDangerous() &&
+        !ContainsKey(dangerous_downloads_seen(), download->GetId())) {
+      dangerous_downloads_seen().insert(download->GetId());
+
+      switch (dangerous_download_action()) {
+        case ON_DANGEROUS_DOWNLOAD_ACCEPT:
+          BrowserThread::PostTask(
+              BrowserThread::UI, FROM_HERE,
+              base::Bind(
+                  &DownloadTestObserverAcceptUncommon::AcceptDangerousDownload,
+                  weak_factory_.GetWeakPtr(), download->GetId()));
+          break;
+        default:
+          NOTREACHED();
+      }
+    }
+
+    if (IsDownloadInFinalState(download))
+      DownloadInFinalState(download);
+  }
+
+ private:
+  void AcceptDangerousDownload(uint32_t download_id) {
+    if (!download_manager())
+      return;
+    DownloadItem* download = download_manager()->GetDownload(download_id);
+    if (download && download->IsDangerous() && !download->IsDone()) {
+      DownloadCommands(download).ExecuteCommand(DownloadCommands::KEEP);
+    }
+  }
+
+  base::WeakPtrFactory<DownloadTestObserverAcceptUncommon> weak_factory_;
+};
+
 // IDs and paths of CRX files used in tests.
 const char kGoodCrxId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
 const char kGoodCrxPath[] = "extensions/good.crx";
@@ -388,6 +441,44 @@ class HistoryObserver : public DownloadHistory::Observer {
   DISALLOW_COPY_AND_ASSIGN(HistoryObserver);
 };
 
+class FakeSafeBrowsingService : public safe_browsing::SafeBrowsingService {
+ public:
+  FakeSafeBrowsingService() {}
+
+  std::string GetDownloadReport() const { return report_; }
+
+ protected:
+  ~FakeSafeBrowsingService() override {}
+
+  void SendSerializedDownloadReport(const std::string& report) override {
+    report_ = report;
+  }
+
+  std::string report_;
+};
+
+// Factory that creates FakeSafeBrowsingService instances.
+class TestSafeBrowsingServiceFactory
+    : public safe_browsing::SafeBrowsingServiceFactory {
+ public:
+  TestSafeBrowsingServiceFactory() : fake_safe_browsing_service_(nullptr) {}
+  ~TestSafeBrowsingServiceFactory() override {}
+
+  safe_browsing::SafeBrowsingService* CreateSafeBrowsingService() override {
+    if (!fake_safe_browsing_service_) {
+      fake_safe_browsing_service_ = new FakeSafeBrowsingService();
+    }
+    return fake_safe_browsing_service_.get();
+  }
+
+  scoped_refptr<FakeSafeBrowsingService> fake_safe_browsing_service() {
+    return fake_safe_browsing_service_;
+  }
+
+ private:
+  scoped_refptr<FakeSafeBrowsingService> fake_safe_browsing_service_;
+};
+
 class DownloadTest : public InProcessBrowserTest {
  public:
   // Choice of navigation or direct fetch.  Used by |DownloadFileCheckErrors()|.
@@ -415,13 +506,25 @@ class DownloadTest : public InProcessBrowserTest {
     content::TestFileErrorInjector::FileErrorInfo error_info;
   };
 
-  DownloadTest() {}
+  DownloadTest()
+      : test_safe_browsing_factory_(new TestSafeBrowsingServiceFactory()) {}
+
+  void SetUp() override {
+    safe_browsing::SafeBrowsingService::RegisterFactory(
+        test_safe_browsing_factory_.get());
+    InProcessBrowserTest::SetUp();
+  }
+
+  void TearDown() override {
+    safe_browsing::SafeBrowsingService::RegisterFactory(nullptr);
+    InProcessBrowserTest::TearDown();
+  }
 
   void SetUpOnMainThread() override {
     base::FeatureList::ClearInstanceForTesting();
     scoped_ptr<base::FeatureList> feature_list(new base::FeatureList);
-    feature_list->InitializeFromCommandLine(
-        features::kDownloadResumption.name, std::string());
+    feature_list->InitializeFromCommandLine(features::kDownloadResumption.name,

[asanka, 2016/03/11 22:05:32]

Nit: unnecessary change.

[Jialiu Lin, 2016/03/12 00:15:27]

Done.

+                                            std::string());
     base::FeatureList::SetInstance(std::move(feature_list));
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
@@ -559,6 +662,19 @@ class DownloadTest : public InProcessBrowserTest {
         download_manager, num_downloads, dangerous_download_action);
   }
 
+  // Create a DownloadTestObserverAcceptUncommon that will wait for the
+  // specified number of downloads to finish, or for
+  // an uncommon download warning to be shown.
+  content::DownloadTestObserver* UncommonDownloadWaiter(
+      Browser* browser,
+      int num_downloads,
+      content::DownloadTestObserver::DangerousDownloadAction
+          dangerous_download_action) {
+    DownloadManager* download_manager = DownloadManagerForBrowser(browser);
+    return new DownloadTestObserverAcceptUncommon(
+        download_manager, num_downloads, dangerous_download_action);
+  }
+
   void CheckDownloadStatesForBrowser(Browser* browser,
                                      size_t num,
                                      DownloadItem::DownloadState state) {
@@ -1059,6 +1175,9 @@ class DownloadTest : public InProcessBrowserTest {
     return download;
   }
 
+ protected:
+  scoped_ptr<TestSafeBrowsingServiceFactory> test_safe_browsing_factory_;
+
  private:
   static void EnsureNoPendingDownloadJobsOnIO(bool* result) {
     if (net::URLRequestSlowDownloadJob::NumberOutstandingRequests())
@@ -3270,6 +3389,49 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, FeedbackService) {
   GetDownloads(browser(), &updated_downloads);
   ASSERT_TRUE(updated_downloads.empty());
 }
+
+IN_PROC_BROWSER_TEST_F(DownloadTest, SendUncommonDownloadAcceptReport) {
+  browser()->profile()->GetPrefs()->SetBoolean(prefs::kSafeBrowsingEnabled,
+                                               true);
+  // Make a dangerous file.
+  GURL download_url(
+      net::URLRequestMockHTTPJob::GetMockUrl(kDangerousMockFilePath));
+  scoped_ptr<content::DownloadTestObserver> dangerous_observer(
+      UncommonDownloadWaiter(
+          browser(), 1,
+          content::DownloadTestObserver::ON_DANGEROUS_DOWNLOAD_ACCEPT));
+  scoped_ptr<content::DownloadTestObserver> in_progress_observer(
+      new content::DownloadTestObserverInProgress(
+          DownloadManagerForBrowser(browser()), 1));
+
+  ui_test_utils::NavigateToURL(browser(), download_url);
+  in_progress_observer->WaitForFinished();
+
+  std::vector<DownloadItem*> downloads;
+  DownloadManagerForBrowser(browser())->GetAllDownloads(&downloads);
+  ASSERT_EQ(1u, downloads.size());
+  DownloadItem* download = downloads[0];
+  // Simulates an uncommon download.
+  download->OnContentCheckCompleted(
+      content::DownloadDangerType::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT);
+  ASSERT_EQ(download->GetDangerType(),
+            content::DownloadDangerType::DOWNLOAD_DANGER_TYPE_UNCOMMON_CONTENT);
+  dangerous_observer->WaitForFinished();
+
+  safe_browsing::ClientSafeBrowsingReportRequest actual_report;
+  actual_report.ParseFromString(
+      test_safe_browsing_factory_->fake_safe_browsing_service()
+          ->GetDownloadReport());
+  EXPECT_EQ(safe_browsing::ClientSafeBrowsingReportRequest::
+                DANGEROUS_DOWNLOAD_WARNING,
+            actual_report.type());
+  EXPECT_EQ(safe_browsing::ClientDownloadResponse::UNCOMMON,
+            actual_report.download_verdict());
+  EXPECT_EQ(download->GetURL().spec(), actual_report.url());
+  EXPECT_TRUE(actual_report.did_proceed());
+
+  download->Cancel(true);
+}
 #endif  // FULL_SAFE_BROWSING
 
 class DownloadTestWithShelf : public DownloadTest {