1 files changed, 104 insertions, 74 deletions

diff --git a/chrome/browser/download/download_file_manager.cc b/chrome/browser/download/download_file_manager.cc
index 4b7e860..7a1c298 100644
--- a/chrome/browser/download/download_file_manager.cc
+++ b/chrome/browser/download/download_file_manager.cc
@@ -54,7 +54,6 @@ DownloadFileManager::DownloadFileManager(ResourceDispatcherHost* rdh)
 
 DownloadFileManager::~DownloadFileManager() {
   DCHECK(downloads_.empty());
-  DCHECK(downloads_with_final_name_.empty());
 }
 
 void DownloadFileManager::Shutdown() {
@@ -67,7 +66,6 @@ void DownloadFileManager::Shutdown() {
 void DownloadFileManager::OnShutdown() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   StopUpdateTimer();
-  downloads_with_final_name_.clear();
   STLDeleteValues(&downloads_);
 }
 
@@ -211,31 +209,27 @@ void DownloadFileManager::OnResponseCompleted(int id, DownloadBuffer* buffer) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   delete buffer;
   DownloadFileMap::iterator it = downloads_.find(id);
-  if (it != downloads_.end()) {
-    DownloadFile* download = it->second;
-    download->Finish();
-
-    DownloadManager* download_manager = download->GetDownloadManager();
-    if (download_manager) {
-      std::string hash;
-      if (!download->GetSha256Hash(&hash))
-        hash.clear();
-
-      BrowserThread::PostTask(
-          BrowserThread::UI, FROM_HERE,
-          NewRunnableMethod(
-              download_manager, &DownloadManager::OnAllDataSaved,
-              id, download->bytes_so_far(), hash));
-    }
+  if (it == downloads_.end())
+    return;
+
+  DownloadFile* download = it->second;
+  download->Finish();
 
-    // We need to keep the download around until the UI thread has finalized
-    // the name.
-    if (ContainsKey(downloads_with_final_name_, id))
-      EraseDownload(id);
+  DownloadManager* download_manager = download->GetDownloadManager();
+  if (!download_manager) {
+    CancelDownload(id);
+    return;
   }
 
-  if (downloads_.empty())
-    StopUpdateTimer();
+  std::string hash;
+  if (!download->GetSha256Hash(&hash))
+    hash.clear();
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      NewRunnableMethod(
+          download_manager, &DownloadManager::OnAllDataSaved,
+          id, download->bytes_so_far(), hash));
 }
 
 // This method will be sent via a user action, or shutdown on the UI thread, and
@@ -245,18 +239,32 @@ void DownloadFileManager::CancelDownload(int id) {
   VLOG(20) << __FUNCTION__ << "()" << " id = " << id;
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DownloadFileMap::iterator it = downloads_.find(id);
-  if (it != downloads_.end()) {
-    DownloadFile* download = it->second;
-    VLOG(20) << __FUNCTION__ << "()"
-             << " download = " << download->DebugString();
-    download->Cancel();
-
-    if (ContainsKey(downloads_with_final_name_, id))
-      EraseDownload(id);
-  }
+  if (it == downloads_.end())
+    return;
 
-  if (downloads_.empty())
-    StopUpdateTimer();
+  DownloadFile* download = it->second;
+  VLOG(20) << __FUNCTION__ << "()"
+           << " download = " << download->DebugString();
+  download->Cancel();
+
+  EraseDownload(id);
+}
+
+void DownloadFileManager::CompleteDownload(int id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  if (!ContainsKey(downloads_, id))
+    return;
+
+  DownloadFile* download_file = downloads_[id];
+
+  VLOG(20) << " " << __FUNCTION__ << "()"
+           << " id = " << id
+           << " download_file = " << download_file->DebugString();
+
+  download_file->Detach();
+
+  EraseDownload(id);
 }
 
 void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
@@ -271,7 +279,6 @@ void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
     if (download_file->GetDownloadManager() == manager) {
       download_file->CancelDownloadRequest(resource_dispatcher_host_);
       to_remove.insert(download_file);
-      downloads_with_final_name_.erase(download_file->id());
     }
   }
 
