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

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: test fixes Created 6 years 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..5924f52a7ee36529df65852ac25900e481ddefc8 100644
--- a/chrome/browser/ui/webui/downloads_dom_handler.cc
+++ b/chrome/browser/ui/webui/downloads_dom_handler.cc
@@ -16,8 +16,10 @@
#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/supports_user_data.h"
#include "base/threading/thread.h"
#include "base/value_conversions.h"
#include "base/values.h"
@@ -75,6 +77,35 @@ enum DownloadsDOMEvent {
DOWNLOADS_DOM_EVENT_MAX
};
+static const char kKey[] = "DownloadsDOMHandlerData";
+
+class DownloadsDOMHandlerData : public base::SupportsUserData::Data {
+ public:
+ static DownloadsDOMHandlerData* Get(content::DownloadItem* item) {
+ return static_cast<DownloadsDOMHandlerData*>(item->GetUserData(kKey));
+ }
+
+ static const DownloadsDOMHandlerData* Get(const content::DownloadItem* item) {
+ return static_cast<DownloadsDOMHandlerData*>(item->GetUserData(kKey));
+ }
+
+ static void Set(content::DownloadItem* item, DownloadsDOMHandlerData* data) {
+ item->SetUserData(kKey, data);
+ }
+
+ static DownloadsDOMHandlerData* Create(content::DownloadItem* item) {
+ DownloadsDOMHandlerData* data = new DownloadsDOMHandlerData;
+ item->SetUserData(kKey, data);
+ return data;
+ }
+
+ void set_is_removed(bool is_removed) { is_removed_ = is_removed; }
+ bool is_removed() const { return is_removed_; }
+
+ private:
+ bool is_removed_;
+};
+
void CountDownloadsDOMEvents(DownloadsDOMEvent event) {
UMA_HISTOGRAM_ENUMERATION("Download.DOMEvent",
event,
@@ -131,7 +162,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()));
base::FilePath download_path(download_item->GetTargetFilePath());
file_value->Set("file_path", base::CreateFilePathValue(download_path));
@@ -230,7 +261,7 @@ base::DictionaryValue* CreateDownloadItemValue(
break;
case content::DownloadItem::MAX_DOWNLOAD_STATE:
- NOTREACHED() << "state undefined";
+ NOTREACHED();
}
return file_value;
@@ -238,10 +269,12 @@ base::DictionaryValue* CreateDownloadItemValue(
// Filters out extension downloads and downloads that don't have a filename yet.
bool IsDownloadDisplayable(const content::DownloadItem& item) {
- return (!download_crx_util::IsExtensionDownload(item) &&
- !item.IsTemporary() &&
- !item.GetFileNameToReportUser().empty() &&
- !item.GetTargetFilePath().empty());
+ const DownloadsDOMHandlerData* data = DownloadsDOMHandlerData::Get(&item);
+ return !download_crx_util::IsExtensionDownload(item) &&
+ !item.IsTemporary() &&
+ !item.GetFileNameToReportUser().empty() &&
+ !item.GetTargetFilePath().empty() &&
+ (!data || !data->is_removed());
}
} // namespace
@@ -262,6 +295,12 @@ DownloadsDOMHandler::DownloadsDOMHandler(content::DownloadManager* dlm)
}
DownloadsDOMHandler::~DownloadsDOMHandler() {
+ while (!removed_ids_.empty()) {
+ content::DownloadItem* download = GetDownloadById(removed_ids_.back());
+ removed_ids_.pop_back();
+ if (download)
+ download->Remove();
+ }
}
// DownloadsDOMHandler, public: -----------------------------------------------
@@ -301,6 +340,9 @@ void DownloadsDOMHandler::RegisterMessages() {
web_ui()->RegisterMessageCallback("remove",
base::Bind(&DownloadsDOMHandler::HandleRemove,
weak_ptr_factory_.GetWeakPtr()));
+ web_ui()->RegisterMessageCallback("undo",
+ base::Bind(&DownloadsDOMHandler::HandleUndo,
+ weak_ptr_factory_.GetWeakPtr()));
web_ui()->RegisterMessageCallback("cancel",
base::Bind(&DownloadsDOMHandler::HandleCancel,
weak_ptr_factory_.GetWeakPtr()));
@@ -340,12 +382,18 @@ void DownloadsDOMHandler::OnDownloadUpdated(
(original_notifier_.get() &&
(manager == main_notifier_.GetManager()))));
CallDownloadUpdated(results_value);
+ } else {
+ ScheduleSendCurrentDownloads();
}
}
void DownloadsDOMHandler::OnDownloadRemoved(
content::DownloadManager* manager,
content::DownloadItem* download_item) {
+ DownloadsDOMHandlerData* data = DownloadsDOMHandlerData::Get(download_item);
+ if (data && data->is_removed())
+ return;
+
// This relies on |download_item| being removed from DownloadManager in this
// MessageLoop iteration. |download_item| may not have been removed from
// DownloadManager when OnDownloadRemoved() is fired, so bounce off the
@@ -438,8 +486,28 @@ void DownloadsDOMHandler::HandleRemove(const base::ListValue* args) {
CountDownloadsDOMEvents(DOWNLOADS_DOM_EVENT_REMOVE);
content::DownloadItem* file = GetDownloadByValue(args);
- if (file)
- file->Remove();
+ if (!file)
+ return;
+
+ removed_ids_.push_back(file->GetId());
+ DownloadsDOMHandlerData::Create(file)->set_is_removed(true);
+ file->UpdateObservers();
+}
+
+void DownloadsDOMHandler::HandleUndo(const base::ListValue* args) {
+ // TODO(dbeam): handle more than removed downloads someday?
+ if (removed_ids_.empty())
+ return;
+
+ uint32 last_removed_id = removed_ids_.back();
+ removed_ids_.pop_back();
+
+ content::DownloadItem* download = GetDownloadById(last_removed_id);
+ if (!download)
+ return;
+
+ DownloadsDOMHandlerData::Set(download, nullptr);
+ download->UpdateObservers();
}
void DownloadsDOMHandler::HandleCancel(const base::ListValue* args) {
@@ -515,7 +583,7 @@ void DownloadsDOMHandler::SendCurrentDownloads() {
query.Limit(kMaxDownloads);
query.Search(all_items.begin(), all_items.end(), &filtered_items);
base::ListValue results_value;
- for (content::DownloadManager::DownloadVector::const_iterator
+ for (content::DownloadManager::DownloadVector::iterator
iter = filtered_items.begin(); iter != filtered_items.end(); ++iter) {
results_value.Append(CreateDownloadItemValue(
*iter,
@@ -563,14 +631,27 @@ bool DownloadsDOMHandler::IsDeletingHistoryAllowed() {
content::DownloadItem* DownloadsDOMHandler::GetDownloadByValue(
const base::ListValue* args) {
- int download_id = -1;
- if (!ExtractIntegerValue(args, &download_id))
- return NULL;
+ std::string download_id;
+ if (!args->GetString(0, &download_id)) {
+ NOTREACHED();
+ return nullptr;
+ }
+
+ uint64 id;
+ if (!base::StringToUint64(download_id, &id)) {
+ NOTREACHED();
+ return nullptr;
+ }
+
+ return GetDownloadById(static_cast<uint32>(id));
+}
+
+content::DownloadItem* DownloadsDOMHandler::GetDownloadById(uint32 id) {
content::DownloadItem* item = NULL;
if (main_notifier_.GetManager())
- item = main_notifier_.GetManager()->GetDownload(download_id);
+ item = main_notifier_.GetManager()->GetDownload(id);
if (!item && original_notifier_.get() && original_notifier_->GetManager())
- item = original_notifier_->GetManager()->GetDownload(download_id);
+ item = original_notifier_->GetManager()->GetDownload(id);
return item;
}
« no previous file with comments | « chrome/browser/ui/webui/downloads_dom_handler.h ('k') | chrome/browser/ui/webui/downloads_dom_handler_browsertest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698