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

Unified Diff: chrome/browser/ui/webui/downloads_dom_handler.cc

Issue 722953002: downloads: add the ability to undo download removal. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: comment 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/ui/webui/downloads_dom_handler.cc
diff --git a/chrome/browser/ui/webui/downloads_dom_handler.cc b/chrome/browser/ui/webui/downloads_dom_handler.cc
index f347ba33b455b8cfb0f31555b804c19ea5752ca5..2ee32bf88ae1ac8cf25f9d89b8a3e2647c4003a9 100644
--- a/chrome/browser/ui/webui/downloads_dom_handler.cc
+++ b/chrome/browser/ui/webui/downloads_dom_handler.cc
@@ -16,6 +16,7 @@
#include "base/metrics/field_trial.h"
#include "base/metrics/histogram.h"
#include "base/prefs/pref_service.h"
+#include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread.h"
@@ -131,7 +132,7 @@ base::DictionaryValue* CreateDownloadItemValue(
download_item->GetStartTime(), NULL));
file_value->SetString(
"date_string", base::TimeFormatShortDate(download_item->GetStartTime()));
- file_value->SetInteger("id", download_item->GetId());
+ file_value->SetString("id", base::Uint64ToString(download_item->GetId()));
arv (Not doing code reviews) 2014/11/13 19:02:30 JS numbers cannot represent Uint64. This might lea
Dan Beam 2014/11/19 23:09:32 the JS only gets a string (now)
base::FilePath download_path(download_item->GetTargetFilePath());
file_value->Set("file_path", base::CreateFilePathValue(download_path));
@@ -156,7 +157,7 @@ base::DictionaryValue* CreateDownloadItemValue(
// Keep file names as LTR.
base::string16 file_name =
- download_item->GetFileNameToReportUser().LossyDisplayName();
+ download_item->GetFileNameToReportUser().LossyDisplayName();
file_name = base::i18n::GetDisplayStringInLTRDirectionality(file_name);
file_value->SetString("file_name", file_name);
file_value->SetString("url", download_item->GetURL().spec());
@@ -229,8 +230,9 @@ base::DictionaryValue* CreateDownloadItemValue(
file_value->SetString("state", "COMPLETE");
break;
+ case content::DownloadItem::REMOVED:
benjhayden 2014/11/13 18:01:59 How do you ensure that this isn't reached?
case content::DownloadItem::MAX_DOWNLOAD_STATE:
- NOTREACHED() << "state undefined";
+ NOTREACHED();
}
return file_value;
@@ -240,6 +242,7 @@ base::DictionaryValue* CreateDownloadItemValue(
bool IsDownloadDisplayable(const content::DownloadItem& item) {
return (!download_crx_util::IsExtensionDownload(item) &&
!item.IsTemporary() &&
+ item.GetState() != content::DownloadItem::REMOVED &&
!item.GetFileNameToReportUser().empty() &&
!item.GetTargetFilePath().empty());
}
@@ -262,6 +265,14 @@ DownloadsDOMHandler::DownloadsDOMHandler(content::DownloadManager* dlm)
}
DownloadsDOMHandler::~DownloadsDOMHandler() {
+ while (!removed_ids_.empty()) {
benjhayden 2014/11/13 18:01:59 Could you use DownloadQuery to find downloads whos
Dan Beam 2014/11/19 23:09:32 no because that'd give us all removed downloads, n
Dan Beam 2014/11/19 23:14:06 whoops, meant to delete this -- the code (in gener
+ uint32 remove_id = removed_ids_.back();
+ removed_ids_.pop_back();
+
+ content::DownloadItem* download = GetDownloadById(remove_id);
+ if (download)
+ download->Remove();
+ }
}
// DownloadsDOMHandler, public: -----------------------------------------------
@@ -310,6 +321,9 @@ void DownloadsDOMHandler::RegisterMessages() {
web_ui()->RegisterMessageCallback("openDownloadsFolder",
base::Bind(&DownloadsDOMHandler::HandleOpenDownloadsFolder,
weak_ptr_factory_.GetWeakPtr()));
+ web_ui()->RegisterMessageCallback("undoRemove",
+ base::Bind(&DownloadsDOMHandler::HandleUndoRemove,
+ weak_ptr_factory_.GetWeakPtr()));
}
void DownloadsDOMHandler::OnDownloadCreated(
@@ -340,6 +354,8 @@ void DownloadsDOMHandler::OnDownloadUpdated(
(original_notifier_.get() &&
(manager == main_notifier_.GetManager()))));
CallDownloadUpdated(results_value);
+ } else if (download_item->GetState() == content::DownloadItem::REMOVED) {
+ ScheduleSendCurrentDownloads();
}
}
@@ -438,8 +454,10 @@ void DownloadsDOMHandler::HandleRemove(const base::ListValue* args) {
CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_REMOVE);
content::DownloadItem* file = GetDownloadByValue(args);
- if (file)
- file->Remove();
+ if (file) {
+ removed_ids_.push_back(file->GetId());
+ file->MarkRemoved();
+ }
}
void DownloadsDOMHandler::HandleCancel(const base::ListValue* args) {
@@ -480,6 +498,18 @@ void DownloadsDOMHandler::HandleOpenDownloadsFolder(
}
}
+void DownloadsDOMHandler::HandleUndoRemove(const base::ListValue* args) {
arv (Not doing code reviews) 2014/11/13 19:02:30 Don't we want to send an ID of some kind. My conce
+ if (removed_ids_.empty())
+ return;
+
+ uint32 last_removed = removed_ids_.back();
+ removed_ids_.pop_back();
+
+ content::DownloadItem* item = GetDownloadById(last_removed);
+ if (item)
+ item->UndoRemove();
+}
+
// DownloadsDOMHandler, private: ----------------------------------------------
void DownloadsDOMHandler::ScheduleSendCurrentDownloads() {
@@ -563,9 +593,23 @@ bool DownloadsDOMHandler::IsDeletingHistoryAllowed() {
content::DownloadItem* DownloadsDOMHandler::GetDownloadByValue(
const base::ListValue* args) {
- int download_id = -1;
- if (!ExtractIntegerValue(args, &download_id))
+ std::string download_id;
+ if (!args->GetString(0, &download_id)) {
+ NOTREACHED();
+ return NULL;
+ }
+
+ uint64 id;
+ if (!base::StringToUint64(download_id, &id)) {
+ NOTREACHED();
return NULL;
+ }
+
+ return GetDownloadById(id);
+}
+
+content::DownloadItem* DownloadsDOMHandler::GetDownloadById(
+ uint32 download_id) {
content::DownloadItem* item = NULL;
if (main_notifier_.GetManager())
item = main_notifier_.GetManager()->GetDownload(download_id);

Powered by Google App Engine
This is Rietveld 408576698