Reduced the lifetime of DownloadCreateInfo.

chrome/browser/download/download_manager_unittest.cc

 // 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.
 
 #include <string>
 #include <set>
 
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "build/build_config.h"
+#include "chrome/browser/download/download_create_info.h"
 #include "chrome/browser/download/download_file.h"
 #include "chrome/browser/download/download_file_manager.h"
 #include "chrome/browser/download/download_item.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_prefs.h"
 #include "chrome/browser/download/download_status_updater.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/download/mock_download_manager.h"
-#include "chrome/browser/history/download_create_info.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/testing_profile.h"
 #include "content/browser/browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gmock_mutant.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 class DownloadManagerTest : public testing::Test {
  public:
@@ skipping 22 matching lines @@
   }
 
   void OnAllDataSaved(int32 download_id, int64 size, const std::string& hash) {
     download_manager_->OnAllDataSaved(download_id, size, hash);
   }
 
   void FileSelected(const FilePath& path, int index, void* params) {
     download_manager_->FileSelected(path, index, params);
   }
 
-  void AttachDownloadItem(DownloadCreateInfo* info) {
-    download_manager_->AttachDownloadItem(info);
+  void ContinueDownloadWithPath(DownloadItem* download, const FilePath& path) {
+    download_manager_->ContinueDownloadWithPath(download, path);
   }
 
   void OnDownloadError(int32 download_id, int64 size, int os_error) {
     download_manager_->OnDownloadError(download_id, size, os_error);
   }
 
   // Get the download item with ID |id|.
   DownloadItem* GetActiveDownloadItem(int32 id) {
     if (ContainsKey(download_manager_->active_downloads_, id))
       return download_manager_->active_downloads_[id];
@@ skipping 229 matching lines @@
     DownloadCreateInfo* info = new DownloadCreateInfo;
     info->download_id = static_cast<int>(i);
     info->prompt_user_for_save_location = kStartDownloadCases[i].save_as;
     info->url_chain.push_back(GURL(kStartDownloadCases[i].url));
     info->mime_type = kStartDownloadCases[i].mime_type;
     download_manager_->CreateDownloadItem(info);
 
     DownloadFile* download_file(new DownloadFile(info, download_manager_));
     AddDownloadToFileManager(info->download_id, download_file);
     download_file->Initialize(false);
-    download_manager_->StartDownload(info);
+    download_manager_->StartDownload(info->download_id);
     message_loop_.RunAllPending();
 
-    // NOTE: At this point, |AttachDownloadItem| will have been run if we don't
-    // need to prompt the user, so |info| could have been destructed.
+    // NOTE: At this point, |ContinueDownloadWithPath| will have been run if
+    // we don't need to prompt the user, so |info| could have been destructed.
     // This means that we can't check any of its values.
     // However, SelectFileObserver will have recorded any attempt to open the
     // select file dialog.
     EXPECT_EQ(kStartDownloadCases[i].expected_save_as,
               observer.ShowedFileDialogForId(i));
 
     // If the Save As dialog pops up, it never reached
-    // DownloadManager::AttachDownloadItem(), and never deleted info or
+    // DownloadManager::ContinueDownloadWithPath(), and never deleted info or
     // completed.  This cleans up info.
     // Note that DownloadManager::FileSelectionCanceled() is never called.
     if (observer.ShowedFileDialogForId(i)) {
       delete info;
     }
   }
 }
 
 TEST_F(DownloadManagerTest, DownloadRenameTest) {
   using ::testing::_;
   using ::testing::CreateFunctor;
   using ::testing::Invoke;
   using ::testing::Return;
 
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(kDownloadRenameCases); ++i) {
     // |info| will be destroyed in download_manager_.
     DownloadCreateInfo* info(new DownloadCreateInfo);
     info->download_id = static_cast<int>(i);
     info->prompt_user_for_save_location = false;
     info->url_chain.push_back(GURL());
     info->is_dangerous_file = kDownloadRenameCases[i].is_dangerous_file;
     info->is_dangerous_url = kDownloadRenameCases[i].is_dangerous_url;
-    FilePath new_path(kDownloadRenameCases[i].suggested_path);
+    const FilePath new_path(kDownloadRenameCases[i].suggested_path);
 
     MockDownloadFile* download_file(
         new MockDownloadFile(info, download_manager_));
     AddDownloadToFileManager(info->download_id, download_file);
 
     // |download_file| is owned by DownloadFileManager.
     ::testing::Mock::AllowLeak(download_file);
     EXPECT_CALL(*download_file, Destructed()).Times(1);
 
     if (kDownloadRenameCases[i].expected_rename_count == 1) {
       EXPECT_CALL(*download_file, Rename(new_path)).WillOnce(Return(true));
     } else {
       ASSERT_EQ(2, kDownloadRenameCases[i].expected_rename_count);
       FilePath crdownload(download_util::GetCrDownloadPath(new_path));
       EXPECT_CALL(*download_file, Rename(_))
           .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor(
               download_file, &MockDownloadFile::TestMultipleRename,
               1, crdownload))))
           .WillOnce(testing::WithArgs<0>(Invoke(CreateFunctor(
               download_file, &MockDownloadFile::TestMultipleRename,
               2, new_path))));
     }
     download_manager_->CreateDownloadItem(info);
 
