Refactor downloads into a ProfileKeyedService.

content/browser/download/download_manager.h

 // Copyright (c) 2011 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 276 matching lines @@
 
   // Called when Save Page download is done.
   void SavePageDownloadFinished(DownloadItem* download);
 
   // Get the download item from the active map.  Useful when the item is not
   // yet in the history map.
   DownloadItem* GetActiveDownloadItem(int id);
 
   DownloadManagerDelegate* delegate() const { return delegate_; }
 
+  // For testing only.
+  void SetDownloadManagerDelegate(DownloadManagerDelegate* delegate);

[jam, 2011/10/10 18:19:33]

nit: please add a "ForTesting" suffix. we have presubmit checks that ensure that
they're only called by test code.

[Miranda Callahan, 2011/10/11 14:53:03]

Whoa, I didn't know that we had a presubmit check for that -- that's great! I
added a line to the Chromium style guide on naming to make that clear.

[Randy Smith (Not in Mondays), 2011/10/11 18:24:43]

Can I convince you to rescind this request?  The problem is that I want to keep
the delegates in sync between DownloadService and DownloadManager, and so I call
DownloadManager::SetDownloadManagerDelegate(ForTesting :-}) from
DownloadService::SetDownloadManagerDelegateForTesting.  I can (and will) change
the name of the DownloadService call, but I'm already having to make the test
code wonkier and more prone to error than I'd like (see response to other
comment), and I'd rather have DownloadService call the DownloadManager version. 

[jam, 2011/10/11 19:31:13]

sure, sounds reasonable. ideally, the presubmit check wouldn't count calls from
other ForTest functions :)

+
  private:
   typedef std::set<DownloadItem*> DownloadSet;
   typedef base::hash_map<int64, DownloadItem*> DownloadMap;
 
   // For testing.
   friend class DownloadManagerTest;
+  friend class DownloadTest;
   friend class MockDownloadManager;
-  friend class DownloadTest;
 
   friend class base::RefCountedThreadSafe<DownloadManager,
                                           BrowserThread::DeleteOnUIThread>;
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class DeleteTask<DownloadManager>;
 
-  void set_delegate(DownloadManagerDelegate* delegate) { delegate_ = delegate; }
-
   virtual ~DownloadManager();
 
   // Called on the FILE thread to check the existence of a downloaded file.
   void CheckForFileRemovalOnFileThread(int64 db_handle, const FilePath& path);
 
   // Called on the UI thread if the FILE thread detects the removal of
   // the downloaded file. The UI thread updates the state of the file
   // and then notifies this update to the file's observer.
   void OnFileRemovalDetected(int64 db_handle);
 
@@ skipping 104 matching lines @@
   DownloadManagerDelegate* delegate_;
 
   // TODO(rdsmith): Remove when http://crbug.com/85408 is fixed.
   // For debugging only.
   int64 largest_db_handle_in_history_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadManager);
 };
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_DOWNLOAD_MANAGER_H_