@@ -286,18 +293,22 @@ void DownloadFileManager::OnDownloadManagerShutdown(DownloadManager* manager) {
 
 // The DownloadManager in the UI thread has provided an intermediate .crdownload
 // name for the download specified by 'id'. Rename the in progress download.
-void DownloadFileManager::OnIntermediateDownloadName(
-    int id, const FilePath& full_path, DownloadManager* download_manager) {
+//
+// There are 2 possible rename cases where this method can be called:
+// 1. tmp -> foo.crdownload (not final, safe)
+// 2. tmp-> Unconfirmed.xxx.crdownload (not final, dangerous)
+void DownloadFileManager::RenameInProgressDownloadFile(
+    int id, const FilePath& full_path) {
   VLOG(20) << __FUNCTION__ << "()" << " id = " << id
            << " full_path = \"" << full_path.value() << "\"";
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  DownloadFileMap::iterator it = downloads_.find(id);
-  if (it == downloads_.end())
+
+  DownloadFile* download = GetDownloadFile(id);
+  if (!download)
     return;
-  DCHECK(!ContainsKey(downloads_with_final_name_, id));
 
-  DownloadFile* download = it->second;
   VLOG(20) << __FUNCTION__ << "()" << " download = " << download->DebugString();
+
   if (!download->Rename(full_path)) {
     // Error. Between the time the UI thread generated 'full_path' to the time
     // this code runs, something happened that prevents us from renaming.
@@ -306,16 +317,14 @@ void DownloadFileManager::OnIntermediateDownloadName(
 }
 
 // The DownloadManager in the UI thread has provided a final name for the
-// download specified by 'id'. Rename the in progress download, and remove it
-// from our table if it has been completed or cancelled already.
+// download specified by 'id'. Rename the completed download.
 //
 // There are 2 possible rename cases where this method can be called:
-// 1. foo.crdownload -> foo
-// 2. tmp-> Unconfirmed.xxx.crdownload
-// We don't call this function before a safe temp file has been renamed (in
-// that case tmp -> foo.crdownload occurs in |OnIntermediateDownloadName|).
-void DownloadFileManager::OnFinalDownloadName(
-    int id, const FilePath& full_path, DownloadManager* download_manager) {
+// 1. foo.crdownload -> foo (final, safe)
+// 2. Unconfirmed.xxx.crdownload -> xxx (final, validated)
+void DownloadFileManager::RenameFinishedDownloadFile(
+    int id, const FilePath& full_path)
+{
   VLOG(20) << __FUNCTION__ << "()" << " id = " << id
            << " full_path = \"" << full_path.value() << "\"";
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
@@ -323,36 +332,48 @@ void DownloadFileManager::OnFinalDownloadName(
   DownloadFile* download = GetDownloadFile(id);
   if (!download)
     return;
+
+  DCHECK(download->GetDownloadManager());
+  DownloadManager* download_manager = download->GetDownloadManager();
+
   VLOG(20) << __FUNCTION__ << "()" << " download = " << download->DebugString();
-  DCHECK(!ContainsKey(downloads_with_final_name_, id));
-  if (download->Rename(full_path)) {
-    downloads_with_final_name_[id] = download;
-#if defined(OS_MACOSX)
-    // Done here because we only want to do this once; see
-    // http://crbug.com/13120 for details.
-    download->AnnotateWithSourceInformation();
-#endif
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(
-            download_manager, &DownloadManager::DownloadRenamedToFinalName, id,
-            full_path));
-  } else {
+
+  int uniquifier = 0;
+  FilePath new_path = full_path;
+  // Make our name unique at this point, as if a dangerous file is
+  // downloading and a 2nd download is started for a file with the same
+  // name, they would have the same path.  This is because we uniquify
+  // the name on download start, and at that time the first file does
+  // not exists yet, so the second file gets the same name.
+  // This should not happen in the SAFE case, and we check for that in the UI
+  // thread.
+  uniquifier = download_util::GetUniquePathNumber(new_path);
+  if (uniquifier > 0) {
+    download_util::AppendNumberToPath(&new_path, uniquifier);
+  }
+
+  // Rename the file, overwriting if necessary.
+  if (!download->Rename(new_path)) {
     // Error. Between the time the UI thread generated 'full_path' to the time
     // this code runs, something happened that prevents us from renaming.
     CancelDownloadOnRename(id);
+    return;
   }
 
-  // If the download has completed before we got this final name, we remove it
-  // from our in progress map.
-  if (!download->in_progress())
-    EraseDownload(id);
+#if defined(OS_MACOSX)
+  // Done here because we only want to do this once; see
+  // http://crbug.com/13120 for details.
+  download->AnnotateWithSourceInformation();
+#endif
 
-  if (downloads_.empty())
-    StopUpdateTimer();
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      NewRunnableMethod(
+          download_manager, &DownloadManager::OnDownloadRenamedToFinalName, id,
+          new_path, uniquifier));
 }
 
-// Called only from OnFinalDownloadName or OnIntermediateDownloadName
+// Called only from RenameInProgressDownloadFile and RenameFinishedDownloadFile
 // on the FILE thread.
 void DownloadFileManager::CancelDownloadOnRename(int id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
@@ -363,6 +384,9 @@ void DownloadFileManager::CancelDownloadOnRename(int id) {
 
   DownloadManager* download_manager = download->GetDownloadManager();
   if (!download_manager) {
+    // Without a download manager, we can't cancel the request normally, so we
+    // need to do it here.  The normal path will also update the download
+    // history before cancelling the request.
     download->CancelDownloadRequest(resource_dispatcher_host_);
     return;
   }
@@ -374,15 +398,21 @@ void DownloadFileManager::CancelDownloadOnRename(int id) {
 }
 
 void DownloadFileManager::EraseDownload(int id) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
   if (!ContainsKey(downloads_, id))
     return;
 
   DownloadFile* download_file = downloads_[id];
 
-  downloads_.erase(id);
+  VLOG(20) << " " << __FUNCTION__ << "()"
+           << " id = " << id
+           << " download_file = " << download_file->DebugString();
 
-  if (ContainsKey(downloads_with_final_name_, id))
-    downloads_with_final_name_.erase(id);
+  downloads_.erase(id);
 
   delete download_file;
+
+  if (downloads_.empty())
+    StopUpdateTimer();
 }