Include high-fidelity metadata about a download in incident reports.

chrome/browser/safe_browsing/download_protection_service_unittest.cc

Index: chrome/browser/safe_browsing/download_protection_service_unittest.cc
diff --git a/chrome/browser/safe_browsing/download_protection_service_unittest.cc b/chrome/browser/safe_browsing/download_protection_service_unittest.cc
index b311980cd0c95a263a26bf68e6bdbb7619a1f423..b06e8c8e7fb4675594dbd5d91e637bae479ab9fa 100644
--- a/chrome/browser/safe_browsing/download_protection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/download_protection_service_unittest.cc
@@ -217,6 +217,10 @@ class DownloadProtectionServiceTest : public testing::Test {
     download_service_ = sb_service_->download_protection_service();
     download_service_->binary_feature_extractor_ = binary_feature_extractor_;
     download_service_->SetEnabled(true);
+    client_download_request_subscription_ =
+        download_service_->RegisterClientDownloadRequestCallback(
+            base::Bind(&DownloadProtectionServiceTest::OnClientDownloadRequest,
+                       base::Unretained(this)));
     base::RunLoop().RunUntilIdle();
     has_result_ = false;
 
@@ -231,6 +235,7 @@ class DownloadProtectionServiceTest : public testing::Test {
   }
 
   void TearDown() override {
+    client_download_request_subscription_.reset();
     sb_service_->ShutDown();
     // Flush all of the thread message loops to ensure that there are no
     // tasks currently running.
@@ -300,6 +305,12 @@ class DownloadProtectionServiceTest : public testing::Test {
     return certs.empty() ? NULL : certs[0];
   }
 
+  bool HasClientDownloadRequest() const {
+    return last_client_download_request_.get() != NULL;
+  }
+
+  void ClearClientDownloadRequest() { last_client_download_request_.reset(); }
+
  private:
   // Helper functions for FlushThreadMessageLoops.
   void RunAllPendingAndQuitUI() {
@@ -332,6 +343,14 @@ class DownloadProtectionServiceTest : public testing::Test {
     MessageLoop::current()->Run();
   }
 
+  void OnClientDownloadRequest(content::DownloadItem* download,
+                               const ClientDownloadRequest* request) {
+    if (request)
+      last_client_download_request_.reset(new ClientDownloadRequest(*request));
+    else
+      last_client_download_request_.reset();
+  }
+
  public:
   void CheckDoneCallback(
       DownloadProtectionService::DownloadCheckResult result) {
@@ -373,6 +392,9 @@ class DownloadProtectionServiceTest : public testing::Test {
 #if defined(OS_MACOSX)
   scoped_ptr<base::FieldTrialList> field_trial_list_;
 #endif
+  DownloadProtectionService::ClientDownloadRequestSubscription
+      client_download_request_subscription_;
+  scoped_ptr<ClientDownloadRequest> last_client_download_request_;
 };
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadInvalidUrl) {
@@ -395,6 +417,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadInvalidUrl) {
                  base::Unretained(this)));
   MessageLoop::current()->Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+  EXPECT_FALSE(HasClientDownloadRequest());
   Mock::VerifyAndClearExpectations(&item);
 
   url_chain.push_back(GURL("file://www.google.com/"));
@@ -411,6 +434,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadInvalidUrl) {
                  base::Unretained(this)));
   MessageLoop::current()->Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+  EXPECT_FALSE(HasClientDownloadRequest());
 }
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadNotABinary) {
@@ -434,6 +458,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadNotABinary) {
                  base::Unretained(this)));
   MessageLoop::current()->Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+  EXPECT_FALSE(HasClientDownloadRequest());
 }
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
@@ -492,6 +517,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
+
   // Check that the referrer is not matched against the whitelist.
   referrer = GURL("http://www.google.com/");
   download_service_->CheckClientDownload(
@@ -505,6 +538,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
+
   // Redirect from a site shouldn't be checked either.
   url_chain.insert(url_chain.begin(), GURL("http://www.google.com/redirect"));
   download_service_->CheckClientDownload(
@@ -518,6 +559,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
+
   // Only if the final url is whitelisted should it be SAFE.
   url_chain.push_back(GURL("http://www.google.com/a.exe"));
   download_service_->CheckClientDownload(
@@ -530,6 +579,9 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadWhitelistedUrl) {
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
 #endif
+  // TODO(grt): Make the service produce the request even when the URL is
+  // whitelisted.
+  EXPECT_FALSE(HasClientDownloadRequest());
 }
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadFetchFailed) {
@@ -619,11 +671,19 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
 #if defined(OS_WIN)
   EXPECT_TRUE(IsResult(DownloadProtectionService::SAFE));
 #else
-  // On !OS_WIN, no file types are currently supported. Hence all erquests to
+  // On !OS_WIN, no file types are currently supported. Hence all requests to
   // CheckClientDownload() result in a verdict of UNKNOWN.
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
+
   // Invalid response should result in UNKNOWN.
   response.Clear();
   factory.SetFakeResponse(
@@ -637,6 +697,12 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
                  base::Unretained(this)));
   MessageLoop::current()->Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
   std::string feedback_ping;
   std::string feedback_response;
   EXPECT_FALSE(DownloadFeedbackService::GetPingsForDownloadForTesting(
@@ -662,6 +728,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
+
   // If the response is uncommon the result should also be marked as uncommon.
   response.set_verdict(ClientDownloadResponse::UNCOMMON);
   factory.SetFakeResponse(
@@ -682,6 +756,8 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
   EXPECT_TRUE(decoded_request.ParseFromString(feedback_ping));
   EXPECT_EQ(url_chain.back().spec(), decoded_request.url());
   EXPECT_EQ(response.SerializeAsString(), feedback_response);
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
@@ -704,6 +780,8 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
   EXPECT_TRUE(DownloadFeedbackService::GetPingsForDownloadForTesting(
       item, &feedback_ping, &feedback_response));
   EXPECT_EQ(response.SerializeAsString(), feedback_response);
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
@@ -726,6 +804,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadSuccess) {
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
 }
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadHTTPS) {
@@ -775,6 +861,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadHTTPS) {
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
 }
 
 TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) {
@@ -826,6 +920,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) {
                  base::Unretained(this)));
   MessageLoop::current()->Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+  EXPECT_FALSE(HasClientDownloadRequest());
   Mock::VerifyAndClearExpectations(sb_service_.get());
   Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());
 
