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 b7b4af6fdca4bb335febbcf51ce79690289d6a27..3fec07e7020ead50e96076ac5894ddd46e259064 100644 |
--- a/chrome/browser/download/chrome_download_manager_delegate.cc |
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc |
@@ -11,13 +11,13 @@ |
#include "base/path_service.h" |
#include "base/rand_util.h" |
#include "base/stringprintf.h" |
+#include "base/time.h" |
#include "chrome/browser/browser_process.h" |
#include "chrome/browser/download/download_crx_util.h" |
#include "chrome/browser/download/download_extensions.h" |
#include "chrome/browser/download/download_file_picker.h" |
#include "chrome/browser/download/download_history.h" |
#include "chrome/browser/download/download_prefs.h" |
-#include "chrome/browser/download/download_safe_browsing_client.h" |
#include "chrome/browser/download/download_util.h" |
#include "chrome/browser/download/save_package_file_picker.h" |
#include "chrome/browser/extensions/crx_installer.h" |
@@ -41,6 +41,7 @@ |
#include "ui/base/l10n/l10n_util.h" |
using content::BrowserThread; |
+using safe_browsing::DownloadProtectionService; |
ChromeDownloadManagerDelegate::ChromeDownloadManagerDelegate(Profile* profile) |
: profile_(profile), |
@@ -86,17 +87,22 @@ bool ChromeDownloadManagerDelegate::ShouldStartDownload(int32 download_id) { |
return false; |
#if defined(ENABLE_SAFE_BROWSING) |
- // Create a client to verify download URL with safebrowsing. |
- // It deletes itself after the callback. |
- scoped_refptr<DownloadSBClient> sb_client = new DownloadSBClient( |
- download_id, download->url_chain(), download->referrer_url(), |
- profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)); |
- sb_client->CheckDownloadUrl( |
- base::Bind(&ChromeDownloadManagerDelegate::CheckDownloadUrlDone, |
- base::Unretained(this))); |
-#else |
- CheckDownloadUrlDone(download_id, false); |
+ SafeBrowsingService* sb_service = |
+ g_browser_process->safe_browsing_service(); |
+ if (sb_service && sb_service->download_protection_service() && |
+ profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled)) { |
+ VLOG(2) << __FUNCTION__ << "() Start SB URL check for download = " |
+ << download->DebugString(false); |
+ sb_service->download_protection_service()->CheckDownloadUrl( |
+ DownloadProtectionService::DownloadInfo::FromDownloadItem(*download), |
+ base::Bind( |
+ &ChromeDownloadManagerDelegate::CheckDownloadUrlDone, |
+ this, |
+ download->id())); |
+ return false; |
+ } |
#endif |
+ CheckDownloadUrlDone(download_id, DownloadProtectionService::SAFE); |
return false; |
} |
@@ -147,29 +153,41 @@ bool ChromeDownloadManagerDelegate::ShouldOpenFileBasedOnExtension( |
} |
bool ChromeDownloadManagerDelegate::ShouldCompleteDownload(DownloadItem* item) { |
+#if defined(ENABLE_SAFE_BROWSING) |
+ // See if there is already a pending SafeBrowsing check for that download. |
+ SafeBrowsingStateMap::iterator it = safe_browsing_state_.find(item->id()); |
+ if (it != safe_browsing_state_.end()) { |
+ SafeBrowsingState state = it->second; |
+ if (!state.pending) { |
+ safe_browsing_state_.erase(it); |
+ } |
+ 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)) { |
+ VLOG(2) << __FUNCTION__ << "() Start SB download check for download = " |
+ << item->DebugString(false); |
+ sb_service->download_protection_service()->CheckClientDownload( |
+ DownloadProtectionService::DownloadInfo::FromDownloadItem(*item), |
+ base::Bind( |
+ &ChromeDownloadManagerDelegate::CheckClientDownloadDone, |
+ this, |
+ item->id())); |
+ SafeBrowsingState state; |
+ state.pending = true; |
+ state.verdict = DownloadProtectionService::SAFE; |
+ safe_browsing_state_[item->id()] = state; |
+ return false; |
+ } |
+#endif |
return true; |
} |
bool ChromeDownloadManagerDelegate::ShouldOpenDownload(DownloadItem* item) { |
if (!IsExtensionDownload(item)) { |
-#if defined(ENABLE_SAFE_BROWSING) |
- // 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)) { |
- using safe_browsing::DownloadProtectionService; |
- sb_service->download_protection_service()->CheckClientDownload( |
- DownloadProtectionService::DownloadInfo::FromDownloadItem(*item), |
- base::Bind( |
- &ChromeDownloadManagerDelegate::CheckClientDownloadDone, |
- this, item->id())); |
- // For now, we won't delay the download for this. |
- } |
-#else |
- // Assume safe. |
-#endif |
- |
return true; |
} |
@@ -201,24 +219,7 @@ bool ChromeDownloadManagerDelegate::GenerateFileHash() { |
} |
void ChromeDownloadManagerDelegate::OnResponseCompleted(DownloadItem* item) { |
-#if defined(ENABLE_SAFE_BROWSING) |
- // When hash is not available, it means either it is not calculated |
- // or there is error while it is calculated. We will skip the download hash |
- // check in that case. |
- if (item->hash().empty()) |
- return; |
- |
- scoped_refptr<DownloadSBClient> sb_client = |
- new DownloadSBClient(item->id(), |
- item->url_chain(), |
- item->referrer_url(), |
- profile_->GetPrefs()->GetBoolean( |
- prefs::kSafeBrowsingEnabled)); |
- sb_client->CheckDownloadHash( |
- item->hash(), |
- base::Bind(&ChromeDownloadManagerDelegate::CheckDownloadHashDone, |
- base::Unretained(this))); |
-#endif |
+ // TODO(noelutz): remove this method from the delegate API. |
jam
2012/01/27 23:06:34
ping: can you send me a change to remove this? the
|
} |
void ChromeDownloadManagerDelegate::AddItemToPersistentStore( |
@@ -239,12 +240,6 @@ void ChromeDownloadManagerDelegate::UpdatePathForItemInPersistentStore( |
download_history_->UpdateDownloadPath(item, new_path); |
} |
-void ChromeDownloadManagerDelegate::CheckClientDownloadDone( |
- int32 download_id, |
- safe_browsing::DownloadProtectionService::DownloadCheckResult result) { |
- // TODO(bryner): notify the user based on this result |
-} |
- |
void ChromeDownloadManagerDelegate::RemoveItemFromPersistentStore( |
DownloadItem* item) { |
download_history_->RemoveEntry(item); |
@@ -306,15 +301,16 @@ void ChromeDownloadManagerDelegate::DownloadProgressUpdated() { |
void ChromeDownloadManagerDelegate::CheckDownloadUrlDone( |
int32 download_id, |
- bool is_dangerous_url) { |
+ DownloadProtectionService::DownloadCheckResult result) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); |
- |
DownloadItem* download = |
download_manager_->GetActiveDownloadItem(download_id); |
if (!download) |
return; |
- if (is_dangerous_url) |
+ VLOG(2) << __FUNCTION__ << "() download = " << download->DebugString(false) |
+ << " verdict = " << result; |
+ if (result == DownloadProtectionService::DANGEROUS) |
download->MarkUrlDangerous(); |
download_history_->CheckVisitedReferrerBefore( |
@@ -323,6 +319,31 @@ void ChromeDownloadManagerDelegate::CheckDownloadUrlDone( |
base::Unretained(this))); |
} |
+void ChromeDownloadManagerDelegate::CheckClientDownloadDone( |
+ int32 download_id, |
+ DownloadProtectionService::DownloadCheckResult result) { |
+ DownloadItem* item = download_manager_->GetActiveDownloadItem(download_id); |
+ if (!item) { |
+ safe_browsing_state_.erase(download_id); // Just in case. |
+ return; |
+ } |
+ |
+ 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. |
+ |
+ SafeBrowsingStateMap::iterator it = safe_browsing_state_.find(item->id()); |
+ DCHECK(it != safe_browsing_state_.end() && it->second.pending); |
+ if (it != safe_browsing_state_.end()) { |
+ it->second.pending = false; |
+ it->second.verdict = result; |
+ } |
+ download_manager_->MaybeCompleteDownload(item); |
+} |
+ |
// content::NotificationObserver implementation. |
void ChromeDownloadManagerDelegate::Observe( |
int type, |
@@ -520,6 +541,10 @@ 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(); |
@@ -554,14 +579,3 @@ void ChromeDownloadManagerDelegate::OnItemAddedToPersistentStore( |
db_handle = download_history_->GetNextFakeDbHandle(); |
download_manager_->OnItemAddedToPersistentStore(download_id, db_handle); |
} |
- |
-// TODO(noelutz): This function currently works as a callback place holder. |
-// Once we decide the hash check is reliable, we could move the |
-// MaybeCompleteDownload in OnAllDataSaved to this function. |
-void ChromeDownloadManagerDelegate::CheckDownloadHashDone( |
- int32 download_id, |
- bool is_dangerous_hash) { |
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); |
- DVLOG(1) << "CheckDownloadHashDone, download_id: " << download_id |
- << " is dangerous_hash: " << is_dangerous_hash; |
-} |