Propagate the SafeBrowsing download protection verdict to the DownloadItem.

chrome/browser/download/chrome_download_manager_delegate.cc

Index: chrome/browser/download/chrome_download_manager_delegate.cc
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 01d05695d348b7e5f2416b27d6c00154278f6817..2e4c160e88b94fa2d7a33c0cb49905d632d23c66 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -87,13 +87,11 @@ bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) {
     return false;
 
 #if defined(ENABLE_SAFE_BROWSING)
-  SafeBrowsingService* sb_service =
-      g_browser_process->safe_browsing_service();
-  if (sb_service && sb_service->download_protection_service() &&
-      profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
+  DownloadProtectionService* service = GetDownloadProtectionService();
+  if (service) {
     VLOG(2) << __FUNCTION__ << "() Start SB URL check for download = "
             << download->DebugString(false);
-    sb_service->download_protection_service()->CheckDownloadUrl(
+    service->CheckDownloadUrl(
         DownloadProtectionService::DownloadInfo::FromDownloadItem(*download),
         base::Bind(
             &ChromeDownloadManagerDelegate::CheckDownloadUrlDone,
@@ -118,9 +116,9 @@ void ChromeDownloadManagerDelegate::ChooseDownloadPath(
 bool ChromeDownloadManagerDelegate::OverrideIntermediatePath(
     DownloadItem* item,
     FilePath* intermediate_path) {
-  if (item->IsDangerous()) {
-    // The download is not safe.  It's name is already set to an intermediate
-    // name, so no need to override.
+  if (item->GetDangerType() != DownloadStateInfo::NOT_DANGEROUS) {
+    // The download might not be safe.  It's name is already set to an
+    // intermediate name, so no need to override.
     return false;
   }
 
@@ -164,13 +162,11 @@ bool ChromeDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) {
     return !state.pending;
   }
   // Begin the safe browsing download protection check.
-  SafeBrowsingService* sb_service =
-      g_browser_process->safe_browsing_service();
-  if (sb_service && sb_service->download_protection_service() &&
-      profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
+  DownloadProtectionService* service = GetDownloadProtectionService();
+  if (service) {
     VLOG(2) << __FUNCTION__ << "() Start SB download check for download = "
             << item->DebugString(false);
-    sb_service->download_protection_service()->CheckClientDownload(
+    service->CheckClientDownload(
         DownloadProtectionService::DownloadInfo::FromDownloadItem(*item),
         base::Bind(
             &ChromeDownloadManagerDelegate::CheckClientDownloadDone,
@@ -299,6 +295,18 @@ void ChromeDownloadManagerDelegate::DownloadProgressUpdated() {
       download_count, progress_known, progress);
 }
 
+#if defined(ENABLE_SAFE_BROWSING)
+DownloadProtectionService*
+    ChromeDownloadManagerDelegate::GetDownloadProtectionService() {
+  SafeBrowsingService* sb_service = g_browser_process->safe_browsing_service();
+  if (sb_service && sb_service->download_protection_service() &&
+      profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) {
+    return sb_service->download_protection_service();
+  }
+  return NULL;
+}
+#endif
+
 void ChromeDownloadManagerDelegate::CheckDownloadUrlDone(
     int32 download_id,
     DownloadProtectionService::DownloadCheckResult result) {
@@ -330,10 +338,11 @@ void ChromeDownloadManagerDelegate::CheckClientDownloadDone(
 
   VLOG(2) << __FUNCTION__ << "() download = " << item->DebugString(false)
           << " verdict = " << result;
-  // TODO(noelutz):
-  //  1) display a warning if the result is DANGEROUS.
-  //  2) make sure we haven't already displayed a warning for the URL.
-  //  3) disable the existing dangerous file warning for executables.
+  // We only mark the content as being dangerous if the download's safety state
+  // has not been set to DANGEROUS yet.  We don't want to show two warnings.
+  if (result == DownloadProtectionService::DANGEROUS &&
+      item->safety_state() == DownloadItem::SAFE)
+    item->MarkContentDangerous();
 
   SafeBrowsingStateMap::iterator it = safe_browsing_state_.find(item->id());
   DCHECK(it != safe_browsing_state_.end() && it->second.pending);
@@ -415,10 +424,24 @@ void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
     state.suggested_path = state.force_file_name;
   }
 
-  if (!state.prompt_user_for_save_location && state.force_file_name.empty()) {
-    state.is_dangerous_file =
-        IsDangerousFile(*download, state, visited_referrer_before);
+  if (!state.prompt_user_for_save_location &&
+      state.force_file_name.empty() &&
+      IsDangerousFile(*download, state, visited_referrer_before)) {
+    state.danger = DownloadStateInfo::DANGEROUS_FILE;
+  }
+
+#if defined(ENABLE_SAFE_BROWSING)
+  DownloadProtectionService* service = GetDownloadProtectionService();
+  // Return false if this type of files is handled by the enhanced SafeBrowsing
+  // download protection.
+  if (service && service->enabled() &&
+      service->IsSupportedFileType(state.suggested_path.BaseName())) {
+    // TODO(noelutz): if the user changes the extension name in the UI to
+    // something like .exe SafeBrowsing will currently *not* check if the
+    // download is malicious.
+    state.danger = DownloadStateInfo::MAYBE_DANGEROUS_CONTENT;
   }
+#endif
 
   // We need to move over to the download thread because we don't want to stat
   // the suggested path on the UI thread.
@@ -453,8 +476,8 @@ void ChromeDownloadManagerDelegate::CheckIfSuggestedPathExists(
     state.suggested_path = state.suggested_path.Append(filename);
   }
 
-  // If the download is deemed dangerous, we'll use a temporary name for it.
-  if (state.IsDangerous()) {
+  // If the download is possibly dangerous, we'll use a temporary name for it.
+  if (state.danger != DownloadStateInfo::NOT_DANGEROUS) {
     state.target_name = FilePath(state.suggested_path).BaseName();
     // Create a temporary file to hold the file until the user approves its
     // download.
@@ -506,7 +529,7 @@ void ChromeDownloadManagerDelegate::CheckIfSuggestedPathExists(
   // See: http://code.google.com/p/chromium/issues/detail?id=3662
   if (!state.prompt_user_for_save_location &&
       state.force_file_name.empty()) {
-    if (state.IsDangerous())
+    if (state.danger != DownloadStateInfo::NOT_DANGEROUS)
       file_util::WriteFile(state.suggested_path, "", 0);
     else
       file_util::WriteFile(download_util::GetCrDownloadPath(
@@ -541,10 +564,6 @@ bool ChromeDownloadManagerDelegate::IsDangerousFile(
   if (state.transition_type & content::PAGE_TRANSITION_FROM_ADDRESS_BAR)
     return false;
 
-  // Return false if this type of files is handled by the enhanced SafeBrowsing
-  // download protection.
-  // TODO(noelutz): implement that check.
-
   // Extensions that are not from the gallery are considered dangerous.
   if (IsExtensionDownload(&download)) {
     ExtensionService* service = profile_->GetExtensionService();