Refactor downloads into a ProfileKeyedService.

chrome/browser/download/download_browsertest.cc

Index: chrome/browser/download/download_browsertest.cc
diff --git a/chrome/browser/download/download_browsertest.cc b/chrome/browser/download/download_browsertest.cc
index f613b27e8b7f388e38d10b63e3695f0b9b3a8782..dd0b8d3a1488b9e3ff50aecdeebb2484d5f65a82 100644
--- a/chrome/browser/download/download_browsertest.cc
+++ b/chrome/browser/download/download_browsertest.cc
@@ -16,6 +16,8 @@
 #include "chrome/browser/download/download_crx_util.h"
 #include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_prefs.h"
+#include "chrome/browser/download/download_service.h"
+#include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/download/download_shelf.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/extensions/extension_install_ui.h"
@@ -497,7 +499,8 @@ class PickSuggestedFileDelegate : public ChromeDownloadManagerDelegate {
  public:
   explicit PickSuggestedFileDelegate(Profile* profile)
       : ChromeDownloadManagerDelegate(profile) {
-    SetDownloadManager(profile->GetDownloadManager());
+    SetDownloadManager(
+        DownloadServiceFactory::GetForProfile(profile)->GetDownloadManager());
   }
 
   virtual void ChooseDownloadPath(TabContents* tab_contents,
@@ -593,6 +596,11 @@ class MockAutoConfirmExtensionInstallUI : public ExtensionInstallUI {
   virtual void OnInstallFailure(const std::string& error) {}
 };
 
+static DownloadManager* DownloadManagerForBrowser(Browser* browser) {
+  return DownloadServiceFactory::GetForProfile(browser->profile())
+      ->GetDownloadManager();

[Miranda Callahan, 2011/10/06 20:07:32]

nit: I think this is nonstandard wrapping (at least in chrome code); perhaps put
the whole argument on the next line?

return DownloadServiceFactory::GetForProfile(
    browser->profile()->GetDownloadManager());

[Randy Smith (Not in Mondays), 2011/10/08 23:46:54]

The problem is that that's not the right parenthesis nesting, and it looked as
wrong to me with the correct parenthesis nesting as what I actually wrote. 
Here's the alternative:

  return DownloadServiceFactory::GetForProfile(
      browser->profile())->GetDownloadManager();

If you think that's better, I'm happy to go with it--just let me know.

+}
+
 }  // namespace
 
 // While an object of this class exists, it will mock out download
@@ -665,7 +673,7 @@ class DownloadTest : public InProcessBrowserTest {
     browser()->profile()->GetPrefs()->SetBoolean(prefs::kPromptForDownload,
                                                  prompt_for_download);
 
-    DownloadManager* manager = browser()->profile()->GetDownloadManager();
+    DownloadManager* manager = DownloadManagerForBrowser(browser());
     DownloadPrefs::FromDownloadManager(manager)->ResetAutoOpen();
     manager->RemoveAllDownloads();
 
@@ -709,7 +717,7 @@ class DownloadTest : public InProcessBrowserTest {
 
   DownloadPrefs* GetDownloadPrefs(Browser* browser) {
     return DownloadPrefs::FromDownloadManager(
-        browser->profile()->GetDownloadManager());
+        DownloadManagerForBrowser(browser));
   }
 
   FilePath GetDownloadDirectory(Browser* browser) {
@@ -719,8 +727,7 @@ class DownloadTest : public InProcessBrowserTest {
   // Create a DownloadsObserver that will wait for the
   // specified number of downloads to finish.
   DownloadsObserver* CreateWaiter(Browser* browser, int num_downloads) {
-    DownloadManager* download_manager =
-        browser->profile()->GetDownloadManager();
+    DownloadManager* download_manager = DownloadManagerForBrowser(browser);
     return new DownloadsObserver(
         download_manager, num_downloads,
         DownloadItem::COMPLETE,  // Really done
@@ -732,8 +739,7 @@ class DownloadTest : public InProcessBrowserTest {
   // specified number of downloads to start.
   DownloadsObserver* CreateInProgressWaiter(Browser* browser,
                                             int num_downloads) {
-    DownloadManager* download_manager =
-        browser->profile()->GetDownloadManager();
+    DownloadManager* download_manager = DownloadManagerForBrowser(browser);
     return new DownloadsObserver(
         download_manager, num_downloads,
         DownloadItem::IN_PROGRESS,      // Has started
@@ -749,8 +755,7 @@ class DownloadTest : public InProcessBrowserTest {
       int num_downloads,
       DownloadItem::DownloadState final_state,
       DangerousDownloadAction dangerous_download_action) {
-    DownloadManager* download_manager =
-        browser->profile()->GetDownloadManager();
+    DownloadManager* download_manager = DownloadManagerForBrowser(browser);
     return new DownloadsObserver(
         download_manager, num_downloads,
         final_state,
@@ -910,7 +915,7 @@ class DownloadTest : public InProcessBrowserTest {
 
   void GetDownloads(Browser* browser, std::vector<DownloadItem*>* downloads) {
     DCHECK(downloads);
-    DownloadManager* manager = browser->profile()->GetDownloadManager();
+    DownloadManager* manager = DownloadManagerForBrowser(browser);
     manager->SearchDownloads(string16(), downloads);
   }
 
@@ -972,13 +977,9 @@ class DownloadTest : public InProcessBrowserTest {
     PickSuggestedFileDelegate* new_delegate =
         new PickSuggestedFileDelegate(browser->profile());
 
-    DownloadManager* manager = browser->profile()->GetDownloadManager();
-
-    new_delegate->SetDownloadManager(manager);
-    manager->set_delegate(new_delegate);
-
-    // Gives ownership to Profile.
-    browser->profile()->SetDownloadManagerDelegate(new_delegate);
+    // Gives ownership to DownloadService.
+    DownloadServiceFactory::GetForProfile(
+        browser->profile())->SetDownloadManagerDelegate(new_delegate);
   }
 
  private:
@@ -1045,7 +1046,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadMimeTypeSelect) {
   // due to the MIME type, but we still wait until the download completes.
   scoped_ptr<DownloadsObserver> observer(
       new DownloadsObserver(
-          browser()->profile()->GetDownloadManager(),
+          DownloadManagerForBrowser(browser()),
           1,
           DownloadItem::COMPLETE,  // Really done
           false,                   // Continue on select file.
@@ -1490,7 +1491,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadCancelled) {
   observer->WaitForFinished();
 
   std::vector<DownloadItem*> downloads;
-  browser()->profile()->GetDownloadManager()->SearchDownloads(
+  DownloadManagerForBrowser(browser())->SearchDownloads(
       string16(), &downloads);
   ASSERT_EQ(1u, downloads.size());
   ASSERT_EQ(DownloadItem::IN_PROGRESS, downloads[0]->state());
@@ -1499,7 +1500,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadCancelled) {
   // Cancel the download and wait for download system quiesce.
   downloads[0]->Delete(DownloadItem::DELETE_DUE_TO_USER_DISCARD);
   scoped_refptr<DownloadsFlushObserver> flush_observer(
-      new DownloadsFlushObserver(browser()->profile()->GetDownloadManager()));
+      new DownloadsFlushObserver(DownloadManagerForBrowser(browser())));
   flush_observer->WaitForFlush();
 
   // Get the important info from other threads and check it.
@@ -1540,7 +1541,7 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, DownloadHistoryCheck) {
   // Check history results.
   DownloadsHistoryDataCollector history_collector(
       db_handle,
-      browser()->profile()->GetDownloadManager());
+      DownloadManagerForBrowser(browser()));
   DownloadPersistentStoreInfo info;
   EXPECT_TRUE(history_collector.GetDownloadsHistoryEntry(&info)) << db_handle;
   EXPECT_EQ(file, info.path.BaseName());
@@ -1634,13 +1635,13 @@ IN_PROC_BROWSER_TEST_F(DownloadTest, AutoOpen) {
 
   // Mock out external opening on all downloads until end of test.
   MockDownloadOpeningObserver observer(
-      browser()->profile()->GetDownloadManager());
+      DownloadManagerForBrowser(browser()));
 
   DownloadAndWait(browser(), url, EXPECT_NO_SELECT_DIALOG);
 
   // Find the download and confirm it was opened.
   std::vector<DownloadItem*> downloads;
-  browser()->profile()->GetDownloadManager()->SearchDownloads(
+  DownloadManagerForBrowser(browser())->SearchDownloads(
       string16(), &downloads);
   ASSERT_EQ(1u, downloads.size());
   EXPECT_EQ(DownloadItem::COMPLETE, downloads[0]->state());