DownloadManagerDelegate::ShouldCompleteDownload(callback)

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 51 matching lines @@
 
   // 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 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
   // DownloadItem::DelayedDownloadOpened when it's done with the item,
   // and is responsible for opening it.  This function is called
   // after the final rename, but before the download state is set to COMPLETED.
   virtual bool ShouldOpenDownload(DownloadItem* item);
 
   // Returns true if we need to generate a binary hash for downloads.
   virtual bool GenerateFileHash();
@@ skipping 36 matching lines @@
                               const FilePath& suggested_path,
                               const FilePath::StringType& default_extension,
                               bool can_save_as_complete,
                               const SavePackagePathPickedCallback& callback) {
   }
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_DOWNLOAD_MANAGER_DELEGATE_H_