Download filename determination refactor (3/3)

content/public/browser/download_manager_delegate.h

Index: content/public/browser/download_manager_delegate.h
diff --git a/content/public/browser/download_manager_delegate.h b/content/public/browser/download_manager_delegate.h
index 0fd224374d64bb7d9c87324e197ae8c04fef70b6..f2532360aa87320b2e4f45591a7012b6d42443ed 100644
--- a/content/public/browser/download_manager_delegate.h
+++ b/content/public/browser/download_manager_delegate.h
@@ -12,12 +12,13 @@
 #include "base/logging.h"
 #include "base/time.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/download_danger_type.h"
+#include "content/public/browser/download_item.h"
 #include "content/public/browser/save_page_type.h"
 
 namespace content {
 
 class DownloadId;
-class DownloadItem;
 class WebContents;
 
 // Called by SavePackage when it creates a DownloadItem.
@@ -33,6 +34,12 @@ typedef base::Callback<void(const FilePath&,
                             const SavePackageDownloadCreatedCallback&)>
     SavePackagePathPickedCallback;
 
+typedef base::Callback<void(
+    const FilePath& target_path,
+    content::DownloadItem::TargetDisposition disposition,
+    content::DownloadDangerType danger_type,
+    const FilePath& intermediate_path)> DownloadTargetCallback;
+
 // Browser's download manager: manages all downloads and destination view.
 class CONTENT_EXPORT DownloadManagerDelegate {
  public:
@@ -42,23 +49,14 @@ class CONTENT_EXPORT DownloadManagerDelegate {
   // Returns a new DownloadId.
   virtual DownloadId GetNextId();
 
-  // Notifies the delegate that a download is starting. The delegate can return
-  // false to delay the start of the download, in which case it should call
-  // DownloadManager::RestartDownload when it's ready.
-  virtual bool ShouldStartDownload(int32 download_id);
-
-  // Asks the user for the path for a download. The delegate calls
-  // DownloadManager::FileSelected or DownloadManager::FileSelectionCanceled to
-  // give the answer.
-  virtual void ChooseDownloadPath(DownloadItem* item) {}
-
-  // Allows the embedder to set an intermediate name for the download until it's
-  // complete. The return value is the intermediate path to use. If the embedder
-  // doesn't want to set an intermediate path, it should return
-  // item.GetTargetFilePath(). If there's already a file at the returned path,
-  // it will not be overwritten. Instead the path will be uniquified by adding a
-  // suffix to the filename.
-  virtual FilePath GetIntermediatePath(const DownloadItem& item);
+  // Called to notify the delegate that a new download |item| requires a
+  // download target to be determined. The delegate should return |true| if it
+  // will determine the target information and will invoke |callback|. The
+  // callback may be invoked directly (synchronously). If this function returns
+  // |false|, the download manager will continue the download using a default,
+  // possibly empty, target path.

[Randy Smith (Not in Mondays), 2012/07/12 17:01:22]

I'd like to say something about the state changes that are possible on the
DownloadItem when calling this function.  I'd *like* the answer to be "none"
(i.e. only used for external data/observers), but that doesn't allow the user to
cancel if prompted.  So I'll settle for documenting what can and can't happen. 
(Though if it appealed to you to update DownloadTargetCallback to allow
propagating a cancel that way instead, I wouldn't object :-}.)

[asanka, 2012/07/18 21:50:56]

I made a note that the state of |item| shouldn't be changed save for external
data and changed the callback logic so that an empty target_path causes the
download to be canceled. This also means that unless a target path is specified,
the download would be implicitly canceled.

+  virtual bool DetermineDownloadTarget(DownloadItem* item,
+                                       const DownloadTargetCallback& callback);
 
   // Called when the download system wants to alert a WebContents that a
   // download has started, but the TabConetnts has gone away. This lets an