Remove DownloadFileManager in favor of direct ownership of DownloadFiles.

chrome/browser/download/save_page_browsertest.cc

Index: chrome/browser/download/save_page_browsertest.cc
diff --git a/chrome/browser/download/save_page_browsertest.cc b/chrome/browser/download/save_page_browsertest.cc
index d3f737bcfdf7a22d064c104a78db8c2b440e6cf1..e2103439a82841f12d15f0cd6df756b35047f97f 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -64,6 +64,9 @@ static const char* kAppendedExtension =
     ".html";
 #endif
 
+void NullFunction() {
+}
+
 }  // namespace
 
 // Loosely based on logic in DownloadTestObserver.
@@ -101,7 +104,8 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer {
  private:
 
   // DownloadManager::Observer
-  void OnDownloadCreated(DownloadManager* manager, DownloadItem* item) {
+  virtual void OnDownloadCreated(
+      DownloadManager* manager, DownloadItem* item) OVERRIDE {
     DCHECK_EQ(manager, manager_);
     if (!created_item_)
       created_item_ = item;
@@ -110,7 +114,7 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer {
       MessageLoopForUI::current()->Quit();
   }
 
-  void ManagerGoingDownload(DownloadManager* manager) {
+  virtual void ManagerGoingDown(DownloadManager* manager) OVERRIDE {
     manager_->RemoveObserver(this);
     manager_ = NULL;
     if (waiting_)
@@ -124,6 +128,60 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer {
   DISALLOW_COPY_AND_ASSIGN(DownloadItemCreatedObserver);
 };
 
+class DownloadPersistedObserver : public DownloadItem::Observer {
+ public:
+  explicit DownloadPersistedObserver(DownloadItem* item)
+      : waiting_(false), item_(item) {
+    item->AddObserver(this);
+  }
+
+  ~DownloadPersistedObserver() {
+    if (item_)
+      item_->RemoveObserver(this);
+  }
+
+  // Wait for download item to get the persisted bit set.
+  // Note that this class provides no protection against the download
+  // being destroyed between creation and return of WaitForPersisted();
+  // the caller must guarantee that in some other fashion.
+  void WaitForPersisted() {
+    // In combination with OnDownloadDestroyed() below, verify the
+    // above interface contract.
+    DCHECK(item_);
+
+    if (item_->IsPersisted())
+      return;
+
+    waiting_ = true;
+    content::RunMessageLoop();
+    waiting_ = false;
+
+    return;
+  }
+
+ private:
+  // DownloadItem::Observer
+  virtual void OnDownloadUpdated(DownloadItem* item) OVERRIDE {
+    DCHECK_EQ(item, item_);
+
+    if (waiting_ && item->IsPersisted())
+      MessageLoopForUI::current()->Quit();
+  }
+
+  virtual void OnDownloadDestroyed(DownloadItem* item) OVERRIDE {
+    if (item != item_)
+      return;
+
+    item_->RemoveObserver(this);
+    item_ = NULL;
+  }
+
+  bool waiting_;
+  DownloadItem* item_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadPersistedObserver);
+};
+
 class SavePageBrowserTest : public InProcessBrowserTest {
  public:
   SavePageBrowserTest() {}
@@ -165,12 +223,12 @@ class SavePageBrowserTest : public InProcessBrowserTest {
     return current_tab;
   }
 
-
   GURL WaitForSavePackageToFinish() const {
     content::WindowedNotificationObserver observer(
         content::NOTIFICATION_SAVE_PACKAGE_SUCCESSFULLY_FINISHED,
         content::NotificationService::AllSources());
     observer.Wait();
+
     return content::Details<DownloadItem>(observer.details()).ptr()->
         GetOriginalUrl();
   }
@@ -209,11 +267,12 @@ class SavePageBrowserTest : public InProcessBrowserTest {
 
     DownloadPersistentStoreInfoMatch(const GURL& url,
                                      const FilePath& path,
-                                     int64 num_files)
+                                     int64 num_files,
+                                     DownloadItem::DownloadState state)
       : url_(url),
         path_(path),
-        num_files_(num_files) {
-    }
+        num_files_(num_files),
+        state_(state) {}
 
     bool operator() (const DownloadPersistentStoreInfo& info) const {
       return info.url == url_ &&
@@ -223,22 +282,37 @@ class SavePageBrowserTest : public InProcessBrowserTest {
              ((num_files_ < 0) ||
               (info.received_bytes == num_files_)) &&
              info.total_bytes == 0 &&
-             info.state == DownloadItem::COMPLETE;
+             info.state == state_;
     }
 
     GURL url_;
     FilePath path_;
     int64 num_files_;
+    DownloadItem::DownloadState state_;
   };
 
   void CheckDownloadHistory(const GURL& url,
                             const FilePath& path,
-                            int64 num_files) {
+                            int64 num_files,
+                            DownloadItem::DownloadState state) {
+    // Make sure the relevant download item made it into the history.
+    std::vector<DownloadItem*> downloads;
+    GetDownloadManager()->SearchDownloads(string16(), &downloads);
+    ASSERT_EQ(1u, downloads.size());
+    DownloadPersistedObserver(downloads[0]).WaitForPersisted();
+
+    // Make sure any final updates have made it to the history DB.
+    BrowserThread::PostTaskAndReply(
+        BrowserThread::DB, FROM_HERE, base::Bind(&NullFunction),
+        MessageLoop::QuitClosure());
+    content::RunMessageLoop();
+
     QueryDownloadHistory();
 
     std::vector<DownloadPersistentStoreInfo>::iterator found =
       std::find_if(history_entries_.begin(), history_entries_.end(),
-                   DownloadPersistentStoreInfoMatch(url, path, num_files));
+                   DownloadPersistentStoreInfoMatch(url, path, num_files,
+                                                    state));
 
     if (found == history_entries_.end()) {
       LOG(ERROR) << "Missing url=" << url.spec()
@@ -282,7 +356,8 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnly) {
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
-  CheckDownloadHistory(url, full_file_name, 1);  // a.htm is 1 file.
+  // a.htm is 1 file.
+  CheckDownloadHistory(url, full_file_name, 1, DownloadItem::COMPLETE);
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_FALSE(file_util::PathExists(dir));
@@ -310,8 +385,12 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnlyCancel) {
   // Currently it's ignored.
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
+  // -1 to disable number of files check; we don't update after cancel, and
+  // we don't know when the single file completed in relationship to
+  // the cancel.
+  CheckDownloadHistory(url, full_file_name, -1, DownloadItem::CANCELLED);
+
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
-  CheckDownloadHistory(url, full_file_name, 1);  // a.htm is 1 file.
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_FALSE(file_util::PathExists(dir));
@@ -358,7 +437,9 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveViewSourceHTMLOnly) {
   EXPECT_EQ(actual_page_url, WaitForSavePackageToFinish());
 
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
-  CheckDownloadHistory(actual_page_url, full_file_name, 1);  // a.htm is 1 file.
+  // a.htm is 1 file.
+  CheckDownloadHistory(actual_page_url, full_file_name, 1,
+                       DownloadItem::COMPLETE);
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_FALSE(file_util::PathExists(dir));
@@ -378,7 +459,8 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveCompleteHTML) {
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
-  CheckDownloadHistory(url, full_file_name, 3);  // b.htm is 3 files.
+  // b.htm is 3 files.
+  CheckDownloadHistory(url, full_file_name, 3, DownloadItem::COMPLETE);
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_TRUE(file_util::PathExists(dir));
@@ -411,7 +493,8 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, FileNameFromPageTitle) {
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
-  CheckDownloadHistory(url, full_file_name, 3);  // b.htm is 3 files.
+  // b.htm is 3 files.
+  CheckDownloadHistory(url, full_file_name, 3, DownloadItem::COMPLETE);
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   EXPECT_TRUE(file_util::PathExists(dir));
@@ -437,14 +520,16 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, RemoveFromList) {
   EXPECT_EQ(url, WaitForSavePackageToFinish());
 
   EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
-  CheckDownloadHistory(url, full_file_name, 1);  // a.htm is 1 file.
+  // a.htm is 1 file.
+  CheckDownloadHistory(url, full_file_name, 1, DownloadItem::COMPLETE);
 
   EXPECT_EQ(GetDownloadManager()->RemoveAllDownloads(), 1);
 
   // Should not be in history.
   QueryDownloadHistory();
   EXPECT_EQ(std::find_if(history_entries_.begin(), history_entries_.end(),
-      DownloadPersistentStoreInfoMatch(url, full_file_name, 1)),
+                         DownloadPersistentStoreInfoMatch(
+                             url, full_file_name, 1, DownloadItem::COMPLETE)),
       history_entries_.end());
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
@@ -518,7 +603,7 @@ IN_PROC_BROWSER_TEST_F(SavePageAsMHTMLBrowserTest, SavePageAsMHTML) {
         content::NotificationService::AllSources());
   chrome::SavePage(browser());
   observer.Wait();
-  CheckDownloadHistory(url, full_file_name, -1);
+  CheckDownloadHistory(url, full_file_name, -1, DownloadItem::COMPLETE);
 
   EXPECT_TRUE(file_util::PathExists(full_file_name));
   int64 actual_file_size = -1;