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

chrome/browser/safe_browsing/download_protection_service.cc

Index: chrome/browser/safe_browsing/download_protection_service.cc
diff --git a/chrome/browser/safe_browsing/download_protection_service.cc b/chrome/browser/safe_browsing/download_protection_service.cc
index b00646a2c1fbf45d133b1c4bad56e802b5fed15e..5d856c50288c8aa03430b0c165f6ed7c07e3c582 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -732,6 +732,8 @@ class DownloadProtectionService::CheckClientDownloadRequest
       return;
     }
 
+    service_->client_download_request_callbacks_.Notify(item_, &request);
+
     VLOG(2) << "Sending a request for URL: "
             << item_->GetUrlChain().back();
     fetcher_.reset(net::URLFetcher::Create(0 /* ID used for testing */,
@@ -782,6 +784,14 @@ class DownloadProtectionService::CheckClientDownloadRequest
                             base::TimeTicks::Now() - timeout_start_time_);
       }
     }
+    if (result == SAFE && (reason == REASON_WHITELISTED_URL ||
+                           reason == REASON_TRUSTED_EXECUTABLE)) {

[mattm, 2014/10/24 02:11:10]

This seems error prone, I'm not confident this condition is correct, though I'm
not entirely sure what it's trying to do.

[grt (UTC plus 2), 2014/10/30 19:07:35]

I've clarified the comment and added TODOs to the related code in
CheckWhitelists. I hope to address these TODOs in a separate change since
they'll involve changing the CheckClientDownloadRequest state machine.

+      // A ClientDownloadRequest was not generated for this download, so
+      // callbakcs were not run with one. Run them with null so they see that a

[mattm, 2014/10/24 02:11:09]

callbacks

[grt (UTC plus 2), 2014/10/30 19:07:35]

Done.

+      // previously-seen ClientDownloadRequest is now stale.

[mattm, 2014/10/24 02:11:10]

This comment doesn't make sense to me. How would anything have seen a
ClientDownloadRequest if none was ever generated?

[grt (UTC plus 2), 2014/10/30 19:07:35]

From an unrelated previous download. I'll clarify in the comment.

+      // TODO(grt): persist metadata for these downloads as well.
+      service_->client_download_request_callbacks_.Notify(item_, nullptr);
+    }
     if (service_) {
       VLOG(2) << "SafeBrowsing download verdict for: "
               << item_->DebugString(true) << " verdict:" << reason
@@ -961,6 +971,12 @@ bool DownloadProtectionService::IsSupportedDownload(
 #endif
 }
 
+DownloadProtectionService::ClientDownloadRequestSubscription
+DownloadProtectionService::RegisterClientDownloadRequestCallback(
+    const ClientDownloadRequestCallback& callback) {
+  return client_download_request_callbacks_.Add(callback);
+}
+
 void DownloadProtectionService::CancelPendingRequests() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   for (std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it =