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..5b3e328946c554487f0081feff8fd213cedc8417 100644
--- a/content/public/browser/download_manager_delegate.h
+++ b/content/public/browser/download_manager_delegate.h
@@ -42,23 +42,15 @@ 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);
+  // Notifies the delegate that a new download needs

[Randy Smith (Not in Mondays), 2012/07/05 15:42:32]

nit: dangling sentence.

[asanka, 2012/07/09 21:13:46]

Done.

+
+  // The delegate needs to generate a filename for the new download |item|. Once
+  // the target information has been determined, the delegate is required to
+  // call DownloadItem::OnDownloadTargetDetermined(). This function should
+  // return |true| if it has or will invoke OnDownloadTargetDetermined(). If it

[Randy Smith (Not in Mondays), 2012/07/05 15:42:32]

The interface on DMI is OnDownloadTargetAvailable, which I think is an
inconsistency?

[Randy Smith (Not in Mondays), 2012/07/05 15:42:32]

Could we do this through a callback rather than a specified interface on the DM?

[asanka, 2012/07/09 21:13:46]

Done.

[asanka, 2012/07/09 21:13:46]

Done.

+  // returns |false|, the download manager will continue the download using a
+  // default, possibly empty, target path.

[Randy Smith (Not in Mondays), 2012/07/05 15:42:32]

When I look at the new DMI interface (OnDownloadTargetAvailable) it looks to me
like this is expecting the DMD to change state directly on the DownloadItem.  My
preference would be to keep state changes inside of the core download system,
which would mean making the callback take the passed back information and doing
the state transition then.   How hard would it be to do it this way?

(Eventually, I'd like the call out to come from the DI, indirect through the DM,
and hit the DMD, with a callback passed to get the results, which means we won't
have to have any setters for this process on the DI.  Not for this CL :-}.)  

[asanka, 2012/07/09 21:13:46]

I added a callback parameter that the DMD is supposed to invoke if it wants to
determine an explicit path.

+  virtual bool DetermineDownloadTarget(DownloadItem* item);
 
   // Called when the download system wants to alert a WebContents that a
   // download has started, but the TabConetnts has gone away. This lets an
@@ -117,6 +109,10 @@ class CONTENT_EXPORT DownloadManagerDelegate {
       base::Time remove_begin,
       base::Time remove_end) {}
 
+  // Invoked when history is being cleared to clear the delegate's transient
+  // state.
+  virtual void ClearTransientState() {}
+
   // Retrieve the directories to save html pages and downloads to.
   virtual void GetSaveDir(WebContents* web_contents,
                           FilePath* website_save_dir,