Replace the DownloadFileManager with direct ownership

content/browser/download/download_file.h

Index: content/browser/download/download_file.h
diff --git a/content/browser/download/download_file.h b/content/browser/download/download_file.h
index e707a26ca10607149db7f8e14d587451e6d20cb8..e53602d46ecd6956451e77acaf8e1e3529b8fb96 100644
--- a/content/browser/download/download_file.h
+++ b/content/browser/download/download_file.h
@@ -11,7 +11,6 @@
 #include "base/callback_forward.h"
 #include "base/file_path.h"
 #include "content/common/content_export.h"
-#include "content/public/browser/download_id.h"
 #include "content/public/browser/download_interrupt_reasons.h"
 
 namespace content {
@@ -24,6 +23,12 @@ class DownloadManager;
 // cancelled, the DownloadFile is destroyed.
 class CONTENT_EXPORT DownloadFile {
  public:
+  // Callback used with Initialize.  On a successful initialize, |reason| will
+  // be DOWNLOAD_INTERRUPT_REASON_NONE; on a failed initialize, it will be
+  // set to the reason for the failure.
+  typedef base::Callback<void(content::DownloadInterruptReason reason)>
+      InitializeCallback;
+
   // Callback used with Rename().  On a successful rename |reason| will be
   // DOWNLOAD_INTERRUPT_REASON_NONE and |path| the path the rename
   // was done to.  On a failed rename, |reason| will contain the
@@ -33,10 +38,10 @@ class CONTENT_EXPORT DownloadFile {
 
   virtual ~DownloadFile() {}
 
-  // If calculate_hash is true, sha256 hash will be calculated.
   // Returns DOWNLOAD_INTERRUPT_REASON_NONE on success, or a network
-  // error code on failure.
-  virtual DownloadInterruptReason Initialize() = 0;
+  // error code on failure.  Upon completion, |callback| will be
+  // called on the UI thread as per the comment above.
+  virtual void Initialize(const InitializeCallback& callback) = 0;
 
   // Rename the download file to |full_path|.  If that file exists and
   // |overwrite_existing_file| is false, |full_path| will be uniquified by
@@ -68,14 +73,11 @@ class CONTENT_EXPORT DownloadFile {
   // Returns the current (intermediate) state of the hash as a byte string.
   virtual std::string GetHashState() = 0;
 
-  // Cancels the download request associated with this file.
-  virtual void CancelDownloadRequest() = 0;
-
-  virtual int Id() const = 0;
-  virtual DownloadManager* GetDownloadManager() = 0;
-  virtual const DownloadId& GlobalId() const = 0;
+  // For testing.  Must be called on FILE thread.
+  static int GetNumberOfDownloadFiles();

[jam, 2012/08/06 18:16:37]

nit: if this is just for testing, add a "ForTesting" suffix. that way presubmit
checks ensure that only test code uses this, and production code doesn't
inadvertently start using it.

[benjhayden, 2012/08/10 15:56:41]

Looks like this is only called in download_manager_impl.cc, but is indirectly
only called from download_browsertest.cc. How does presubmit feel about that?

Randy, I know we talked about how to get rid of this mechanism in another CL,
but I don't remember if we came up with a particular substitute. Do you
remember?

[Randy Smith (Not in Mondays), 2012/08/10 20:09:00]

See Ben's comment--this is called from download_manager_impl.cc, but only on the
behalf of download_browsertest.cc.  Leaving as is unless you can guide me as to
a better way to manage this.

[Randy Smith (Not in Mondays), 2012/08/10 20:09:00]

Yep.  I hope I wrote it down in my bucket of download cleanup CLs list; checking
.... yep.  The idea is that we use the DownloadFileFactory method on the
DownloadManagerImpl to put in a DownloadFileFactory that creates objects derived
from DownloadFiles that track their creation and destruction this way.  It's
more complex code, but the complexity will all be in the testing code, so it's
worth it.  I intend (:-J) to do that reasonably soon after I land this CL, as I
feel like I sorta promised it to you in another context.

 
-  virtual std::string DebugString() const = 0;
+ protected:
+  static int number_active_objects_;
 };
 
 }  // namespace content