Download filename determination refactor (3/3)

content/browser/download/download_manager_impl.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 "content/browser/download/download_manager_impl.h"
 
 #include <iterator>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 203 matching lines @@
   if (delegate_)
    id = delegate_->GetNextId();
   if (!id.IsValid()) {
     static int next_id;
     id = DownloadId(browser_context_, ++next_id);
   }
 
   return id;
 }
 
+DownloadFileManager* DownloadManagerImpl::GetDownloadFileManager() {
+  return file_manager_;
+}
+
 bool DownloadManagerImpl::ShouldOpenDownload(DownloadItem* item) {
   if (!delegate_)
     return true;
 
   return delegate_->ShouldOpenDownload(item);
 }
 
 bool DownloadManagerImpl::ShouldOpenFileBasedOnExtension(const FilePath& path) {
   if (!delegate_)
     return false;
@@ skipping 176 matching lines @@
     OnDownloadInterrupted(download_id, 0, "", reason);
     // TODO(rdsmith): It makes no sense to continue along the
     // regular download path after we've gotten an error.  But it's
     // the way the code has historically worked, and this allows us
     // to get the download persisted and observers of the download manager
     // notified, so tests work.  When we execute all side effects of cancel
     // (including queue removal) immedately rather than waiting for
     // persistence we should replace this comment with a "return;".
   }
 
-  if (!delegate_ || delegate_->ShouldStartDownload(download_id))
-    RestartDownload(download_id);
+  DownloadItem* download = GetActiveDownloadItem(download_id);
+  if (!download)
+    return;
+
+  content::DownloadTargetCallback callback =
+      base::Bind(&DownloadManagerImpl::OnDownloadTargetDetermined,
+                 this, download_id);
+  if (!delegate_ || !delegate_->DetermineDownloadTarget(download, callback)) {
+    FilePath target_path = download->GetForcedFilePath();
+    // TODO(asanka): Determine a useful path if |target_path| is empty.
+    callback.Run(target_path,
+                 DownloadItem::TARGET_DISPOSITION_OVERWRITE,
+                 content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS,
+                 target_path);
+  }
+  // Once DownloadItem::OnDownloadTargetDetermined() is called, we expect a
+  // DownloadRenamedToIntermediateName() callback. This is necessary for the
+  // download to proceed.

[Randy Smith (Not in Mondays), 2012/07/10 18:33:09]

nit: Wouldn't the best place for this comment be above the direct call to
DI::OnDownloadTargetDetermined() below?

[asanka, 2012/07/11 20:03:32]

Done.

+}
+
+void DownloadManagerImpl::OnDownloadTargetDetermined(
+    int32 download_id,
+    const FilePath& target_path,
+    DownloadItem::TargetDisposition disposition,
+    content::DownloadDangerType danger_type,
+    const FilePath& intermediate_path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DownloadItem* download = GetActiveDownloadItem(download_id);
+  if (!download)
+    return;
+
+  download->OnDownloadTargetDetermined(target_path, disposition, danger_type,
+                                       intermediate_path);
 }
 
 void DownloadManagerImpl::CheckForHistoryFilesRemoval() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   for (DownloadMap::iterator it = downloads_.begin();
        it != downloads_.end(); ++it) {
     DownloadItem* item = it->second;
     if (item->IsPersisted())
       CheckForFileRemoval(item);
   }
@@ skipping 23 matching lines @@
   }
 }
 
 void DownloadManagerImpl::OnFileRemovalDetected(int32 download_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadItem* download = GetDownload(download_id);
   if (download)
     download->OnDownloadedFileRemoved();
 }
 
-void DownloadManagerImpl::RestartDownload(int32 download_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
-    return;
-
-  VLOG(20) << __FUNCTION__ << "()"
-           << " download = " << download->DebugString(true);
-
-  if (download->GetTargetDisposition() ==
-      DownloadItem::TARGET_DISPOSITION_PROMPT) {
-    // We must ask the user for the place to put the download.
-    if (delegate_) {
-      delegate_->ChooseDownloadPath(download);
-      FOR_EACH_OBSERVER(Observer, observers_,
-                        SelectFileDialogDisplayed(this, download_id));
-    } else {
-      FileSelectionCanceled(download_id);
-    }
-  } else {
-    // No prompting for download, just continue with the current target path.
-    OnTargetPathAvailable(download);
-  }
-}
-
 content::BrowserContext* DownloadManagerImpl::GetBrowserContext() const {
   return browser_context_;
 }
 
