downloads: add the ability to undo download removal.

content/browser/download/download_manager_impl.cc

Index: content/browser/download/download_manager_impl.cc
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index 30296cb82e7bffbcf3716adf1e166e31be1069a9..236adb973146a163f6811d50abcc2dab13d2471e 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -245,6 +245,7 @@ DownloadManagerImpl::DownloadManagerImpl(
 
 DownloadManagerImpl::~DownloadManagerImpl() {
   DCHECK(!shutdown_needed_);
+  DCHECK(removed_downloads_.empty());

[Dan Beam, 2014/11/19 23:16:26]

I could change this to:

  STLDeleteValues(&removed_downloads_);

if you want... but it shouldn't ever be necessary assuming all
DownloadsDomHandlers go down before this class

 }
 
 DownloadItemImpl* DownloadManagerImpl::CreateActiveItem(
@@ -345,7 +346,6 @@ void DownloadManagerImpl::Shutdown() {
       download->Cancel(false);
   }
   STLDeleteValues(&downloads_);
-  downloads_.clear();

[Dan Beam, 2014/11/19 23:16:26]

STLDeleteValues() does this already

 
   // We'll have nothing more to report to the observers after this point.
   observers_.Clear();
@@ -563,7 +563,8 @@ void DownloadManagerImpl::DownloadRemoved(DownloadItemImpl* download) {
   uint32 download_id = download->GetId();
   if (downloads_.erase(download_id) == 0)
     return;
-  delete download;
+
+  removed_downloads_[download_id] = download;
 }
 
 int DownloadManagerImpl::RemoveDownloadsBetween(base::Time remove_begin,
@@ -598,6 +599,29 @@ int DownloadManagerImpl::RemoveAllDownloads() {
   return num_deleted;
 }
 
+void DownloadManagerImpl::ReviveDownload(uint32 id) {
+  auto it = removed_downloads_.find(id);
+  if (it == removed_downloads_.end())
+    return;
+
+  DownloadItemImpl* item = removed_downloads_[id];
+  removed_downloads_.erase(it);
+
+  downloads_[id] = item;
+
+  FOR_EACH_OBSERVER(Observer, observers_, OnDownloadRevived(this, item));
+  FOR_EACH_OBSERVER(Observer, observers_, OnDownloadCreated(this, item));

[benjhayden, 2014/11/21 00:50:01]

Is every listener really ok with having a single item created more than once?

[Dan Beam, 2014/11/21 05:32:21]

According to my interpretation of the documentation, yes.

"this method may be called an arbitrary number of times"

https://code.google.com/p/chromium/codesearch#chromium/src/content/public/bro...

+}
+
+void DownloadManagerImpl::FinalizeRemoval(uint32 id) {
+  auto it = removed_downloads_.find(id);
+  if (it == removed_downloads_.end())
+    return;
+
+  delete it->second;
+  removed_downloads_.erase(it);
+}
+
 void DownloadManagerImpl::DownloadUrl(
     scoped_ptr<DownloadUrlParameters> params) {
   if (params->post_id() >= 0) {