Implement chrome.downloads.onDeterminingFilename()

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"
 #include "base/callback.h"
 #include "base/file_util.h"
 #include "base/prefs/public/pref_member.h"
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
 #include "base/time.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/download/download_completion_blocker.h"
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_extensions.h"
 #include "chrome/browser/download/download_file_picker.h"
 #include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_path_reservation_tracker.h"
 #include "chrome/browser/download/download_prefs.h"
+#include "chrome/browser/download/download_service.h"
+#include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/download/download_status_updater.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/download/save_package_file_picker.h"
 #include "chrome/browser/extensions/api/downloads/downloads_api.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/history/history.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/intents/web_intents_util.h"
@@ skipping 129 matching lines @@
     : profile_(profile),
       next_download_id_(0),
       download_prefs_(new DownloadPrefs(profile)) {
 }
 
 ChromeDownloadManagerDelegate::~ChromeDownloadManagerDelegate() {
 }
 
 void ChromeDownloadManagerDelegate::SetDownloadManager(DownloadManager* dm) {
   download_manager_ = dm;
-#if !defined(OS_ANDROID)
-  extension_event_router_.reset(new ExtensionDownloadsEventRouter(
-      profile_, download_manager_));
-#endif
 }
 
 void ChromeDownloadManagerDelegate::Shutdown() {
   download_prefs_.reset();
-#if !defined(OS_ANDROID)
-  extension_event_router_.reset();
-#endif
 }
 
 DownloadId ChromeDownloadManagerDelegate::GetNextId() {
   if (!profile_->IsOffTheRecord())
     return DownloadId(this, next_download_id_++);
 
   return BrowserContext::GetDownloadManager(profile_->GetOriginalProfile())->
       GetDelegate()->GetNextId();
 }
 
@@ skipping 465 matching lines @@
                     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());
 }
 
+ChromeDownloadManagerDelegate::ContinueFilenameDeterminationInfo::
+    ContinueFilenameDeterminationInfo() {}
+ChromeDownloadManagerDelegate::ContinueFilenameDeterminationInfo::
+    ~ContinueFilenameDeterminationInfo() {}
+
 void ChromeDownloadManagerDelegate::CheckVisitedReferrerBeforeDone(
     int32 download_id,
     const content::DownloadTargetCallback& callback,
     content::DownloadDangerType danger_type,
     bool visited_referrer_before) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   DownloadItem* download =
       download_manager_->GetDownload(download_id);
   if (!download || (download->GetState() != DownloadItem::IN_PROGRESS))
     return;
 
   bool should_prompt = (download->GetTargetDisposition() ==
                         DownloadItem::TARGET_DISPOSITION_PROMPT);
   bool is_forced_path = !download->GetForcedFilePath().empty();
