Download filename determination refactor (3/3)

content/public/browser/download_manager_delegate.h

 // 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.
 
 #ifndef CONTENT_PUBLIC_BROWSER_DOWNLOAD_MANAGER_DELEGATE_H_
 #define CONTENT_PUBLIC_BROWSER_DOWNLOAD_MANAGER_DELEGATE_H_
 #pragma once
 
 #include "base/basictypes.h"
 #include "base/callback.h"
@@ skipping 24 matching lines @@
 
 // Browser's download manager: manages all downloads and destination view.
 class CONTENT_EXPORT DownloadManagerDelegate {
  public:
   // Lets the delegate know that the download manager is shutting down.
   virtual void Shutdown() {}
 
   // 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);
+  // 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.

 
-  // 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);
+  // 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
   // delegate return an alternative WebContents. The delegate can return NULL.
   virtual WebContents* GetAlternativeWebContentsToNotifyForDownload();
 
   // Tests if a file type should be opened automatically.
   virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path);
 
   // Allows the delegate to delay completion of the download.  This function
@@ skipping 38 matching lines @@
 
   // Notifies the delegate that it should remove the download item from its
   // persistent store.
   virtual void RemoveItemFromPersistentStore(DownloadItem* item) {}
 
   // Notifies the delegate to remove downloads from the given time range.
   virtual void RemoveItemsFromPersistentStoreBetween(
       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,
                           FilePath* download_save_dir,
                           bool* skip_dir_check) {}
 
   // Asks the user for the path to save a page. The delegate calls the callback
   // to give the answer.
   virtual void ChooseSavePath(WebContents* web_contents,
                               const FilePath& suggested_path,
                               const FilePath::StringType& default_extension,
                               bool can_save_as_complete,
                               const SavePackagePathPickedCallback& callback) {
   }
 
  protected:
   virtual ~DownloadManagerDelegate();
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_DOWNLOAD_MANAGER_DELEGATE_H_