Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6304)

Unified Diff: chrome/browser/safe_browsing/download_protection_service.cc

Issue 663023007: Include high-fidelity metadata about a download in incident reports. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git/+/master
Patch Set: added DCHECK Created 6 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 3af48ae41417b843685e5cd81ad17fe029ea78cb..564a07520110479405e159934e4109f5e519d6bc 100644
--- a/chrome/browser/safe_browsing/download_protection_service.cc
+++ b/chrome/browser/safe_browsing/download_protection_service.cc
@@ -593,6 +593,9 @@ class DownloadProtectionService::CheckClientDownloadRequest
if (url.is_valid() && database_manager_->MatchDownloadWhitelistUrl(url)) {
VLOG(2) << url << " is on the download whitelist.";
RecordCountOfSignedOrWhitelistedDownload();
+ // TODO(grt): Continue processing without uploading so that
+ // ClientDownloadRequest callbacks can be run even for this type of safe
+ // download.
PostFinishTask(SAFE, REASON_WHITELISTED_URL);
return;
}
@@ -602,6 +605,9 @@ class DownloadProtectionService::CheckClientDownloadRequest
for (int i = 0; i < signature_info_.certificate_chain_size(); ++i) {
if (CertificateChainIsWhitelisted(
signature_info_.certificate_chain(i))) {
+ // TODO(grt): Continue processing without uploading so that
+ // ClientDownloadRequest callbacks can be run even for this type of
+ // safe download.
PostFinishTask(SAFE, REASON_TRUSTED_EXECUTABLE);
return;
}
@@ -732,6 +738,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 +790,15 @@ class DownloadProtectionService::CheckClientDownloadRequest
base::TimeTicks::Now() - timeout_start_time_);
}
}
+ if (result == SAFE && (reason == REASON_WHITELISTED_URL ||
+ reason == REASON_TRUSTED_EXECUTABLE)) {
+ // Due to the short-circuit logic in CheckWhitelists (see TODOs there), a
+ // ClientDownloadRequest was not generated for this download and callbacks
+ // were not run. Run them now with null to indicate that a download has
+ // taken place.
+ // 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 +978,13 @@ bool DownloadProtectionService::IsSupportedDownload(
#endif
}
+DownloadProtectionService::ClientDownloadRequestSubscription
+DownloadProtectionService::RegisterClientDownloadRequestCallback(
+ const ClientDownloadRequestCallback& callback) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ return client_download_request_callbacks_.Add(callback);
+}
+
void DownloadProtectionService::CancelPendingRequests() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
for (std::set<scoped_refptr<CheckClientDownloadRequest> >::iterator it =

Powered by Google App Engine
This is Rietveld 408576698