Adding active_downloads_ map.

chrome/browser/download/download_manager.cc

Index: chrome/browser/download/download_manager.cc
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index da878b0f2687bf7823138c1062daaa22da64b5f1..3713d2708be61f0779e860fe03bd0a64e0437594 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -120,6 +120,7 @@ void DownloadManager::Shutdown() {
 
   // And clear all non-owning containers.
   in_progress_.clear();
+  active_downloads_.clear();
 #if !defined(NDEBUG)
   save_page_as_downloads_.clear();
 #endif
@@ -427,7 +428,9 @@ void DownloadManager::CreateDownloadItem(DownloadCreateInfo* info) {
   DownloadItem* download = new DownloadItem(this, *info,
                                             profile_->IsOffTheRecord());
   DCHECK(!ContainsKey(in_progress_, info->download_id));
+  DCHECK(!ContainsKey(active_downloads_, info->download_id));
   downloads_.insert(download);
+  active_downloads_[info->download_id] = download;
 }
 
 void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info,
@@ -437,22 +440,14 @@ void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info,
   scoped_ptr<DownloadCreateInfo> infop(info);
   info->path = target_path;
 
-  // NOTE(ahendrickson) We will be adding a new map |active_downloads_|, into
-  // which we will be adding the download as soon as it's created.  This will
-  // make this loop unnecessary.
-  // Eventually |active_downloads_| will replace |in_progress_|, but we don't
-  // want to change the semantics yet.
+  // NOTE(ahendrickson) Eventually |active_downloads_| will replace
+  // |in_progress_|, but we don't want to change the semantics yet.
   DCHECK(!ContainsKey(in_progress_, info->download_id));
-  DownloadItem* download = NULL;
-  for (std::set<DownloadItem*>::iterator i = downloads_.begin();
-       i != downloads_.end(); ++i) {
-    DownloadItem* item = (*i);
-    if (item && (item->id() == info->download_id)) {
-      download = item;
-      break;
-    }
-  }
+  DCHECK(ContainsKey(active_downloads_, info->download_id));
+  DownloadItem* download = active_downloads_[info->download_id];
   DCHECK(download != NULL);
+  DCHECK(downloads_.find(download) != downloads_.end());

[Paweł Hajdan Jr., 2011/01/05 08:11:16]

nit: If possible, use ContainsKey.

[ahendrickson, 2011/01/06 16:48:52]

Thanks, I was looking for that!

+
   download->SetFileCheckResults(info->path,
                                 info->is_dangerous,
                                 info->path_uniquifier,
@@ -506,8 +501,9 @@ void DownloadManager::AttachDownloadItem(DownloadCreateInfo* info,
 }
 
 void DownloadManager::UpdateDownload(int32 download_id, int64 size) {
-  DownloadMap::iterator it = in_progress_.find(download_id);
-  if (it != in_progress_.end()) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadMap::iterator it = active_downloads_.find(download_id);
+  if (it != active_downloads_.end()) {
     DownloadItem* download = it->second;
     download->Update(size);
     download_history_->UpdateEntry(download);
@@ -580,6 +576,12 @@ void DownloadManager::OnAllDataSaved(int32 download_id, int64 size) {
   download->OnSafeDownloadFinished(file_manager_);
 }
 
+void DownloadManager::OnDownloadFileCompleted(int32 download_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  active_downloads_.erase(download_id);
+}
+
 void DownloadManager::DownloadRenamedToFinalName(int download_id,
                                                  const FilePath& full_path) {
   VLOG(20) << __FUNCTION__ << "()" << " download_id = " << download_id
@@ -649,6 +651,7 @@ void DownloadManager::DangerousDownloadRenamed(int64 download_handle,
 }
 
 void DownloadManager::DownloadCancelled(int32 download_id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = in_progress_.find(download_id);
   if (it == in_progress_.end())
     return;
@@ -661,6 +664,7 @@ void DownloadManager::DownloadCancelled(int32 download_id) {
   // don't have a valid db_handle yet.
   if (download->db_handle() != DownloadHistory::kUninitializedHandle) {
     in_progress_.erase(it);
+    active_downloads_.erase(download_id);
     download_history_->UpdateEntry(download);
   }
 
@@ -959,6 +963,7 @@ void DownloadManager::OnQueryDownloadEntriesComplete(
 void DownloadManager::OnCreateDownloadEntryComplete(
     DownloadCreateInfo info,
     int64 db_handle) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = in_progress_.find(info.download_id);
   DCHECK(it != in_progress_.end());
 
@@ -983,7 +988,7 @@ void DownloadManager::OnCreateDownloadEntryComplete(
          history_downloads_.end());
   history_downloads_[download->db_handle()] = download;
 
-  // Show in the appropropriate browser UI.
+  // Show in the appropriate browser UI.
   ShowDownloadInBrowser(info, download);
 
   // Inform interested objects about the new download.
@@ -995,6 +1000,12 @@ void DownloadManager::OnCreateDownloadEntryComplete(
   // observers so that they get more than just the start notification.
   if (download->state() != DownloadItem::IN_PROGRESS) {
     in_progress_.erase(it);
+    // TODO(ahendrickson) -- We don't actually know whether or not we can
+    // remove the download item from the |active_downloads_| map, as there
+    // is no state in |DownloadItem::DownloadState| to indicate that the
+    // downloads system is done with an item.  Fix this when we have a
+    // proper final state to check for.
+    active_downloads_.erase(info.download_id);
     download_history_->UpdateEntry(download);
     download->UpdateObservers();
   }
@@ -1048,7 +1059,7 @@ void DownloadManager::AssertContainersConsistent() const {
 #if !defined(NDEBUG)
   // Turn everything into sets.
   DownloadSet in_progress_set, history_set;
-  const DownloadMap* input_maps[] = {&in_progress_, &history_downloads_};
+  const DownloadMap* input_maps[] = {&active_downloads_, &history_downloads_};
   DownloadSet* local_sets[] = {&in_progress_set, &history_set};
   DCHECK_EQ(ARRAYSIZE_UNSAFE(input_maps), ARRAYSIZE_UNSAFE(local_sets));
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(input_maps); i++) {