DownloadManagerDelegate::ShouldCompleteDownload(callback)

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 641920ff3ba0faa34527facd01f98a34bcfe020c..6806f0396496366ce7ca1e24de38e5df9ca92458 100644
--- a/content/public/browser/download_manager_delegate.h
+++ b/content/public/browser/download_manager_delegate.h
@@ -69,13 +69,23 @@ class CONTENT_EXPORT DownloadManagerDelegate {
   virtual bool ShouldOpenFileBasedOnExtension(const FilePath& path);
 
   // Allows the delegate to override completion of the download.  If this
-  // function returns false, the download completion is delayed and the
-  // delegate is responsible for making sure that
-  // DownloadItem::MaybeCompleteDownload is called at some point in the
-  // future.  Note that at that point this function will be called again,
-  // and is responsible for returning true when it really is ok for the
-  // download to complete.
-  virtual bool ShouldCompleteDownload(DownloadItem* item);
+  // function returns false, the download completion is delayed and the delegate
+  // is responsible for making sure that |complete_callback| is run at some
+  // point in the future.  Note that at that point this function will be called
+  // again, and is responsible for returning true when it really is ok for the
+  // download to complete. If this method returns true, then |complete_callback|
+  // will not be run. It is safe to call this method any number of times both
+  // before and after |complete_callback| runs; this method will return false
+  // until the download should complete, then it will run the last
+  // |complete_callback| passed to it and begin returning true when called.
+  // The last complete_callback will be run when the return value of this
+  // function may have changed to true, but that complete_callback being run
+  // doesn't actually indicate that the download is ready for completion; the
+  // function needs to be called again to check.  If the

[Randy Smith (Not in Mondays), 2012/05/09 00:50:56]

Minor point: This is false to how this interface is used in save package, i.e.
we're counting on undocumented behavior, which isn't good.  However, I think
we're rewriting this anyway :-}.  Suggested doc text (please feel free to tweak
if you believe clarity would be enhanced):

Allows the delegate to delay completion of the download.  This function will
either return true (in which case the download is ready to complete) or arrange
for complete_callback to be called at some point in the future when the download
is ready to complete.  

The function may be called multiple times; if it is, only the last callback
specified (while the delegate is delaying completion) will be run.  Calls made
after the callback is run are guaranteed to return true.  

[benjhayden, 2012/05/14 15:35:05]

LGTM Thanks!

+  // method returns true, complete_callback will not be run.
+  virtual bool ShouldCompleteDownload(
+      DownloadItem* item,
+      const base::Closure& complete_callback);
 
   // Allows the delegate to override opening the download. If this function
   // returns false, the delegate needs to call