Replace the DownloadFileManager with direct ownership

content/public/browser/download_destination_controller.h

Index: content/public/browser/download_destination_controller.h
diff --git a/content/public/browser/download_destination_controller.h b/content/public/browser/download_destination_controller.h
new file mode 100644
index 0000000000000000000000000000000000000000..cc9734476f2b8e090b6fb2b56217d3f0d9af710e
--- /dev/null
+++ b/content/public/browser/download_destination_controller.h
@@ -0,0 +1,36 @@
+// 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_BROWSER_DOWNLOAD_DOWNLOAD_DESTINATION_CONTROLLER_H_
+#define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_DESTINATION_CONTROLLER_H_
+
+#include <string>
+
+#include "content/public/browser/download_interrupt_reasons.h"
+
+namespace content {
+
+// Class that receives asynchronous events from a DownloadDestination

[jam, 2012/07/24 20:47:02]

nit: here and below, make the comment take up to 80 chars

[Randy Smith (Not in Mondays), 2012/07/30 01:07:23]

I've reformatted to come closer to the 80 character limit, which is what I think
you wanted; let me know if I missed the point.

+// about downloading progress and completion.  These describe
+// the process of writing the data to the final location (e.g. |bytes_so_far| is
+// bytes_written, and |reason| is errors occuring on writing data).
+//
+// All methods must be invoked on the UI thread.
+//
+// Note that this interface does not deal with cross-thread lifetime

[benjhayden, 2012/07/25 15:19:16]

Would it make sense to make this class SupportsWeakPtr so that DownloadItemImpl
doesn't need to gain a method?
Or make DII derive SWP directly?

[Randy Smith (Not in Mondays), 2012/07/30 01:07:23]

That's not the reason for the method on DownloadItemImpl; that method is there
so that I can make DII inherit privately from this class and only export the
pointer where I choose.  Of course, the method is public, so that's a bit
disingenuous :-}.  Also ...
... there's a thread on chromium-dev about the sublties of SWP and why you
should prefer weak pointer factories.  I think it's possible to use SWP
correctly, but that it's easier to use factories correctly, so I went in this
direction.  But I don't have strong objections to SWP for DII (not DI :-}) if I
can convince myself that it doesn't fall into the traps discussed on
chromium-dev.  How much would you prefer DII inheriting from SWP?  (I *think*
the dangers are less if we make the derived class inherit from SWP rather than
the base class.)

+// issues.
+class DownloadDestinationController {

[jam, 2012/07/24 20:47:02]

nit: this is "Controller" but it just observes things and doesn't control
anything. can it be called Observer to be more like other content api classes? 

[Randy Smith (Not in Mondays), 2012/07/30 01:07:23]

Done.

+ public:
+  virtual void DestinationUpdate(int64 bytes_so_far,
+                                 int64 bytes_per_sec,
+                                 const std::string& hash_state) = 0;
+
+  virtual void DestinationError(DownloadInterruptReason reason) = 0;
+
+  virtual void DestinationCompleted(const std::string& final_hash) = 0;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_FILE_CONTROLLER_H_