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

chrome/browser/download/chrome_download_manager_delegate.cc

 // Copyright (c) 2012 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/chrome_download_manager_delegate.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 68 matching lines @@
   }
 
  private:
   DownloadProtectionService::DownloadCheckResult verdict_;
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingState);
 };
 
 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 which platform path to
+// return.
+enum PlatformDownloadPathType {
+  // Return the platform specific target path.
+  PLATFORM_TARGET_PATH,
+
+  // Return the platform specific current path. If the download is in-progress
+  // and the download location is a local filesystem path, then
+  // GetPlatformDownloadPath will return the path to the intermediate file.
+  PLATFORM_CURRENT_PATH
+};
+
+// 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.
+//
+// How the platform path is determined is based on PlatformDownloadPathType.
 base::FilePath GetPlatformDownloadPath(Profile* profile,
-                                       const DownloadItem* download) {
+                                       const DownloadItem* download,
+                                       PlatformDownloadPathType path_type) {
 #if defined(OS_CHROMEOS)
+  // Drive downloads always return the target path for all types.

[Randy Smith (Not in Mondays), 2013/05/31 21:18:19]

To me, this comment is a code smell here: the consumer asks for a specific type
of path, and we silently override based on a very specific feature of the
download.  It suggests to me that the override determination should be made at a
higher level, probably based on whether we've virtualized the path.  I'm
disinclined towards changing it in this CL, but you might want to keep in the
back of your head the question of whether there's a better way to do it.

[asanka, 2013/05/31 21:58:40]

Yup. Thanks.

This CL doesn't change 'show in folder' behavior for Drive downloads, but you're
right that the API behavior is non-ideal.

   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());
+
+  if (path_type == PLATFORM_TARGET_PATH)
     return download->GetTargetFilePath();
-  }
-
-  DCHECK(!download->GetFullPath().empty());
   return download->GetFullPath();
 }
 
 // Callback invoked by DownloadProtectionService::CheckClientDownload.
 // |is_content_check_supported| is true if the SB service supports scanning the
 // download for malicious content.
 // |callback| is invoked with a danger type determined as follows:
 //
 // Danger type is (in order of preference):
 //   * DANGEROUS_URL, if the URL is a known malware site.
@@ skipping 44 matching lines @@
   if (!profile_->IsOffTheRecord())
     return DownloadId(this, next_download_id_++);
 
   return content::BrowserContext::GetDownloadManager(
       profile_->GetOriginalProfile())->GetDelegate()->GetNextId();
 }
 
 bool ChromeDownloadManagerDelegate::DetermineDownloadTarget(
     DownloadItem* download,
     const content::DownloadTargetCallback& callback) {
-  DownloadTargetDeterminer::Start(download,
-                                  download_prefs_.get(),
-                                  this,
-                                  callback);
+  DownloadTargetDeterminer::Start(
+      download,
+      GetPlatformDownloadPath(
+          profile_, download, PLATFORM_TARGET_PATH),
+      download_prefs_.get(),
+      this,
+      callback);
   return true;
 }
 
 bool ChromeDownloadManagerDelegate::ShouldOpenFileBasedOnExtension(
     const base::FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (path.Extension().empty())
     return false;
   // TODO(asanka): This determination is done based on |path|, while
   // ShouldOpenDownload() detects extension downloads based on the
@@ skipping 128 matching lines @@
       default_extension,
       can_save_as_complete,
       download_prefs_.get(),
       callback);
 }
 
 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, PLATFORM_TARGET_PATH));
+  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, PLATFORM_CURRENT_PATH));
+  DCHECK(!platform_path.empty());
+  platform_util::ShowItemInFolder(platform_path);
 }
 
 void ChromeDownloadManagerDelegate::CheckForFileExistence(
     DownloadItem* download,
     const content::CheckForFileExistenceCallback& callback) {
 #if defined(OS_CHROMEOS)
   drive::DownloadHandler* drive_download_handler =
       drive::DownloadHandler::GetForProfile(profile_);
   if (drive_download_handler &&
       drive_download_handler->IsDriveDownload(download)) {
@@ skipping 160 matching lines @@
   registrar_.Remove(this,
                     chrome::NOTIFICATION_CRX_INSTALLER_DONE,
                     source);
 
   scoped_refptr<extensions::CrxInstaller> installer =
       content::Source<extensions::CrxInstaller>(source).ptr();
   content::DownloadOpenDelayedCallback callback = crx_installers_[installer];
   crx_installers_.erase(installer.get());
   callback.Run(installer->did_handle_successfully());
 }