summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/automation/testing_automation_provider.cc4
-rw-r--r--chrome/browser/download/download_browsertest.cc2
-rw-r--r--chrome/browser/download/download_shelf_context_menu.cc4
-rw-r--r--chrome/browser/extensions/api/downloads/downloads_api.cc2
-rw-r--r--chrome/browser/extensions/api/downloads/downloads_api_unittest.cc4
-rw-r--r--chrome/browser/extensions/api/streams_private/streams_private_apitest.cc2
-rw-r--r--chrome/browser/ui/cocoa/download/download_item_controller.mm6
-rw-r--r--chrome/browser/ui/gtk/download/download_item_gtk.cc6
-rw-r--r--chrome/browser/ui/views/download/download_item_view.cc6
-rw-r--r--chrome/browser/ui/webui/downloads_dom_handler.cc4
-rw-r--r--content/browser/android/download_controller_android_impl.cc4
-rw-r--r--content/browser/download/download_browsertest.cc4
-rw-r--r--content/browser/download/download_item_impl.cc80
-rw-r--r--content/browser/download/download_item_impl.h5
-rw-r--r--content/browser/download/download_item_impl_unittest.cc157
-rw-r--r--content/browser/download/download_manager_impl.cc33
-rw-r--r--content/browser/download/download_manager_impl_unittest.cc10
-rw-r--r--content/browser/download/download_stats.cc22
-rw-r--r--content/browser/download/download_stats.h16
-rw-r--r--content/public/browser/download_item.h29
-rw-r--r--content/public/test/download_test_observer.cc7
-rw-r--r--content/public/test/mock_download_item.h5
22 files changed, 261 insertions, 151 deletions
diff --git a/chrome/browser/automation/testing_automation_provider.cc b/chrome/browser/automation/testing_automation_provider.cc
index eeeb9c9..c9cf4df 100644
--- a/chrome/browser/automation/testing_automation_provider.cc
+++ b/chrome/browser/automation/testing_automation_provider.cc
@@ -2641,11 +2641,11 @@ void TestingAutomationProvider::PerformActionOnDownload(
} else if (action == "decline_dangerous_download") {
new AutomationProviderDownloadModelChangedObserver(
this, reply_message, download_manager);
- selected_item->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ selected_item->Remove();
} else if (action == "save_dangerous_download") {
selected_item->AddObserver(new AutomationProviderDownloadUpdatedObserver(
this, reply_message, false, browser->profile()->IsOffTheRecord()));
- selected_item->DangerousDownloadValidated();
+ selected_item->ValidateDangerousDownload();
} else if (action == "pause") {
if (!selected_item->IsInProgress() || selected_item->IsPaused()) {
// Action would be a no-op; respond right from here. No-op implies
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index f74c304..87107176 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -1782,7 +1782,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadHistoryDangerCheck) {
std::vector<DownloadItem*> downloads;
DownloadManagerForBrowser(browser())->GetAllDownloads(&downloads);
ASSERT_EQ(1u, downloads.size());
- downloads[0]->DangerousDownloadValidated();
+ downloads[0]->ValidateDangerousDownload();
download_observer->WaitForFinished();
// Get history details and confirm it's what you expect.
diff --git a/chrome/browser/download/download_shelf_context_menu.cc b/chrome/browser/download/download_shelf_context_menu.cc
index 48fb237..34e4bbe 100644
--- a/chrome/browser/download/download_shelf_context_menu.cc
+++ b/chrome/browser/download/download_shelf_context_menu.cc
@@ -137,10 +137,10 @@ void DownloadShelfContextMenu::ExecuteCommand(int command_id, int event_flags) {
}
break;
case DISCARD:
- download_item_->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download_item_->Remove();
break;
case KEEP:
- download_item_->DangerousDownloadValidated();
+ download_item_->ValidateDangerousDownload();
break;
case LEARN_MORE_SCANNING: {
#if defined(FULL_SAFE_BROWSING)
diff --git a/chrome/browser/extensions/api/downloads/downloads_api.cc b/chrome/browser/extensions/api/downloads/downloads_api.cc
index ddb8303..d3775df 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api.cc
@@ -1073,7 +1073,7 @@ void DownloadsAcceptDangerFunction::DangerPromptCallback(
DownloadItem* download_item = GetDownloadIfInProgress(
profile(), include_incognito(), download_id);
if (download_item)
- download_item->DangerousDownloadValidated();
+ download_item->ValidateDangerousDownload();
}
SendResponse(error_.empty());
}
diff --git a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
index 5d78e13..5034052 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api_unittest.cc
@@ -1099,7 +1099,7 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,
// Once the download item is deleted, we should return kInvalidOperationError.
int id = download_item->GetId();
- download_item->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download_item->Remove();
download_item = NULL;
EXPECT_EQ(static_cast<DownloadItem*>(NULL),
GetCurrentManager()->GetDownload(id));
@@ -2369,7 +2369,7 @@ IN_PROC_BROWSER_TEST_F(
" \"current\":false}}]",
result_id)));
- item->DangerousDownloadValidated();
+ item->ValidateDangerousDownload();
ASSERT_TRUE(WaitFor(events::kOnDownloadChanged,
base::StringPrintf("[{\"id\": %d,"
" \"dangerAccepted\": {"
diff --git a/chrome/browser/extensions/api/streams_private/streams_private_apitest.cc b/chrome/browser/extensions/api/streams_private/streams_private_apitest.cc
index 9e4a683..7fd3672 100644
--- a/chrome/browser/extensions/api/streams_private/streams_private_apitest.cc
+++ b/chrome/browser/extensions/api/streams_private/streams_private_apitest.cc
@@ -187,7 +187,7 @@ class StreamsPrivateApiTest : public ExtensionApiTest {
DownloadManager* manager) {
scoped_refptr<content::DownloadTestFlushObserver> flush_observer(
new content::DownloadTestFlushObserver(manager));
- download->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download->Remove();
flush_observer->WaitForFlush();
}
diff --git a/chrome/browser/ui/cocoa/download/download_item_controller.mm b/chrome/browser/ui/cocoa/download/download_item_controller.mm
index 4e9d747..9487cd4 100644
--- a/chrome/browser/ui/cocoa/download/download_item_controller.mm
+++ b/chrome/browser/ui/cocoa/download/download_item_controller.mm
@@ -306,16 +306,14 @@ class DownloadShelfContextMenuMac : public DownloadShelfContextMenu {
UMA_HISTOGRAM_LONG_TIMES("clickjacking.save_download",
base::Time::Now() - creationTime_);
// This will change the state and notify us.
- bridge_->download_model()->download()->DangerousDownloadValidated();
+ bridge_->download_model()->download()->ValidateDangerousDownload();
}
- (IBAction)discardDownload:(id)sender {
UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download",
base::Time::Now() - creationTime_);
DownloadItem* download = bridge_->download_model()->download();
- if (download->IsPartialDownload())
- download->Cancel(true);
- download->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download->Remove();
// WARNING: we are deleted at this point. Don't access 'this'.
}
diff --git a/chrome/browser/ui/gtk/download/download_item_gtk.cc b/chrome/browser/ui/gtk/download/download_item_gtk.cc
index 707fab27..ab51e23 100644
--- a/chrome/browser/ui/gtk/download/download_item_gtk.cc
+++ b/chrome/browser/ui/gtk/download/download_item_gtk.cc
@@ -918,13 +918,11 @@ gboolean DownloadItemGtk::OnDangerousPromptExpose(GtkWidget* widget,
void DownloadItemGtk::OnDangerousAccept(GtkWidget* button) {
UMA_HISTOGRAM_LONG_TIMES("clickjacking.save_download",
base::Time::Now() - creation_time_);
- download()->DangerousDownloadValidated();
+ download()->ValidateDangerousDownload();
}
void DownloadItemGtk::OnDangerousDecline(GtkWidget* button) {
UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download",
base::Time::Now() - creation_time_);
- if (download()->IsPartialDownload())
- download()->Cancel(true);
- download()->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download()->Remove();
}
diff --git a/chrome/browser/ui/views/download/download_item_view.cc b/chrome/browser/ui/views/download/download_item_view.cc
index dce9024..debec62c 100644
--- a/chrome/browser/ui/views/download/download_item_view.cc
+++ b/chrome/browser/ui/views/download/download_item_view.cc
@@ -524,9 +524,7 @@ void DownloadItemView::ButtonPressed(
if (sender == discard_button_) {
UMA_HISTOGRAM_LONG_TIMES("clickjacking.discard_download",
base::Time::Now() - creation_time_);
- if (download()->IsPartialDownload())
- download()->Cancel(true);
- download()->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download()->Remove();
// WARNING: we are deleted at this point. Don't access 'this'.
} else if (save_button_ && sender == save_button_) {
// The user has confirmed a dangerous download. We'd record how quickly the
@@ -534,7 +532,7 @@ void DownloadItemView::ButtonPressed(
UMA_HISTOGRAM_LONG_TIMES("clickjacking.save_download",
base::Time::Now() - creation_time_);
// This will change the state and notify us.
- download()->DangerousDownloadValidated();
+ download()->ValidateDangerousDownload();
}
}
diff --git a/chrome/browser/ui/webui/downloads_dom_handler.cc b/chrome/browser/ui/webui/downloads_dom_handler.cc
index fc22628..47fb18d 100644
--- a/chrome/browser/ui/webui/downloads_dom_handler.cc
+++ b/chrome/browser/ui/webui/downloads_dom_handler.cc
@@ -372,7 +372,7 @@ void DownloadsDOMHandler::HandleDiscardDangerous(const base::ListValue* args) {
CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_DISCARD_DANGEROUS);
content::DownloadItem* file = GetDownloadByValue(args);
if (file)
- file->Delete(content::DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ file->Remove();
}
void DownloadsDOMHandler::HandleShow(const base::ListValue* args) {
@@ -513,7 +513,7 @@ void DownloadsDOMHandler::DangerPromptAccepted(int download_id) {
if (!item || (item->GetState() != content::DownloadItem::IN_PROGRESS))
return;
CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_SAVE_DANGEROUS);
- item->DangerousDownloadValidated();
+ item->ValidateDangerousDownload();
}
bool DownloadsDOMHandler::IsDeletingHistoryAllowed() {
diff --git a/content/browser/android/download_controller_android_impl.cc b/content/browser/android/download_controller_android_impl.cc
index f6bbd83..2f7c271 100644
--- a/content/browser/android/download_controller_android_impl.cc
+++ b/content/browser/android/download_controller_android_impl.cc
@@ -364,9 +364,9 @@ void DownloadControllerAndroidImpl::DangerousDownloadValidated(
if (!item)
return;
if (accept)
- item->DangerousDownloadValidated();
+ item->ValidateDangerousDownload();
else
- item->Delete(content::DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ item->Remove();
}
DownloadControllerAndroidImpl::DownloadInfoAndroid::DownloadInfoAndroid(
diff --git a/content/browser/download/download_browsertest.cc b/content/browser/download/download_browsertest.cc
index 8f1e542..eec268d 100644
--- a/content/browser/download/download_browsertest.cc
+++ b/content/browser/download/download_browsertest.cc
@@ -701,7 +701,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, DownloadCancelled) {
ASSERT_EQ(DownloadItem::IN_PROGRESS, downloads[0]->GetState());
// Cancel the download and wait for download system quiesce.
- downloads[0]->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ downloads[0]->Cancel(true);
scoped_refptr<DownloadTestFlushObserver> flush_observer(
new DownloadTestFlushObserver(DownloadManagerForShell(shell())));
flush_observer->WaitForFlush();
@@ -1542,7 +1542,7 @@ IN_PROC_BROWSER_TEST_F(DownloadContentTest, CancelResumingDownload) {
// call, and that's for the second download created below.
EXPECT_CALL(dm_observer, OnDownloadCreated(_,_)).Times(1);
download->Resume();
- download->Cancel(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download->Cancel(true);
// The intermediate file should now be gone.
RunAllPendingInMessageLoop(BrowserThread::FILE);
diff --git a/content/browser/download/download_item_impl.cc b/content/browser/download/download_item_impl.cc
index e2bf57a..f4adec4 100644
--- a/content/browser/download/download_item_impl.cc
+++ b/content/browser/download/download_item_impl.cc
@@ -68,9 +68,12 @@ void DeleteDownloadedFile(const base::FilePath& path) {
// Wrapper around DownloadFile::Detach and DownloadFile::Cancel that
// takes ownership of the DownloadFile and hence implicitly destroys it
// at the end of the function.
-static void DownloadFileDetach(scoped_ptr<DownloadFile> download_file) {
+static base::FilePath DownloadFileDetach(
+ scoped_ptr<DownloadFile> download_file) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+ base::FilePath full_path = download_file->FullPath();
download_file->Detach();
+ return full_path;
}
static void DownloadFileCancel(scoped_ptr<DownloadFile> download_file) {
@@ -275,7 +278,7 @@ void DownloadItemImpl::UpdateObservers() {
FOR_EACH_OBSERVER(Observer, observers_, OnDownloadUpdated(this));
}
-void DownloadItemImpl::DangerousDownloadValidated() {
+void DownloadItemImpl::ValidateDangerousDownload() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK_EQ(IN_PROGRESS, GetState());
DCHECK(IsDangerous());
@@ -285,9 +288,7 @@ void DownloadItemImpl::DangerousDownloadValidated() {
if (GetState() != IN_PROGRESS)
return;
- UMA_HISTOGRAM_ENUMERATION("Download.DangerousDownloadValidated",
- GetDangerType(),
- DOWNLOAD_DANGER_TYPE_MAX);
+ RecordDangerousDownloadAccept(GetDangerType());
danger_type_ = DOWNLOAD_DANGER_TYPE_USER_VALIDATED;
@@ -300,6 +301,25 @@ void DownloadItemImpl::DangerousDownloadValidated() {
MaybeCompleteDownload();
}
+void DownloadItemImpl::StealDangerousDownload(
+ const AcquireFileCallback& callback) {
+ VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ DCHECK(IsDangerous());
+ if (download_file_) {
+ BrowserThread::PostTaskAndReplyWithResult(
+ BrowserThread::FILE,
+ FROM_HERE,
+ base::Bind(&DownloadFileDetach, base::Passed(&download_file_)),
+ callback);
+ } else {
+ callback.Run(current_path_);
+ }
+ current_path_.clear();
+ Remove();
+ // We have now been deleted.
+}
+
void DownloadItemImpl::Pause() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -350,9 +370,15 @@ void DownloadItemImpl::Cancel(bool user_cancel) {
return;
}
- last_reason_ = user_cancel ?
- DOWNLOAD_INTERRUPT_REASON_USER_CANCELED :
- DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN;
+ if (IsDangerous()) {
+ RecordDangerousDownloadDiscard(
+ user_cancel ? DOWNLOAD_DISCARD_DUE_TO_USER_ACTION
+ : DOWNLOAD_DISCARD_DUE_TO_SHUTDOWN,
+ GetDangerType());
+ }
+
+ last_reason_ = user_cancel ? DOWNLOAD_INTERRUPT_REASON_USER_CANCELED
+ : DOWNLOAD_INTERRUPT_REASON_USER_SHUTDOWN;
RecordDownloadCount(CANCELLED_COUNT);
@@ -381,36 +407,6 @@ void DownloadItemImpl::Cancel(bool user_cancel) {
TransitionTo(CANCELLED_INTERNAL);
}
-void DownloadItemImpl::Delete(DeleteReason reason) {
- VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
- switch (reason) {
- case DELETE_DUE_TO_USER_DISCARD:
- UMA_HISTOGRAM_ENUMERATION(
- "Download.UserDiscard", GetDangerType(),
- DOWNLOAD_DANGER_TYPE_MAX);
- break;
- case DELETE_DUE_TO_BROWSER_SHUTDOWN:
- UMA_HISTOGRAM_ENUMERATION(
- "Download.Discard", GetDangerType(),
- DOWNLOAD_DANGER_TYPE_MAX);
- break;
- default:
- NOTREACHED();
- }
-
- // Delete the file if it exists and is not owned by a DownloadFile object.
- // (In the latter case the DownloadFile object will delete it on cancel.)
- if (!current_path_.empty() && download_file_.get() == NULL) {
- BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
- base::Bind(&DeleteDownloadedFile, current_path_));
- current_path_.clear();
- }
- Remove();
- // We have now been deleted.
-}
-
void DownloadItemImpl::Remove() {
VLOG(20) << __FUNCTION__ << "() download = " << DebugString(true);
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
@@ -1399,9 +1395,11 @@ void DownloadItemImpl::ReleaseDownloadFile(bool destroy_file) {
current_path_.clear();
} else {
BrowserThread::PostTask(
- BrowserThread::FILE, FROM_HERE,
- // Will be deleted at end of task execution.
- base::Bind(&DownloadFileDetach, base::Passed(&download_file_)));
+ BrowserThread::FILE,
+ FROM_HERE,
+ base::Bind(base::IgnoreResult(&DownloadFileDetach),
+ // Will be deleted at end of task execution.
+ base::Passed(&download_file_)));
}
// Don't accept any more messages from the DownloadFile, and null
// out any previous "all data received". This also breaks links to
diff --git a/content/browser/download/download_item_impl.h b/content/browser/download/download_item_impl.h
index 0acc830..d1d0c36 100644
--- a/content/browser/download/download_item_impl.h
+++ b/content/browser/download/download_item_impl.h
@@ -90,11 +90,12 @@ class CONTENT_EXPORT DownloadItemImpl
virtual void AddObserver(DownloadItem::Observer* observer) OVERRIDE;
virtual void RemoveObserver(DownloadItem::Observer* observer) OVERRIDE;
virtual void UpdateObservers() OVERRIDE;
- virtual void DangerousDownloadValidated() OVERRIDE;
+ virtual void ValidateDangerousDownload() OVERRIDE;
+ virtual void StealDangerousDownload(const AcquireFileCallback& callback)
+ OVERRIDE;
virtual void Pause() OVERRIDE;
virtual void Resume() OVERRIDE;
virtual void Cancel(bool user_cancel) OVERRIDE;
- virtual void Delete(DeleteReason reason) OVERRIDE;
virtual void Remove() OVERRIDE;
virtual void OpenDownload() OVERRIDE;
virtual void ShowDownloadInShell() OVERRIDE;
diff --git a/content/browser/download/download_item_impl_unittest.cc b/content/browser/download/download_item_impl_unittest.cc
index fab8e99..baa2928 100644
--- a/content/browser/download/download_item_impl_unittest.cc
+++ b/content/browser/download/download_item_impl_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "base/callback.h"
#include "base/command_line.h"
#include "base/message_loop.h"
#include "base/stl_util.h"
@@ -13,8 +14,8 @@
#include "content/browser/download/download_item_impl_delegate.h"
#include "content/browser/download/download_request_handle.h"
#include "content/browser/download/mock_download_file.h"
-#include "content/public/browser/download_id.h"
#include "content/public/browser/download_destination_observer.h"
+#include "content/public/browser/download_id.h"
#include "content/public/browser/download_interrupt_reasons.h"
#include "content/public/browser/download_url_parameters.h"
#include "content/public/common/content_switches.h"
@@ -278,7 +279,8 @@ class DownloadItemTest : public testing::Test {
// Perform the intermediate rename for |item|. The target path for the
// download will be set to kDummyPath. Returns the MockDownloadFile* that was
// added to the DownloadItem.
- MockDownloadFile* DoIntermediateRename(DownloadItemImpl* item) {
+ MockDownloadFile* DoIntermediateRename(DownloadItemImpl* item,
+ DownloadDangerType danger_type) {
EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
EXPECT_TRUE(item->GetTargetFilePath().empty());
DownloadItemImplDelegate::DownloadTargetCallback callback;
@@ -291,7 +293,7 @@ class DownloadItemTest : public testing::Test {
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
intermediate_path));
callback.Run(target_path, DownloadItem::TARGET_DISPOSITION_OVERWRITE,
- DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS, intermediate_path);
+ danger_type, intermediate_path);
RunAllPendingInMessageLoops();
return download_file;
}
@@ -324,6 +326,11 @@ class DownloadItemTest : public testing::Test {
return &delegate_;
}
+ void OnDownloadFileAcquired(base::FilePath* return_path,
+ const base::FilePath& path) {
+ *return_path = path;
+ }
+
private:
base::MessageLoopForUI loop_;
TestBrowserThread ui_thread_; // UI thread
@@ -389,7 +396,8 @@ TEST_F(DownloadItemTest, NotificationAfterDownloadedFileRemoved) {
TEST_F(DownloadItemTest, NotificationAfterInterrupted) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
EXPECT_CALL(*download_file, Cancel());
MockObserver observer(item);
@@ -401,17 +409,6 @@ TEST_F(DownloadItemTest, NotificationAfterInterrupted) {
ASSERT_TRUE(observer.CheckUpdated());
}
-TEST_F(DownloadItemTest, NotificationAfterDelete) {
- DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = AddDownloadFileToDownloadItem(item, NULL);
- EXPECT_CALL(*download_file, Cancel());
- EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
- MockObserver observer(item);
-
- item->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
- ASSERT_TRUE(observer.CheckUpdated());
-}
-
TEST_F(DownloadItemTest, NotificationAfterDestroyed) {
DownloadItemImpl* item = CreateDownloadItem();
MockObserver observer(item);
@@ -427,9 +424,12 @@ TEST_F(DownloadItemTest, ContinueAfterInterrupted) {
DownloadItemImpl* item = CreateDownloadItem();
MockObserver observer(item);
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
// Interrupt the download, using a continuable interrupt.
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
item->DestinationObserverAsWeakPtr()->DestinationError(
DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR);
@@ -452,7 +452,8 @@ TEST_F(DownloadItemTest, RestartAfterInterrupted) {
DownloadItemImpl* item = CreateDownloadItem();
MockObserver observer(item);
DownloadItemImplDelegate::DownloadTargetCallback callback;
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
// Interrupt the download, using a restartable interrupt.
EXPECT_CALL(*download_file, Cancel());
@@ -491,6 +492,9 @@ TEST_F(DownloadItemTest, LimitRestartsAfterInterrupted) {
mock_request_handle = new NiceMock<MockRequestHandle>;
request_handle.reset(mock_request_handle);
+ ON_CALL(*mock_download_file, FullPath())
+ .WillByDefault(Return(base::FilePath()));
+
// It's too complicated to set up a WebContents instance that would cause
// the MockDownloadItemDelegate's ResumeInterruptedDownload() function
// to be callled, so we simply verify that GetWebContents() is called.
@@ -560,7 +564,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
unsafeurl_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_URL);
EXPECT_TRUE(unsafeurl_observer.CheckUpdated());
- unsafeurl_item->DangerousDownloadValidated();
+ unsafeurl_item->ValidateDangerousDownload();
EXPECT_TRUE(unsafeurl_observer.CheckUpdated());
DownloadItemImpl* unsafefile_item =
@@ -572,7 +576,7 @@ TEST_F(DownloadItemTest, NotificationAfterOnContentCheckCompleted) {
unsafefile_item->OnContentCheckCompleted(DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
EXPECT_TRUE(unsafefile_observer.CheckUpdated());
- unsafefile_item->DangerousDownloadValidated();
+ unsafefile_item->ValidateDangerousDownload();
EXPECT_TRUE(unsafefile_observer.CheckUpdated());
}
@@ -696,6 +700,8 @@ TEST_F(DownloadItemTest, CallbackAfterRename) {
EXPECT_CALL(*download_file, RenameAndAnnotate(final_path, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
final_path));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
item->DestinationObserverAsWeakPtr()->DestinationCompleted(std::string());
RunAllPendingInMessageLoops();
@@ -730,7 +736,8 @@ TEST_F(DownloadItemTest, CallbackAfterInterruptedRename) {
TEST_F(DownloadItemTest, Interrupted) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
const DownloadInterruptReason reason(
DOWNLOAD_INTERRUPT_REASON_FILE_ACCESS_DENIED);
@@ -829,7 +836,8 @@ TEST_F(DownloadItemTest, DestinationUpdate) {
TEST_F(DownloadItemTest, DestinationError) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
base::WeakPtr<DownloadDestinationObserver> as_observer(
item->DestinationObserverAsWeakPtr());
MockObserver observer(item);
@@ -879,7 +887,8 @@ TEST_F(DownloadItemTest, DestinationCompleted) {
TEST_F(DownloadItemTest, EnabledActionsForNormalDownload) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
// InProgress
ASSERT_TRUE(item->IsInProgress());
@@ -893,6 +902,8 @@ TEST_F(DownloadItemTest, EnabledActionsForNormalDownload) {
base::FilePath(kDummyPath)));
EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
.WillOnce(Return(true));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
item->DestinationObserverAsWeakPtr()->DestinationCompleted(std::string());
RunAllPendingInMessageLoops();
@@ -904,7 +915,8 @@ TEST_F(DownloadItemTest, EnabledActionsForNormalDownload) {
TEST_F(DownloadItemTest, EnabledActionsForTemporaryDownload) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
item->SetIsTemporary(true);
// InProgress Temporary
@@ -920,6 +932,8 @@ TEST_F(DownloadItemTest, EnabledActionsForTemporaryDownload) {
EXPECT_CALL(*download_file, RenameAndAnnotate(_, _))
.WillOnce(ScheduleRenameCallback(DOWNLOAD_INTERRUPT_REASON_NONE,
base::FilePath(kDummyPath)));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
item->DestinationObserverAsWeakPtr()->DestinationCompleted(std::string());
RunAllPendingInMessageLoops();
@@ -931,7 +945,8 @@ TEST_F(DownloadItemTest, EnabledActionsForTemporaryDownload) {
TEST_F(DownloadItemTest, EnabledActionsForInterruptedDownload) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
EXPECT_CALL(*download_file, Cancel());
item->DestinationObserverAsWeakPtr()->DestinationError(
@@ -946,7 +961,8 @@ TEST_F(DownloadItemTest, EnabledActionsForInterruptedDownload) {
TEST_F(DownloadItemTest, EnabledActionsForCancelledDownload) {
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
EXPECT_CALL(*download_file, Cancel());
item->Cancel(true);
@@ -964,7 +980,8 @@ TEST_F(DownloadItemTest, CompleteDelegate_ReturnTrue) {
// Test to confirm that if we have a callback that returns true,
// we complete immediately.
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
// Drive the delegate interaction.
EXPECT_CALL(*mock_delegate(), ShouldCompleteDownload(item, _))
@@ -979,6 +996,8 @@ TEST_F(DownloadItemTest, CompleteDelegate_ReturnTrue) {
base::FilePath(kDummyPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
RunAllPendingInMessageLoops();
EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
@@ -989,7 +1008,8 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockOnce) {
// Test to confirm that if we have a callback that returns true,
// we complete immediately.
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
// Drive the delegate interaction.
base::Closure delegate_callback;
@@ -1014,6 +1034,8 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockOnce) {
base::FilePath(kDummyPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
RunAllPendingInMessageLoops();
EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
@@ -1024,7 +1046,8 @@ TEST_F(DownloadItemTest, CompleteDelegate_SetDanger) {
// Test to confirm that if we have a callback that returns true,
// we complete immediately.
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
// Drive the delegate interaction.
base::Closure delegate_callback;
@@ -1052,12 +1075,14 @@ TEST_F(DownloadItemTest, CompleteDelegate_SetDanger) {
base::FilePath(kDummyPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
RunAllPendingInMessageLoops();
EXPECT_EQ(DownloadItem::IN_PROGRESS, item->GetState());
EXPECT_TRUE(item->IsDangerous());
- item->DangerousDownloadValidated();
+ item->ValidateDangerousDownload();
EXPECT_EQ(DOWNLOAD_DANGER_TYPE_USER_VALIDATED, item->GetDangerType());
RunAllPendingInMessageLoops();
EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
@@ -1068,7 +1093,8 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockTwice) {
// Test to confirm that if we have a callback that returns true,
// we complete immediately.
DownloadItemImpl* item = CreateDownloadItem();
- MockDownloadFile* download_file = DoIntermediateRename(item);
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
// Drive the delegate interaction.
base::Closure delegate_callback;
@@ -1100,11 +1126,82 @@ TEST_F(DownloadItemTest, CompleteDelegate_BlockTwice) {
base::FilePath(kDummyPath)));
EXPECT_CALL(*mock_delegate(), ShouldOpenDownload(item, _))
.WillOnce(Return(true));
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(base::FilePath()));
EXPECT_CALL(*download_file, Detach());
RunAllPendingInMessageLoops();
EXPECT_EQ(DownloadItem::COMPLETE, item->GetState());
}
+TEST_F(DownloadItemTest, StealDangerousDownload) {
+ DownloadItemImpl* item = CreateDownloadItem();
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
+ ASSERT_TRUE(item->IsDangerous());
+ base::FilePath full_path(FILE_PATH_LITERAL("foo.txt"));
+ base::FilePath returned_path;
+
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(full_path));
+ EXPECT_CALL(*download_file, Detach());
+ EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
+ base::WeakPtrFactory<DownloadItemTest> weak_ptr_factory(this);
+ item->StealDangerousDownload(
+ base::Bind(&DownloadItemTest::OnDownloadFileAcquired,
+ weak_ptr_factory.GetWeakPtr(),
+ base::Unretained(&returned_path)));
+ RunAllPendingInMessageLoops();
+ EXPECT_EQ(full_path, returned_path);
+}
+
+TEST_F(DownloadItemTest, StealInterruptedDangerousDownload) {
+ CommandLine::ForCurrentProcess()->AppendSwitch(
+ switches::kEnableDownloadResumption);
+ base::FilePath returned_path;
+ DownloadItemImpl* item = CreateDownloadItem();
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
+ base::FilePath full_path = item->GetFullPath();
+ EXPECT_FALSE(full_path.empty());
+ EXPECT_CALL(*download_file, FullPath())
+ .WillOnce(Return(full_path));
+ EXPECT_CALL(*download_file, Detach());
+ item->DestinationObserverAsWeakPtr()->DestinationError(
+ DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED);
+ ASSERT_TRUE(item->IsDangerous());
+
+ EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
+ base::WeakPtrFactory<DownloadItemTest> weak_ptr_factory(this);
+ item->StealDangerousDownload(
+ base::Bind(&DownloadItemTest::OnDownloadFileAcquired,
+ weak_ptr_factory.GetWeakPtr(),
+ base::Unretained(&returned_path)));
+ RunAllPendingInMessageLoops();
+ EXPECT_EQ(full_path, returned_path);
+}
+
+TEST_F(DownloadItemTest, StealInterruptedNonResumableDangerousDownload) {
+ CommandLine::ForCurrentProcess()->AppendSwitch(
+ switches::kEnableDownloadResumption);
+ base::FilePath returned_path;
+ DownloadItemImpl* item = CreateDownloadItem();
+ MockDownloadFile* download_file =
+ DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE);
+ EXPECT_CALL(*download_file, Cancel());
+ item->DestinationObserverAsWeakPtr()->DestinationError(
+ DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
+ ASSERT_TRUE(item->IsDangerous());
+
+ EXPECT_CALL(*mock_delegate(), DownloadRemoved(_));
+ base::WeakPtrFactory<DownloadItemTest> weak_ptr_factory(this);
+ item->StealDangerousDownload(
+ base::Bind(&DownloadItemTest::OnDownloadFileAcquired,
+ weak_ptr_factory.GetWeakPtr(),
+ base::Unretained(&returned_path)));
+ RunAllPendingInMessageLoops();
+ EXPECT_TRUE(returned_path.empty());
+}
+
TEST(MockDownloadItem, Compiles) {
MockDownloadItem mock_item;
}
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 16d1b29..7a2336e 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -335,35 +335,16 @@ void DownloadManagerImpl::Shutdown() {
FOR_EACH_OBSERVER(Observer, observers_, ManagerGoingDown(this));
// TODO(benjhayden): Consider clearing observers_.
- // Go through all downloads in downloads_. Dangerous ones we need to
- // remove on disk, and in progress ones we need to cancel.
- for (DownloadMap::iterator it = downloads_.begin(); it != downloads_.end();) {
+ // If there are in-progress downloads, cancel them. This also goes for
+ // dangerous downloads which will remain in history if they aren't explicitly
+ // accepted or discarded. Canceling will remove the intermediate download
+ // file.
+ for (DownloadMap::iterator it = downloads_.begin(); it != downloads_.end();
+ ++it) {
DownloadItemImpl* download = it->second;
-
- // Save iterator from potential erases in this set done by called code.
- // Iterators after an erasure point are still valid for lists and
- // associative containers such as sets.
- it++;
-
- if (download->IsDangerous() && download->IsPartialDownload()) {
- // The user hasn't accepted it, so we need to remove it
- // from the disk. This may or may not result in it being
- // removed from the DownloadManager queues and deleted
- // (specifically, DownloadManager::DownloadRemoved only
- // removes and deletes it if it's known to the history service)
- // so the only thing we know after calling this function is that
- // the download was deleted if-and-only-if it was removed
- // from all queues.
- download->Delete(DownloadItem::DELETE_DUE_TO_BROWSER_SHUTDOWN);
- } else if (download->IsPartialDownload()) {
+ if (download->GetState() == DownloadItem::IN_PROGRESS)
download->Cancel(false);
- }
}
-
- // At this point, all dangerous downloads have had their files removed
- // and all in progress downloads have been cancelled. We can now delete
- // anything left.
-
STLDeleteValues(&downloads_);
downloads_.clear();
diff --git a/content/browser/download/download_manager_impl_unittest.cc b/content/browser/download/download_manager_impl_unittest.cc
index 300eb2e..a6dd330 100644
--- a/content/browser/download/download_manager_impl_unittest.cc
+++ b/content/browser/download/download_manager_impl_unittest.cc
@@ -93,7 +93,8 @@ class MockDownloadItemImpl : public DownloadItemImpl {
MOCK_METHOD0(ShouldOpenFileBasedOnExtension, bool());
MOCK_METHOD0(OpenDownload, void());
MOCK_METHOD0(ShowDownloadInShell, void());
- MOCK_METHOD0(DangerousDownloadValidated, void());
+ MOCK_METHOD0(ValidateDangerousDownload, void());
+ MOCK_METHOD1(StealDangerousDownload, void(const AcquireFileCallback&));
MOCK_METHOD3(UpdateProgress, void(int64, int64, const std::string&));
MOCK_METHOD1(Cancel, void(bool));
MOCK_METHOD0(MarkAsComplete, void());
@@ -107,7 +108,6 @@ class MockDownloadItemImpl : public DownloadItemImpl {
MOCK_METHOD2(MockStart, void(DownloadFile*, DownloadRequestHandleInterface*));
- MOCK_METHOD1(Delete, void(DeleteReason));
MOCK_METHOD0(Remove, void());
MOCK_CONST_METHOD1(TimeRemaining, bool(base::TimeDelta*));
MOCK_CONST_METHOD0(CurrentSpeed, int64());
@@ -472,10 +472,8 @@ class DownloadManagerTest : public testing::Test {
virtual void TearDown() {
while (MockDownloadItemImpl*
item = mock_download_item_factory_->PopItem()) {
- EXPECT_CALL(*item, IsDangerous())
- .WillOnce(Return(false));
- EXPECT_CALL(*item, IsPartialDownload())
- .WillOnce(Return(false));
+ EXPECT_CALL(*item, GetState())
+ .WillOnce(Return(DownloadItem::CANCELLED));
}
EXPECT_CALL(GetMockObserver(), ManagerGoingDown(download_manager_.get()))
.WillOnce(Return());
diff --git a/content/browser/download/download_stats.cc b/content/browser/download/download_stats.cc
index e13200e..347f592 100644
--- a/content/browser/download/download_stats.cc
+++ b/content/browser/download/download_stats.cc
@@ -145,6 +145,28 @@ void RecordDownloadInterrupted(DownloadInterruptReason reason,
UMA_HISTOGRAM_BOOLEAN("Download.InterruptedUnknownSize", unknown_size);
}
+void RecordDangerousDownloadAccept(DownloadDangerType danger_type) {
+ UMA_HISTOGRAM_ENUMERATION("Download.DangerousDownloadValidated",
+ danger_type,
+ DOWNLOAD_DANGER_TYPE_MAX);
+}
+
+void RecordDangerousDownloadDiscard(DownloadDiscardReason reason,
+ DownloadDangerType danger_type) {
+ switch (reason) {
+ case DOWNLOAD_DISCARD_DUE_TO_USER_ACTION:
+ UMA_HISTOGRAM_ENUMERATION(
+ "Download.UserDiscard", danger_type, DOWNLOAD_DANGER_TYPE_MAX);
+ break;
+ case DOWNLOAD_DISCARD_DUE_TO_SHUTDOWN:
+ UMA_HISTOGRAM_ENUMERATION(
+ "Download.Discard", danger_type, DOWNLOAD_DANGER_TYPE_MAX);
+ break;
+ default:
+ NOTREACHED();
+ }
+}
+
void RecordDownloadWriteSize(size_t data_len) {
RecordDownloadCount(WRITE_SIZE_COUNT);
int max = 1024 * 1024; // One Megabyte.
diff --git a/content/browser/download/download_stats.h b/content/browser/download/download_stats.h
index 282d0e0..8b4f094 100644
--- a/content/browser/download/download_stats.h
+++ b/content/browser/download/download_stats.h
@@ -11,6 +11,7 @@
#include "base/basictypes.h"
#include "content/common/content_export.h"
+#include "content/public/browser/download_danger_type.h"
#include "content/public/browser/download_interrupt_reasons.h"
namespace base {
@@ -92,6 +93,14 @@ enum DownloadSource {
DOWNLOAD_SOURCE_LAST_ENTRY
};
+enum DownloadDiscardReason {
+ // The download is being discarded due to a user action.
+ DOWNLOAD_DISCARD_DUE_TO_USER_ACTION,
+
+ // The download is being discarded due to the browser being shut down.
+ DOWNLOAD_DISCARD_DUE_TO_SHUTDOWN
+};
+
// Increment one of the above counts.
void RecordDownloadCount(DownloadCountTypes type);
@@ -106,6 +115,13 @@ void RecordDownloadInterrupted(DownloadInterruptReason reason,
int64 received,
int64 total);
+// Record a dangerous download accept event.
+void RecordDangerousDownloadAccept(DownloadDangerType danger_type);
+
+// Record a dangerous download discard event.
+void RecordDangerousDownloadDiscard(DownloadDiscardReason reason,
+ DownloadDangerType danger_type);
+
// Records the mime type of the download.
void RecordDownloadMimeType(const std::string& mime_type);
diff --git a/content/public/browser/download_item.h b/content/public/browser/download_item.h
index b3003ef..2b80d7f 100644
--- a/content/public/browser/download_item.h
+++ b/content/public/browser/download_item.h
@@ -21,6 +21,7 @@
#include <string>
#include <vector>
+#include "base/callback_forward.h"
#include "base/files/file_path.h"
#include "base/string16.h"
#include "base/supports_user_data.h"
@@ -67,12 +68,6 @@ class CONTENT_EXPORT DownloadItem : public base::SupportsUserData {
MAX_DOWNLOAD_STATE
};
- // Reason for deleting the download. Passed to Delete().
- enum DeleteReason {
- DELETE_DUE_TO_BROWSER_SHUTDOWN = 0,
- DELETE_DUE_TO_USER_DISCARD
- };
-
// How the final target path should be used.
enum TargetDisposition {
TARGET_DISPOSITION_OVERWRITE, // Overwrite if the target already exists.
@@ -81,6 +76,9 @@ class CONTENT_EXPORT DownloadItem : public base::SupportsUserData {
// TARGET_DISPOSITION_OVERWRITE.
};
+ // Callback used with AcquireFileAndDeleteDownload().
+ typedef base::Callback<void(const base::FilePath&)> AcquireFileCallback;
+
static const char kEmptyFileHash[];
// Interface that observers of a particular download must implement in order
@@ -111,7 +109,14 @@ class CONTENT_EXPORT DownloadItem : public base::SupportsUserData {
// User Actions --------------------------------------------------------------
// Called when the user has validated the download of a dangerous file.
- virtual void DangerousDownloadValidated() = 0;
+ virtual void ValidateDangerousDownload() = 0;
+
+ // Called to acquire a dangerous download and remove the DownloadItem from
+ // views and history. |callback| will be invoked on the UI thread with the
+ // path to the downloaded file. The caller is responsible for cleanup.
+ // Note: It is important for |callback| to be valid since the downloaded file
+ // will not be cleaned up if the callback fails.
+ virtual void StealDangerousDownload(const AcquireFileCallback& callback) = 0;
// Pause a download. Will have no effect if the download is already
// paused.
@@ -132,11 +137,9 @@ class CONTENT_EXPORT DownloadItem : public base::SupportsUserData {
// when resuming a download (assuming the server supports byte ranges).
virtual void Cancel(bool user_cancel) = 0;
- // Deletes the file from disk and removes the download from the views and
- // history.
- virtual void Delete(DeleteReason reason) = 0;
-
- // Removes the download from the views and history.
+ // Removes the download from the views and history. If the download was
+ // in-progress or interrupted, then the intermediate file will also be
+ // deleted.
virtual void Remove() = 0;
// Open the file associated with this download. If the download is
@@ -241,7 +244,7 @@ class CONTENT_EXPORT DownloadItem : public base::SupportsUserData {
virtual bool GetFileExternallyRemoved() const = 0;
// True if the file that will be written by the download is dangerous
- // and we will require a call to DangerousDownloadValidated() to complete.
+ // and we will require a call to ValidateDangerousDownload() to complete.
// False if the download is safe or that function has been called.
virtual bool IsDangerous() const = 0;
diff --git a/content/public/test/download_test_observer.cc b/content/public/test/download_test_observer.cc
index 17d763ef..ee5961f 100644
--- a/content/public/test/download_test_observer.cc
+++ b/content/public/test/download_test_observer.cc
@@ -141,7 +141,7 @@ void DownloadTestObserver::OnDownloadUpdated(DownloadItem* download) {
!ContainsKey(dangerous_downloads_seen_, download->GetId())) {
dangerous_downloads_seen_.insert(download->GetId());
- // Calling DangerousDownloadValidated() at this point will
+ // Calling ValidateDangerousDownload() at this point will
// cause the download to be completed twice. Do what the real UI
// code does: make the call as a delayed task.
switch (dangerous_download_action_) {
@@ -222,7 +222,7 @@ void DownloadTestObserver::AcceptDangerousDownload(int32 download_id) {
return;
DownloadItem* download = download_manager_->GetDownload(download_id);
if (download && (download->GetState() == DownloadItem::IN_PROGRESS))
- download->DangerousDownloadValidated();
+ download->ValidateDangerousDownload();
}
void DownloadTestObserver::DenyDangerousDownload(int32 download_id) {
@@ -232,8 +232,7 @@ void DownloadTestObserver::DenyDangerousDownload(int32 download_id) {
return;
DownloadItem* download = download_manager_->GetDownload(download_id);
if (download && (download->GetState() == DownloadItem::IN_PROGRESS)) {
- download->Cancel(true);
- download->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
+ download->Remove();
}
}
diff --git a/content/public/test/mock_download_item.h b/content/public/test/mock_download_item.h
index 02ec038..0a84044 100644
--- a/content/public/test/mock_download_item.h
+++ b/content/public/test/mock_download_item.h
@@ -5,6 +5,7 @@
#ifndef CONTENT_PUBLIC_TEST_MOCK_DOWNLOAD_ITEM_H_
#define CONTENT_PUBLIC_TEST_MOCK_DOWNLOAD_ITEM_H_
+#include "base/callback.h"
#include "base/time.h"
#include "content/public/browser/download_id.h"
#include "content/public/browser/download_interrupt_reasons.h"
@@ -22,11 +23,11 @@ class MockDownloadItem : public DownloadItem {
MOCK_METHOD1(AddObserver, void(DownloadItem::Observer*));
MOCK_METHOD1(RemoveObserver, void(DownloadItem::Observer*));
MOCK_METHOD0(UpdateObservers, void());
- MOCK_METHOD0(DangerousDownloadValidated, void());
+ MOCK_METHOD0(ValidateDangerousDownload, void());
+ MOCK_METHOD1(StealDangerousDownload, void(const AcquireFileCallback&));
MOCK_METHOD0(Pause, void());
MOCK_METHOD0(Resume, void());
MOCK_METHOD1(Cancel, void(bool));
- MOCK_METHOD1(Delete, void(DeleteReason));
MOCK_METHOD0(Remove, void());
MOCK_METHOD0(OpenDownload, void());
MOCK_METHOD0(ShowDownloadInShell, void());