-FilePath DownloadManagerImpl::LastDownloadPath() {
-  return last_download_path_;
-}
-
 net::BoundNetLog DownloadManagerImpl::CreateDownloadItem(
     DownloadCreateInfo* info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   net::BoundNetLog bound_net_log =
       net::BoundNetLog::Make(net_log_, net::NetLog::SOURCE_DOWNLOAD);
   if (!info->download_id.IsValid())
     info->download_id = GetNextId();
   DownloadItem* download = factory_->CreateActiveItem(
       this, *info,
@@ skipping 33 matching lines @@
   DCHECK(!ContainsKey(save_page_downloads_, download->GetId()));
   save_page_downloads_[download->GetId()] = download;
 
   // Will notify the observer in the callback.
   if (delegate_)
     delegate_->AddItemToPersistentStore(download);
 
   return download;
 }
 
-// The target path for the download item is now valid. We proceed with the
-// determination of an intermediate path.
-void DownloadManagerImpl::OnTargetPathAvailable(DownloadItem* download) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(download);
-  DCHECK(ContainsKey(downloads_, download->GetId()));
-  DCHECK(ContainsKey(active_downloads_, download->GetId()));
-
-  VLOG(20) << __FUNCTION__ << "()"
-           << " download = " << download->DebugString(true);
-
-  // Rename to intermediate name.
-  // TODO(asanka): Skip this rename if download->AllDataSaved() is true. This
-  //               avoids a spurious rename when we can just rename to the final
-  //               filename. Unnecessary renames may cause bugs like
-  //               http://crbug.com/74187.
-  FilePath intermediate_path;
-  if (delegate_)
-    intermediate_path = delegate_->GetIntermediatePath(*download);
-  else
-    intermediate_path = download->GetTargetFilePath();
-
-  // We want the intermediate and target paths to refer to the same directory so
-  // that they are both on the same device and subject to same
-  // space/permission/availability constraints.
-  DCHECK(intermediate_path.DirName() ==
-         download->GetTargetFilePath().DirName());
-  download->OnIntermediatePathDetermined(file_manager_, intermediate_path);
-}
-
 void DownloadManagerImpl::UpdateDownload(int32 download_id,
                                          int64 bytes_so_far,
                                          int64 bytes_per_sec,
                                          const std::string& hash_state) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DownloadMap::iterator it = active_downloads_.find(download_id);
   if (it != active_downloads_.end()) {
     DownloadItem* download = it->second;
     if (download->IsInProgress()) {
       download->UpdateProgress(bytes_so_far, bytes_per_sec, hash_state);
@@ skipping 292 matching lines @@
   // TODO: It is the responsibility of the observers to query the
   // DownloadManager. Remove the following call from here and update all
   // observers.
   observer->ModelChanged(this);
 }
 
 void DownloadManagerImpl::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-void DownloadManagerImpl::FileSelected(const FilePath& path,
-                                       int32 download_id) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(!path.empty());
-
-  DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
-    return;
-  VLOG(20) << __FUNCTION__ << "()" << " path = \"" << path.value() << "\""
-           << " download = " << download->DebugString(true);
-
-  // Retain the last directory. Exclude temporary downloads since the path
-  // likely points at the location of a temporary file.
-  if (!download->IsTemporary())
-    last_download_path_ = path.DirName();
-
-  // Make sure the initial file name is set only once.
-  download->OnTargetPathSelected(path);
-  OnTargetPathAvailable(download);
-}
-
-void DownloadManagerImpl::FileSelectionCanceled(int32 download_id) {
-  // The user didn't pick a place to save the file, so need to cancel the
-  // download that's already in progress to the temporary location.
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  DownloadItem* download = GetActiveDownloadItem(download_id);
-  if (!download)
-    return;
-
-  VLOG(20) << __FUNCTION__ << "()"
-           << " download = " << download->DebugString(true);
-
-  download->Cancel(true);
-}
-
 // Operations posted to us from the history service ----------------------------
 
 // The history service has retrieved all download entries. 'entries' contains
 // 'DownloadPersistentStoreInfo's in sorted order (by ascending start_time).
 void DownloadManagerImpl::OnPersistentStoreQueryComplete(
     std::vector<DownloadPersistentStoreInfo>* entries) {
   history_size_ = entries->size();
   for (size_t i = 0; i < entries->size(); ++i) {
     int64 db_handle = entries->at(i).db_handle;
     base::debug::Alias(&db_handle);
@@ skipping 101 matching lines @@
   for (DownloadMap::const_iterator it = active_downloads_.begin();
        it != active_downloads_.end(); ++it) {
     DownloadItem* item = it->second;
     if (item->IsInProgress())
       ++count;
   }
   return count;
 }
 
 // Clears the last download path, used to initialize "save as" dialogs.
-void DownloadManagerImpl::ClearLastDownloadPath() {
-  last_download_path_ = FilePath();
+void DownloadManagerImpl::ClearTransientState() {
+  if (delegate_)
+    delegate_->ClearTransientState();
 }
 
 void DownloadManagerImpl::NotifyModelChanged() {
   FOR_EACH_OBSERVER(Observer, observers_, ModelChanged(this));
 }
 
 DownloadItem* DownloadManagerImpl::GetDownloadItem(int download_id) {
   DownloadItem* download = GetDownload(download_id);
   return (download && download->IsPersisted()) ? download : NULL;
 }
@@ skipping 105 matching lines @@
     if (item->IsComplete() &&
         !item->GetOpened())
       ++num_unopened;
   }
   download_stats::RecordOpensOutstanding(num_unopened);
 }
 
 void DownloadManagerImpl::DownloadRenamedToIntermediateName(
     DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  // If the rename failed, we receive an OnDownloadInterrupted() call before we
-  // receive the DownloadRenamedToIntermediateName() call.
-  if (delegate_)
+  // download->GetFullPath() is only expected to be meaningful after this
+  // callback is received. Therefore we can now add the download to a persistent
+  // store. If the rename failed, we receive an OnDownloadInterrupted() call
+  // before we receive the DownloadRenamedToIntermediateName() call.
+  if (delegate_) {
     delegate_->AddItemToPersistentStore(download);
+  } else {
+    OnItemAddedToPersistentStore(download->GetId(),
+                                 DownloadItem::kUninitializedHandle);
+  }
 }
 
 void DownloadManagerImpl::DownloadRenamedToFinalName(
     DownloadItem* download) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   // If the rename failed, we receive an OnDownloadInterrupted() call before we
   // receive the DownloadRenamedToFinalName() call.
   if (delegate_) {
     delegate_->UpdatePathForItemInPersistentStore(
         download, download->GetFullPath());
   }
 }