Rename the download to its final name only after the download is finished

chrome/browser/download/download_manager.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/download/download_manager.h"
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/callback.h"
 #include "base/file_util.h"
@@ skipping 459 matching lines @@
   // Check writability of the suggested path. If we can't write to it, default
   // to the user's "My Documents" directory. We'll prompt them in this case.
   FilePath dir = info->suggested_path.DirName();
   FilePath filename = info->suggested_path.BaseName();
   if (!file_util::PathIsWritable(dir)) {
     info->prompt_user_for_save_location = true;
     PathService::Get(chrome::DIR_USER_DOCUMENTS, &info->suggested_path);
     info->suggested_path = info->suggested_path.Append(filename);
   }
 
-  // Do not add the path uniquifier if we are saving to a specific path as in
-  // the drag-out case.
-  if (info->save_info.file_path.empty()) {
-    info->path_uniquifier = download_util::GetUniquePathNumber(
-        info->suggested_path);
-  }
-
   // If the download is deemed dangerous, we'll use a temporary name for it.
   if (info->is_dangerous) {
     info->original_name = FilePath(info->suggested_path).BaseName();
     // Create a temporary file to hold the file until the user approves its
     // download.
     FilePath::StringType file_name;
     FilePath path;
     while (path.empty()) {
-      SStringPrintf(&file_name, FILE_PATH_LITERAL("unconfirmed %d.download"),
+      SStringPrintf(&file_name, FILE_PATH_LITERAL("unconfirmed %d.crdownload"),
                     base::RandInt(0, 100000));
       path = dir.Append(file_name);
       if (file_util::PathExists(path))
         path = FilePath();
     }
     info->suggested_path = path;
   } else {
+    // Do not add the path uniquifier if we are saving to a specific path as in
+    // the drag-out case.
+    if (info->save_info.file_path.empty()) {
+      info->path_uniquifier = download_util::GetUniquePathNumberWithCrDownload(
+          info->suggested_path);
+    }
     // We know the final path, build it if necessary.
     if (info->path_uniquifier > 0) {
       download_util::AppendNumberToPath(&(info->suggested_path),
                                         info->path_uniquifier);
       // Setting path_uniquifier to 0 to make sure we don't try to unique it
       // later on.
       info->path_uniquifier = 0;
     } else if (info->path_uniquifier == -1) {
       // We failed to find a unique path.  We have to prompt the user.
       info->prompt_user_for_save_location = true;
     }
   }
 
+  // Create an empty file at the suggested path so that we don't allocate the
+  // same "non-existant" path to multiple downloads.
+  // See: http://code.google.com/p/chromium/issues/detail?id=3662
   if (!info->prompt_user_for_save_location &&
       info->save_info.file_path.empty()) {
-    // Create an empty file at the suggested path so that we don't allocate the
-    // same "non-existant" path to multiple downloads.
-    // See: http://code.google.com/p/chromium/issues/detail?id=3662
-    file_util::WriteFile(info->suggested_path, "", 0);
+    if (info->is_dangerous)
+      file_util::WriteFile(info->suggested_path, "", 0);
+    else
+      file_util::WriteFile(download_util::GetCrDownloadPath(
+          info->suggested_path), "", 0);
   }
 
   // Now we return to the UI thread.
   ChromeThread::PostTask(
       ChromeThread::UI, FROM_HERE,
       NewRunnableMethod(this,
                         &DownloadManager::OnPathExistenceAvailable,
                         info));
 }
 
@@ skipping 52 matching lines @@
                                 profile_->IsOffTheRecord(),
                                 info->is_extension_install,
                                 !info->save_info.file_path.empty());
     download->set_manager(this);
     in_progress_[info->download_id] = download;
   } else {
     NOTREACHED();  // Should not exist!
     return;
   }
 
-  // Called before DownloadFinished in order to avoid a race condition where we
-  // attempt to open a completed download before it has been renamed.
-  ChromeThread::PostTask(
-      ChromeThread::FILE, FROM_HERE,
-      NewRunnableMethod(
-          file_manager_, &DownloadFileManager::OnFinalDownloadName,
-          download->id(), target_path, this));
-
-  // If the download already completed by the time we reached this point, then
-  // notify observers that it did.
   PendingFinishedMap::iterator pending_it =
       pending_finished_downloads_.find(info->download_id);
