Replace the DownloadFileManager with direct ownership

content/browser/download/download_item_impl.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_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_IMPL_H_
 #define CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_IMPL_H_
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "base/time.h"
 #include "base/timer.h"
 #include "content/browser/download/download_net_log_parameters.h"
 #include "content/browser/download/download_request_handle.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/download_destination_controller.h"
 #include "content/public/browser/download_id.h"
 #include "content/public/browser/download_item.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 
 class DownloadItemImplDelegate;
 
+namespace content {
+class DownloadFile;
+}
+
 // See download_item.h for usage.
-class CONTENT_EXPORT DownloadItemImpl : public content::DownloadItem {
+class CONTENT_EXPORT DownloadItemImpl
+    : public content::DownloadItem,
+      private content::DownloadDestinationController {
  public:
   // Note that it is the responsibility of the caller to ensure that a
   // DownloadItemImplDelegate passed to a DownloadItemImpl constructor
   // outlives the DownloadItemImpl.
 
   // Constructing from persistent store:
   // |bound_net_log| is constructed externally for our use.
   DownloadItemImpl(DownloadItemImplDelegate* delegate,
                    content::DownloadId download_id,
                    const content::DownloadPersistentStoreInfo& info,
@@ skipping 14 matching lines @@
                    const GURL& url,
                    bool is_otr,
                    content::DownloadId download_id,
                    const std::string& mime_type,
                    const net::BoundNetLog& bound_net_log);
 
   virtual ~DownloadItemImpl();
 
   // Implementation functions (not part of the DownloadItem interface).
 
+  // Start the download
+  virtual void Start(scoped_ptr<content::DownloadFile> download_file);
+
   // Indicate that an error has occurred on the download.
   virtual void Interrupt(content::DownloadInterruptReason reason);
 
   // Mark the item as having been persisted.
   virtual void SetIsPersisted();
 
   // Set the item's DB handle.
   virtual void SetDbHandle(int64 handle);
 
   // Cancels the off-thread aspects of the download.
   // TODO(rdsmith): This should be private and only called from
   // DownloadItem::Cancel/Interrupt; it isn't now because we can't
   // call those functions from
   // DownloadManager::FileSelectionCancelled() without doing some
   // rewrites of the DownloadManager queues.
-  virtual void OffThreadCancel(DownloadFileManager* file_manager);
+  virtual void OffThreadCancel();
 
   // Called when the downloaded file is removed.
   virtual void OnDownloadedFileRemoved();
 
   // Called when the download is ready to complete.
   // This may perform final rename if necessary and will eventually call
   // DownloadItem::Completed().
-  virtual void OnDownloadCompleting(DownloadFileManager* file_manager);
+  virtual void OnDownloadCompleting();
 
   // Called periodically from the download thread, or from the UI thread
   // for saving packages.
   // |bytes_so_far| is the number of bytes received so far.
   // |hash_state| is the current hash state.
   virtual void UpdateProgress(int64 bytes_so_far,
                               int64 bytes_per_sec,
                               const std::string& hash_state);
 
   // Called by SavePackage to display progress when the DownloadItem
   // should be considered complete.
   virtual void MarkAsComplete();
 
   // Called when all data has been saved. Only has display effects.
-  virtual void OnAllDataSaved(int64 size, const std::string& final_hash);
+  virtual void OnAllDataSaved(const std::string& final_hash);
 
   // Called by SavePackage to set the total number of bytes on the item.
   virtual void SetTotalBytes(int64 total_bytes);
 
+  // Provide a weak pointer reference to a DownloadDestinationController
+  // for use by download destinations.
+  base::WeakPtr<content::DownloadDestinationController>
+      DestinationControllerAsWeakPtr();
+
   // Overridden from DownloadItem.
   virtual void AddObserver(DownloadItem::Observer* observer) OVERRIDE;
   virtual void RemoveObserver(DownloadItem::Observer* observer) OVERRIDE;
   virtual void UpdateObservers() OVERRIDE;
   virtual bool CanShowInFolder() OVERRIDE;
   virtual bool CanOpenDownload() OVERRIDE;
   virtual bool ShouldOpenFileBasedOnExtension() OVERRIDE;
   virtual void OpenDownload() OVERRIDE;
   virtual void ShowDownloadInShell() OVERRIDE;
   virtual void DangerousDownloadValidated() OVERRIDE;
@@ skipping 16 matching lines @@
   virtual const FilePath& GetFullPath() const OVERRIDE;
   virtual const FilePath& GetTargetFilePath() const OVERRIDE;
   virtual TargetDisposition GetTargetDisposition() const OVERRIDE;
   virtual void OnTargetPathDetermined(
       const FilePath& target_path,
       TargetDisposition disposition,
       content::DownloadDangerType danger_type) OVERRIDE;
   virtual void OnTargetPathSelected(const FilePath& target_path) OVERRIDE;
   virtual void OnContentCheckCompleted(
       content::DownloadDangerType danger_type) OVERRIDE;
-  virtual void OnIntermediatePathDetermined(DownloadFileManager* file_manager,
-                                            const FilePath& path) OVERRIDE;
+  virtual void OnIntermediatePathDetermined(const FilePath& path) OVERRIDE;
   virtual const GURL& GetURL() const OVERRIDE;
   virtual const std::vector<GURL>& GetUrlChain() const OVERRIDE;
   virtual const GURL& GetOriginalUrl() const OVERRIDE;
   virtual const GURL& GetReferrerUrl() const OVERRIDE;
   virtual std::string GetSuggestedFilename() const OVERRIDE;
   virtual std::string GetContentDisposition() const OVERRIDE;
   virtual std::string GetMimeType() const OVERRIDE;
   virtual std::string GetOriginalMimeType() const OVERRIDE;
   virtual std::string GetReferrerCharset() const OVERRIDE;
   virtual std::string GetRemoteAddress() const OVERRIDE;
@@ skipping 34 matching lines @@
   virtual FilePath GetFileNameToReportUser() const OVERRIDE;
   virtual void SetDisplayName(const FilePath& name) OVERRIDE;
   virtual FilePath GetUserVerifiedFilePath() const OVERRIDE;
   virtual std::string DebugString(bool verbose) const OVERRIDE;
   virtual void MockDownloadOpenForTesting() OVERRIDE;
   virtual ExternalData* GetExternalData(const void* key) OVERRIDE;
   virtual const ExternalData* GetExternalData(const void* key) const OVERRIDE;
   virtual void SetExternalData(const void* key, ExternalData* data) OVERRIDE;
 
  private:
+  // DownloadDestinationController
+  virtual void DestinationUpdate(int64 bytes_so_far,
+                                 int64 bytes_per_sec,
+                                 const std::string& hash_state) OVERRIDE;
+  virtual void DestinationError(
+      content::DownloadInterruptReason reason) OVERRIDE;
+  virtual void DestinationCompleted(const std::string& final_hash) OVERRIDE;
+
+  // For weak pointer downlcasting.

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

I have the same problem! You start typing some word that happens to start with
"down", and before you know it you've already finished typing "load".

We need foot pedals that type "Download" and "download" so that we never have to
type them and our brains can start to heal some of the damage.

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

I couldn't figure out what you were talking about for a moment (I mean, I know
exactly the experience you're talking about, but couldn't figure out how it
related to this line), but you got me LOLing when I did.  Fixed.

+  friend class base::WeakPtr<content::DownloadDestinationController>;
+
   // Construction common to all constructors. |active| should be true for new
   // downloads and false for downloads from the history.
   // |download_type| indicates to the net log system what kind of download
   // this is.
   void Init(bool active, download_net_logs::DownloadType download_type);
 
   // Returns true if the download still needs to be renamed to
   // GetTargetFilePath().
   bool NeedsRename() const;
 
   // If all pre-requisites have been met, complete download processing, i.e.  do
   // internal cleanup, file rename, and potentially auto-open.  (Dangerous
   // downloads still may block on user acceptance after this point.)
   void MaybeCompleteDownload();
 
-  // Internal helper for maintaining consistent received and total sizes, and
-  // setting the final hash.
-  // Should only be called from |OnAllDataSaved|.
-  void ProgressComplete(int64 bytes_so_far,
-                        const std::string& final_hash);
-
   // Called when the entire download operation (including renaming etc)
   // is completed.
   void Completed();
 
   // Call to transition state; all state transitions should go through this.
   void TransitionTo(DownloadState new_state);
 
   // Set the |danger_type_| and invoke obserers if necessary.
   void SetDangerType(content::DownloadDangerType danger_type);
 
   // Set the |current_path_| to |new_path|.
   void SetFullPath(const FilePath& new_path);
 
   // Callback invoked when the download has been renamed to its final name.
-  void OnDownloadRenamedToFinalName(DownloadFileManager* file_manager,
-                                    content::DownloadInterruptReason reason,
+  void OnDownloadRenamedToFinalName(content::DownloadInterruptReason reason,
                                     const FilePath& full_path);
 
   // Callback invoked when the download has been renamed to its intermediate
   // name.
   void OnDownloadRenamedToIntermediateName(
       content::DownloadInterruptReason reason, const FilePath& full_path);
 
+  // Callback from file thread when we initialize the DownloadFile.
+  void OnDownloadFileInitialized(
+      content::DownloadInterruptReason result);
+
   // Callback from file thread when we release the DownloadFile.
   void OnDownloadFileReleased();
 
   // The handle to the request information.  Used for operations outside the
   // download system.
   scoped_ptr<DownloadRequestHandleInterface> request_handle_;
 
   // Download ID assigned by DownloadResourceHandler.
   content::DownloadId download_id_;
 
@@ skipping 145 matching lines @@
   // Do we actually open downloads when requested?  For testing purposes only.
   bool open_enabled_;
 
   // Did the delegate delay calling Complete on this download?
   bool delegate_delayed_complete_;
 
   // External Data storage.  All objects in the store
   // are owned by the DownloadItemImpl.
   std::map<const void*, ExternalData*> external_data_map_;
 
+  // DownloadFile associated with this download.  Note that this
+  // pointer may only be used or destroyed on the FILE thread.

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

Then maybe it shouldn't be scoped_ptr? scoped_ptr suggests that this member will
be deleted when the owner is deleted.
Is there a generalization of scoped_ptr that affords deletion-on-another-thread?

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

Well, I want to strongly indicate the ownership relationship.  I don't think
there's a generalization of scoped_ptr<>, but I do have a DCHECK() in the
destructor of DownloadItem that the scoped_ptr is null at that point.  That good
enough for expressing ownership but warding off wrong-thread destruction?

+  scoped_ptr<content::DownloadFile> download_file_;
+
   // Net log to use for this download.
   const net::BoundNetLog bound_net_log_;
 
   base::WeakPtrFactory<DownloadItemImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadItemImpl);
 };
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_ITEM_IMPL_H_