@@ -851,6 +946,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) {
   // verdict is UNKNOWN.
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
   Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());
 
   // If the response is dangerous the result should also be marked as
@@ -871,6 +974,14 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadZip) {
 #else
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
 #endif
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  // OSX sends pings for evaluation purposes.
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
   Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());
 }
 
@@ -908,6 +1019,7 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadCorruptZip) {
                  base::Unretained(this)));
   MessageLoop::current()->Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+  EXPECT_FALSE(HasClientDownloadRequest());
   Mock::VerifyAndClearExpectations(sb_service_.get());
   Mock::VerifyAndClearExpectations(binary_feature_extractor_.get());
 }
@@ -1010,6 +1122,7 @@ TEST_F(DownloadProtectionServiceTest,
   MessageLoop::current()->Run();
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   EXPECT_EQ(NULL, fetcher);
+  EXPECT_FALSE(HasClientDownloadRequest());
 }
 #endif
 
@@ -1056,9 +1169,12 @@ TEST_F(DownloadProtectionServiceTest, CheckClientDownloadValidateRequest) {
   MessageLoop::current()->Run();
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   EXPECT_EQ(NULL, fetcher);
+  EXPECT_FALSE(HasClientDownloadRequest());
 #else
   // Run the message loop(s) until SendRequest is called.
   FlushThreadMessageLoops();
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   ASSERT_TRUE(fetcher);
   ClientDownloadRequest request;
@@ -1141,9 +1257,12 @@ TEST_F(DownloadProtectionServiceTest,
   MessageLoop::current()->Run();
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   EXPECT_EQ(NULL, fetcher);
+  EXPECT_FALSE(HasClientDownloadRequest());
 #else
   // Run the message loop(s) until SendRequest is called.
   FlushThreadMessageLoops();
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
   net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
   ASSERT_TRUE(fetcher);
   ClientDownloadRequest request;
@@ -1229,8 +1348,11 @@ TEST_F(DownloadProtectionServiceTest,
     MessageLoop::current()->Run();
     net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
     EXPECT_EQ(NULL, fetcher);
+    EXPECT_FALSE(HasClientDownloadRequest());
 #else
     EXPECT_EQ(0, fetcher_watcher.WaitForRequest());
+    EXPECT_TRUE(HasClientDownloadRequest());
+    ClearClientDownloadRequest();
     net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
     ASSERT_TRUE(fetcher);
     ClientDownloadRequest request;
@@ -1304,8 +1426,11 @@ TEST_F(DownloadProtectionServiceTest,
     MessageLoop::current()->Run();
     net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
     EXPECT_EQ(NULL, fetcher);
+    EXPECT_FALSE(HasClientDownloadRequest());
 #else
     EXPECT_EQ(0, fetcher_watcher.WaitForRequest());
+    EXPECT_TRUE(HasClientDownloadRequest());
+    ClearClientDownloadRequest();
     net::TestURLFetcher* fetcher = factory.GetFetcherByID(0);
     ASSERT_TRUE(fetcher);
     ClientDownloadRequest request;
@@ -1460,6 +1585,12 @@ TEST_F(DownloadProtectionServiceTest, TestDownloadRequestTimeout) {
   // anything yet.
   MessageLoop::current()->Run();
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  EXPECT_TRUE(HasClientDownloadRequest());
+  ClearClientDownloadRequest();
+#else
+  EXPECT_FALSE(HasClientDownloadRequest());
+#endif
 }
 
 TEST_F(DownloadProtectionServiceTest, TestDownloadItemDestroyed) {
@@ -1503,6 +1634,7 @@ TEST_F(DownloadProtectionServiceTest, TestDownloadItemDestroyed) {
   }
 
   EXPECT_TRUE(IsResult(DownloadProtectionService::UNKNOWN));
+  EXPECT_FALSE(HasClientDownloadRequest());
 }
 
 TEST_F(DownloadProtectionServiceTest, GetCertificateWhitelistStrings) {