-  if (pending_it != pending_finished_downloads_.end())
+  bool download_finished = (pending_it != pending_finished_downloads_.end());
+
+  if (download_finished || info->is_dangerous) {
+    // The download has already finished or the download is not safe.
+    // We can now rename the file to its final name (or its tentative name
+    // in dangerous download cases).
+    ChromeThread::PostTask(
+        ChromeThread::FILE, FROM_HERE,
+        NewRunnableMethod(
+            file_manager_, &DownloadFileManager::OnFinalDownloadName,
+            download->id(), target_path, !info->is_dangerous, this));
+  } else {
+    // The download hasn't finished and it is a safe download.  We need to
+    // rename it to its intermediate '.crdownload' path.
+    FilePath download_path = download_util::GetCrDownloadPath(target_path);
+    ChromeThread::PostTask(
+        ChromeThread::FILE, FROM_HERE,
+        NewRunnableMethod(
+            file_manager_, &DownloadFileManager::OnIntermediateDownloadName,
+            download->id(), download_path, this));
+    download->set_need_final_rename(true);
+  }
+
+  if (download_finished) {
+    // If the download already completed by the time we reached this point, then
+    // notify observers that it did.
     DownloadFinished(pending_it->first, pending_it->second);
+  }
 
   download->Rename(target_path);
 
   // Do not store the download in the history database for a few special cases:
   // - incognito mode (that is the point of this mode)
   // - extensions (users don't think of extension installation as 'downloading')
   // - temporary download, like in drag-and-drop
   // We have to make sure that these handles don't collide with normal db
   // handles, so we use a negative value. Eventually, they could overlap, but
   // you'd have to do enough downloading that your ISP would likely stab you in
@@ skipping 104 matching lines @@
     // We first need to rename the downloaded file from its temporary name to
     // its final name before we can continue.
     ChromeThread::PostTask(
       ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(
             this, &DownloadManager::ProceedWithFinishedDangerousDownload,
             download->db_handle(),
             download->full_path(), download->original_name()));
     return;
   }
+
+  if (download->need_final_rename()) {
+    ChromeThread::PostTask(
+        ChromeThread::FILE, FROM_HERE,
+        NewRunnableMethod(
+            file_manager_, &DownloadFileManager::OnFinalDownloadName,
+            download->id(), download->full_path(), false, this));
+    return;
+  }
+
   ContinueDownloadFinished(download);
 }
 
 void DownloadManager::DownloadRenamedToFinalName(int download_id,
                                                  const FilePath& full_path) {
   DownloadMap::iterator it = downloads_.begin();
   while (it != downloads_.end()) {
     DownloadItem* download = it->second;
     if (download->id() == download_id) {
       // The download file is meant to be completed if both the filename is
       // finalized and the file data is downloaded. The ordering of these two
       // actions is indeterministic. Thus, if we are still in downloading the
       // file, delay the notification.
       download->set_name_finalized(true);
       if (download->state() == DownloadItem::COMPLETE)
         download->NotifyObserversDownloadFileCompleted();
+
+      // This was called from DownloadFinished; continue to call
+      // ContinueDownloadFinished.
+      if (download->need_final_rename()) {
+        download->set_need_final_rename(false);
+        ContinueDownloadFinished(download);
+      }
       return;
     }
     it++;
   }
 }
 
 void DownloadManager::ContinueDownloadFinished(DownloadItem* download) {
   // If this was a dangerous download, it has now been approved and must be
   // removed from dangerous_finished_ so it does not get deleted on shutdown.
   DownloadMap::iterator it = dangerous_finished_.find(download->id());
@@ skipping 840 matching lines @@
   observing_download_manager_->NotifyModelChanged();
 }
 
 void DownloadManager::OtherDownloadManagerObserver::SetDownloads(
     std::vector<DownloadItem*>& downloads) {
 }
 
 void DownloadManager::OtherDownloadManagerObserver::ManagerGoingDown() {
   observed_download_manager_ = NULL;
 }