Move creation and ownership of DownloadManager from the embedder to content. This matches all the o…

content/public/browser/download_manager.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.
 //
 // The DownloadManager object manages the process of downloading, including
 // updates to the history system and providing the information for displaying
 // the downloads view in the Destinations tab. There is one DownloadManager per
 // active browser context in Chrome.
 //
 // Download observers:
@@ skipping 42 matching lines @@
 
 class BrowserContext;
 class DownloadManagerDelegate;
 class DownloadQuery;
 class DownloadUrlParameters;
 
 // Browser's download manager: manages all downloads and destination view.
 class CONTENT_EXPORT DownloadManager
     : public base::RefCountedThreadSafe<DownloadManager> {
  public:
-  static DownloadManager* Create(
-      DownloadManagerDelegate* delegate,
-      net::NetLog* net_log);
-
   // A method that can be used in tests to ensure that all the internal download
   // classes have no pending downloads.
   static bool EnsureNoPendingDownloadsForTesting();
 
-  // Shutdown the download manager. Must be called before destruction.
+  // Sets the delegate for this DownloadManager. The delegate has to live past
+  // its Shutdown method being called.

[Randy Smith (Not in Mondays), 2012/06/06 15:51:59]

Which is under the control of the DownloadManager?  I don't see how the contract
is useful to the DM/Delegate interface unless there's also a guarantee that the
delegate's Shutdown method won't be called except by the DM, and the DM is
required to call it?

[jam, 2012/06/06 16:06:15]

I've clarified the comment to say that Shutdown is called by DM.

+  virtual void SetDelegate(DownloadManagerDelegate* delegate) = 0;

[tfarina, 2012/06/06 15:44:15]

There is already a SetDownloadManagerDelegate in this interface, see line 238

[jam, 2012/06/06 15:53:05]

thanks, I'll remove SetDownloadManagerDelegate 

+
+  // Shutdown the download manager. Called by content before destruction.
   virtual void Shutdown() = 0;
 
   // Interface to implement for observers that wish to be informed of changes
   // to the DownloadManager's collection of downloads.
   class CONTENT_EXPORT Observer {
    public:
     // New or deleted download, observers should query us for the current set
     // of downloads.
     virtual void ModelChanged(DownloadManager* manager) = 0;
 
@@ skipping 142 matching lines @@
 
   // Called when Save Page download is done.
   virtual void SavePageDownloadFinished(DownloadItem* download) = 0;
 
   // Get the download item from the active map.  Useful when the item is not
   // yet in the history map.
   virtual DownloadItem* GetActiveDownloadItem(int id) = 0;
 
   virtual bool GenerateFileHash() = 0;
 
   virtual DownloadManagerDelegate* delegate() const = 0;

[tfarina, 2012/06/06 15:44:15]

nit: s/delegate/GetDelegate

 
   // For testing only.  May be called from tests indirectly (through
   // other for testing only methods).
   virtual void SetDownloadManagerDelegate(
       DownloadManagerDelegate* delegate) = 0;
 
  protected:
   virtual ~DownloadManager() {}
 
  private:
   friend class base::RefCountedThreadSafe<DownloadManager>;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_PUBLIC_BROWSER_DOWNLOAD_MANAGER_H_