+    int32* id_ptr = new int32;
+    *id_ptr = i;  // Deleted in FileSelected().
     if (kDownloadRenameCases[i].finish_before_rename) {
       OnAllDataSaved(i, 1024, std::string("fake_hash"));
       message_loop_.RunAllPending();
-      FileSelected(new_path, i, info);
+      FileSelected(new_path, i, id_ptr);
     } else {
-      FileSelected(new_path, i, info);
+      FileSelected(new_path, i, id_ptr);
       message_loop_.RunAllPending();
       OnAllDataSaved(i, 1024, std::string("fake_hash"));
     }
 
     message_loop_.RunAllPending();
     EXPECT_TRUE(VerifySafetyState(kDownloadRenameCases[i].is_dangerous_file,
                                   kDownloadRenameCases[i].is_dangerous_url,
                                   i));
   }
 }
@@ skipping 27 matching lines @@
   download_manager_->CreateDownloadItem(info);
 
   DownloadItem* download = GetActiveDownloadItem(0);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
   scoped_ptr<ItemObserver> observer(new ItemObserver(download));
 
   download_file->AppendDataToFile(kTestData, kTestDataLen);
 
-  info->path = new_path;
-  AttachDownloadItem(info);
+  ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
   EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
 
   OnDownloadError(0, 1024, -6);
   message_loop_.RunAllPending();
 
   EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
   EXPECT_EQ(DownloadItem::INTERRUPTED, download->state());
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_TRUE(observer->hit_state(DownloadItem::INTERRUPTED));
@@ skipping 42 matching lines @@
   EXPECT_CALL(*download_file, Rename(cr_path)).WillOnce(Return(true));
 
   download_manager_->CreateDownloadItem(info);
 
   DownloadItem* download = GetActiveDownloadItem(0);
   ASSERT_TRUE(download != NULL);
 
   EXPECT_EQ(DownloadItem::IN_PROGRESS, download->state());
   scoped_ptr<ItemObserver> observer(new ItemObserver(download));
 
-  info->path = new_path;
-  AttachDownloadItem(info);
+  ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
   EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
 
   download_file->AppendDataToFile(kTestData, kTestDataLen);
 
   download->Cancel(false);
   message_loop_.RunAllPending();
 
   EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
@@ skipping 57 matching lines @@
   // name has been chosen, so we need to initialize the download file
   // properly.
   DownloadFile* download_file(
       new DownloadFile(info, download_manager_));
   download_file->Rename(cr_path);
   // This creates the .crdownload version of the file.
   download_file->Initialize(false);
   // |download_file| is owned by DownloadFileManager.
   AddDownloadToFileManager(info->download_id, download_file);
 
-  info->path = new_path;
-  AttachDownloadItem(info);
+  ContinueDownloadWithPath(download, new_path);
   message_loop_.RunAllPending();
   EXPECT_TRUE(GetActiveDownloadItem(0) != NULL);
 
   download_file->AppendDataToFile(kTestData, kTestDataLen);
 
   // Finish the download.
   OnAllDataSaved(0, kTestDataLen, "");
   message_loop_.RunAllPending();
 
   // Download is complete.
   EXPECT_TRUE(GetActiveDownloadItem(0) == NULL);
   EXPECT_TRUE(observer->hit_state(DownloadItem::IN_PROGRESS));
   EXPECT_FALSE(observer->hit_state(DownloadItem::CANCELLED));
   EXPECT_FALSE(observer->hit_state(DownloadItem::INTERRUPTED));
   EXPECT_TRUE(observer->hit_state(DownloadItem::COMPLETE));
   EXPECT_FALSE(observer->hit_state(DownloadItem::REMOVING));
   EXPECT_TRUE(observer->was_updated());
   EXPECT_FALSE(observer->was_opened());
   EXPECT_EQ(DownloadItem::COMPLETE, download->state());
 
   EXPECT_TRUE(file_util::PathExists(new_path));
   EXPECT_FALSE(file_util::PathExists(cr_path));
   EXPECT_FALSE(file_util::PathExists(unique_new_path));
   std::string file_contents;
   EXPECT_TRUE(file_util::ReadFileToString(new_path, &file_contents));
   EXPECT_EQ(std::string(kTestData), file_contents);
 }