[Resumption 9/11] Handle filename determination for resumed downloads.

chrome/browser/download/chrome_download_manager_delegate.cc

Index: chrome/browser/download/chrome_download_manager_delegate.cc
diff --git a/chrome/browser/download/chrome_download_manager_delegate.cc b/chrome/browser/download/chrome_download_manager_delegate.cc
index 7feb00636d62350be3b251315a085d6366f681f7..5bb275417d49dca067ec629987e4b5f2aac202e0 100644
--- a/chrome/browser/download/chrome_download_manager_delegate.cc
+++ b/chrome/browser/download/chrome_download_manager_delegate.cc
@@ -86,36 +86,60 @@ class SafeBrowsingState : public DownloadCompletionBlocker {
 
 SafeBrowsingState::~SafeBrowsingState() {}
 
-// Returns a file path in the form that is expected by
-// platform_util::OpenItem/ShowItemInFolder, including any transformation
-// required for download abstractions layered on top of the core system,
-// e.g. download to Drive.
+// Used with GetPlatformDownloadPath() to indicate the intended usage of the
+// returned path.
+enum PlatformPathUsage {
+  PATH_FOR_OPEN,
+  PATH_FOR_SHOW_IN_FOLDER,
+  PATH_FOR_TARGET_DETERMINATION
+};
+
+// Returns a path in the form that that is expected by platform_util::OpenItem /
+// platform_util::ShowItemInFolder / DownloadTargetDeterminer.
+//
+// DownloadItems corresponding to Drive downloads use a temporary file as the
+// target path. The paths returned by DownloadItem::GetFullPath() /
+// GetTargetFilePath() refer to this temporary file. This function looks up the
+// corresponding path in Drive for these downloads.
+//
+// For downloads that target the local filesystem, this function returns either
+// the path to the intermediate file or the target path depending on
+// |path_usage|:
+//   PATH_FOR_OPEN: Always returns the target path.
+//   PATH_FOR_SHOW_IN_FOLDER: Path to the intermediate file if the download is
+//     still in progress. Otherwise returns the target path.
+//   PATH_FOR_TARGET_DETERMINATION: Always returns the target path.

[Randy Smith (Not in Mondays), 2013/05/30 21:26:24]

Is there a reason not to make this an "always_return_target" boolean and move
the DCHECKs up a level?  I'm good with your preference, but how the consumer is
going to use it seems like Too Much Information for this routine.

[asanka, 2013/05/31 19:23:59]

Done.

 base::FilePath GetPlatformDownloadPath(Profile* profile,
-                                       const DownloadItem* download) {
+                                       const DownloadItem* download,
+                                       PlatformPathUsage path_usage) {
 #if defined(OS_CHROMEOS)
+  // Drive downloads always return the target path for all usages.
   drive::DownloadHandler* drive_download_handler =
       drive::DownloadHandler::GetForProfile(profile);
   if (drive_download_handler &&
       drive_download_handler->IsDriveDownload(download))
     return drive_download_handler->GetTargetPath(download);
 #endif
-  // The caller wants to open the download or show it in a file browser. The
-  // download could be in one of three states:
-  // - Complete: The path we want is GetTargetFilePath().
-  // - Not complete, but there's an intermediate file: GetFullPath() will be
-  //     non-empty and is the location of the intermediate file. Since no target
-  //     file exits yet, use GetFullPath(). This should only happen during
-  //     ShowDownloadInShell().
-  // - Not Complete, and there's no intermediate file: GetFullPath() will be
-  //     empty. This shouldn't happen since CanShowInFolder() returns false and
-  //     this function shouldn't have been called.
-  if (download->GetState() == DownloadItem::COMPLETE) {
-    DCHECK(!download->GetTargetFilePath().empty());
-    return download->GetTargetFilePath();
+
+  switch (path_usage) {
+    case PATH_FOR_OPEN:
+      DCHECK_EQ(DownloadItem::COMPLETE, download->GetState());
+      return download->GetTargetFilePath();
+
+    case PATH_FOR_SHOW_IN_FOLDER:
+      DCHECK(download->GetState() == DownloadItem::IN_PROGRESS ||
+             download->GetState() == DownloadItem::COMPLETE);
+      return (download->GetState() == DownloadItem::IN_PROGRESS)
+                 ? download->GetFullPath()
+                 : download->GetTargetFilePath();
+
+    case PATH_FOR_TARGET_DETERMINATION:
+      DCHECK_EQ(DownloadItem::IN_PROGRESS, download->GetState());
+      return download->GetTargetFilePath();
   }
 
-  DCHECK(!download->GetFullPath().empty());
-  return download->GetFullPath();
+  NOTREACHED();
+  return base::FilePath();
 }
 
 // Callback invoked by DownloadProtectionService::CheckClientDownload.
@@ -179,10 +203,13 @@ DownloadId ChromeDownloadManagerDelegate::GetNextId() {
 bool ChromeDownloadManagerDelegate::DetermineDownloadTarget(
     DownloadItem* download,
     const content::DownloadTargetCallback& callback) {
-  DownloadTargetDeterminer::Start(download,
-                                  download_prefs_.get(),
-                                  this,
-                                  callback);
+  DownloadTargetDeterminer::Start(
+      download,
+      GetPlatformDownloadPath(
+          profile_, download, PATH_FOR_TARGET_DETERMINATION),
+      download_prefs_.get(),
+      this,
+      callback);
   return true;
 }
 
@@ -331,14 +358,20 @@ void ChromeDownloadManagerDelegate::OpenDownload(DownloadItem* download) {
   DCHECK_EQ(DownloadItem::COMPLETE, download->GetState());
   if (!download->CanOpenDownload())
     return;
-  platform_util::OpenItem(GetPlatformDownloadPath(profile_, download));
+  base::FilePath platform_path(
+      GetPlatformDownloadPath(profile_, download, PATH_FOR_OPEN));
+  DCHECK(!platform_path.empty());
+  platform_util::OpenItem(platform_path);
 }
 
 void ChromeDownloadManagerDelegate::ShowDownloadInShell(
     DownloadItem* download) {
   if (!download->CanShowInFolder())
     return;
-  platform_util::ShowItemInFolder(GetPlatformDownloadPath(profile_, download));
+  base::FilePath platform_path(
+      GetPlatformDownloadPath(profile_, download, PATH_FOR_SHOW_IN_FOLDER));
+  DCHECK(!platform_path.empty());
+  platform_util::ShowItemInFolder(platform_path);
 }
 
 void ChromeDownloadManagerDelegate::CheckForFileExistence(