+  FilePath generated_name;
   FilePath suggested_path;
 
   // Check whether this download is for an extension install or not.
   // Allow extensions to be explicitly saved.
   if (!is_forced_path) {
-    FilePath generated_name;
     GenerateFileNameFromRequest(
         *download,
         &generated_name,
         profile_->GetPrefs()->GetString(prefs::kDefaultCharset));
 
     // Freeze the user's preference for showing a Save As dialog.  We're going
     // to bounce around a bunch of threads and we don't want to worry about race
     // conditions where the user changes this pref out from under us.
     if (download_prefs_->PromptForDownload()) {
       // But ignore the user's preference for the following scenarios:
@@ skipping 10 matching lines @@
       should_prompt = false;
 
     // Determine the proper path for a download, by either one of the following:
     // 1) using the default download directory.
     // 2) prompting the user.
     FilePath target_directory;
     if (should_prompt && !last_download_path_.empty())
       target_directory = last_download_path_;
     else
       target_directory = download_prefs_->DownloadPath();
+    // If an extension overrides the filename, then the target directory will be
+    // forced to download_prefs_->DownloadPath() since extensions cannot place
+    // downloaded files anywhere except there. This prevents subdirectories from
+    // accumulating: if an extension is allowed to say that a file should go in
+    // last_download_path/music/foo.mp3, then last_download_path will accumulate
+    // the subdirectory /music/ so that the next download may end up in
+    // last_download_path/music/music/bar.mp3.

[asanka, 2013/01/11 21:56:54]

I understand why this comment is here, but I think it's better if it was in
OnExtensionsChangingFilename(). We are potentially overriding the path and
filename based on the extension anyway.

[benjhayden, 2013/01/11 22:04:52]

Done.

     suggested_path = target_directory.Append(generated_name);
   } else {
     DCHECK(!should_prompt);
     suggested_path = download->GetForcedFilePath();
   }
 
   // If we will open the file with a web intents dispatch,
   // give it a name that will not allow the OS to open it using usual
   // associated apps.
   if (ShouldOpenWithWebIntents(download)) {
     download->SetDisplayName(suggested_path.BaseName());
     suggested_path = suggested_path.AddExtension(kWebIntentsFileExtension);
   }
 
+  ContinueFilenameDeterminationInfo continue_info;
+  continue_info.download_id = download_id;
+  continue_info.callback = callback;
+  continue_info.danger_type = danger_type;
+  continue_info.visited_referrer_before = visited_referrer_before;
+  continue_info.should_prompt = should_prompt;
+
+  base::Closure filename_determined = base::Bind(
+      &ChromeDownloadManagerDelegate::OnExtensionsDeterminedFilename,
+      this,
+      continue_info,
+      suggested_path,
+      is_forced_path);
+#if !defined(OS_ANDROID)
+  if (is_forced_path ||
+      ShouldOpenWithWebIntents(download) ||
+      !DownloadServiceFactory::GetForProfile(profile_)
+        ->GetExtensionEventRouter()) {
+    filename_determined.Run();
+  } else {
+    DownloadServiceFactory::GetForProfile(profile_)->GetExtensionEventRouter()
+      ->OnDownloadFilenameDetermined(
+        download,
+        generated_name,
+        filename_determined,
+        base::Bind(&ChromeDownloadManagerDelegate::OnExtensionsChangingFilename,
+                   this, continue_info));
+  }
+#else
+  filename_determined.Run();

[asanka, 2013/01/11 21:56:54]

Why not just call OnExtensionsDeterminedFilename()? Looks like the callback is
only needed if going through the ExtensionDownloadsEventRouter.

[benjhayden, 2013/01/11 22:04:52]

If OnExtensionsDeterminedFilename's signature changes, we only need to change
the base::Bind, not 3 different places.

+#endif
+}
+
+void ChromeDownloadManagerDelegate::OnExtensionsChangingFilename(
+    const ContinueFilenameDeterminationInfo& continue_info,
+    const FilePath& changed_filename,
+    bool overwrite) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  CHECK(!changed_filename.empty());
+  CHECK(!changed_filename.IsAbsolute());
+  DownloadItem* download =
+      download_manager_->GetDownload(continue_info.download_id);
+  if (!download || (download->GetState() != DownloadItem::IN_PROGRESS))

[asanka, 2013/01/11 21:56:54]

We need to invoke the DownloadTargetCallback regardless of whether the download
is interrupted or not.

[benjhayden, 2013/01/11 22:04:52]

Why?
That's not what SubstituteDriveDownloadPathCallback, OnPathReservationAvailable,
and OnTargetPathDetermined do. Should I fix them?

+    return;
+  FilePath temp_filename(download_prefs_->DownloadPath().Append(
+      changed_filename));
+  net::GenerateSafeFileName(download->GetMimeType(), false, &temp_filename);
+  // If |is_forced_path| were true, then extensions would not have been
+  // consulted, so use |overwrite| instead of |is_forced_path|. This does NOT
+  // set DownloadItem::GetForcedFilePath()!
+  OnExtensionsDeterminedFilename(continue_info, temp_filename, overwrite);
+}
+
+void ChromeDownloadManagerDelegate::OnExtensionsDeterminedFilename(
+    const ContinueFilenameDeterminationInfo& continue_info,
+    const FilePath& suggested_path,
+    bool is_forced_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  int32 download_id = continue_info.download_id;
+  const content::DownloadTargetCallback& callback = continue_info.callback;
+  content::DownloadDangerType danger_type = continue_info.danger_type;
+  bool visited_referrer_before = continue_info.visited_referrer_before;
+  bool should_prompt = continue_info.should_prompt;
+  DownloadItem* download =
+      download_manager_->GetDownload(download_id);
+  if (!download || (download->GetState() != DownloadItem::IN_PROGRESS))

[asanka, 2013/01/11 21:56:54]

As above.

[benjhayden, 2013/01/11 22:04:52]

ditto.

+    return;
+
   // If the download hasn't already been marked dangerous (could be
   // DANGEROUS_URL), check if it is a dangerous file.
   if (danger_type == content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS) {
     if (!should_prompt && !is_forced_path &&
         IsDangerousFile(*download, suggested_path, visited_referrer_before)) {
       danger_type = content::DOWNLOAD_DANGER_TYPE_DANGEROUS_FILE;
     }
 
 #if defined(FULL_SAFE_BROWSING)
     DownloadProtectionService* service = GetDownloadProtectionService();
@@ skipping 107 matching lines @@
     // TODO(asanka): This logic is a hack. DownloadFilePicker should give us a
     //               directory to persist. Or perhaps, if the Drive path
     //               substitution logic is moved here, then we would have a
     //               persistable path after the DownloadFilePicker is done.
     if (disposition == DownloadItem::TARGET_DISPOSITION_PROMPT &&
         !download->IsTemporary())
       last_download_path_ = target_path.DirName();
   }
   callback.Run(target_path, disposition, danger_type, intermediate_path);
 }