Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(5875)

Unified Diff: chrome/browser/download/save_page_browsertest.cc

Issue 10915180: Make DownloadHistory observe manager, items (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: . Created 8 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 bbf65f77c2940b27a49c788b5638a4ae0699ffc0..c2d359a9da792b07b198045162f47b7d8cbb421c 100644
--- a/chrome/browser/download/save_page_browsertest.cc
+++ b/chrome/browser/download/save_page_browsertest.cc
@@ -17,6 +17,8 @@
#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/history/download_persistent_store_info.h"
+#include "chrome/browser/history/history_service_factory.h"
#include "chrome/browser/net/url_request_mock_util.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/profiles/profile.h"
@@ -32,7 +34,6 @@
#include "chrome/test/base/ui_test_utils.h"
#include "content/public/browser/download_item.h"
#include "content/public/browser/download_manager.h"
-#include "content/public/browser/download_persistent_store_info.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/web_contents.h"
@@ -50,11 +51,49 @@ using content::BrowserContext;
using content::BrowserThread;
using content::DownloadItem;
using content::DownloadManager;
-using content::DownloadPersistentStoreInfo;
using content::WebContents;
namespace {
+class DownloadPersistedObserver : public DownloadItem::Observer {
+ public:
+ DownloadPersistedObserver(DownloadItem* item)
+ : item_(item) {
+ item->AddObserver(this);
+ }
+
+ ~DownloadPersistedObserver() {
+ if (item_)
+ item_->RemoveObserver(this);
+ }
+
+ void WaitForPersisted() {
+ if (persisted_)
+ return;
+ waiting_ = true;
+ content::RunMessageLoop();
+ waiting_ = false;
+ }
+
+ virtual void OnDownloadDestroyed(DownloadItem* item) {
+ item_->RemoveObserver(this);
+ item_ = NULL;
+ }
+
+ virtual void OnDownloadUpdated(DownloadItem* item) {
+ CHECK_EQ(item_, item);
+ persisted_ = true;
+ if (waiting_)
+ MessageLoopForUI::current()->Quit();
+ }
+
+ private:
+ DownloadItem* item_;
+ bool waiting_;
+ bool persisted_;
+ DISALLOW_COPY_AND_ASSIGN(DownloadPersistedObserver);
+};
+
static const FilePath::CharType* kTestDir = FILE_PATH_LITERAL("save_page");
static const char* kAppendedExtension =
@@ -99,7 +138,6 @@ class DownloadItemCreatedObserver : public DownloadManager::Observer {
}
private:
-
// DownloadManager::Observer
void OnDownloadCreated(DownloadManager* manager, DownloadItem* item) {
DCHECK_EQ(manager, manager_);
@@ -184,12 +222,12 @@ class SavePageBrowserTest : public InProcessBrowserTest {
void QueryDownloadHistory() {
// Query the history system.
- ChromeDownloadManagerDelegate* delegate =
- static_cast<ChromeDownloadManagerDelegate*>(
- GetDownloadManager()->GetDelegate());
- delegate->download_history()->Load(
- base::Bind(&SavePageBrowserTest::OnQueryDownloadEntriesComplete,
- base::Unretained(this)));
+ HistoryService* hs = HistoryServiceFactory::GetForProfile(
+ browser()->profile(), Profile::EXPLICIT_ACCESS);
+ hs->QueryDownloads(base::Bind(
+ &SavePageBrowserTest::OnQueryDownloadEntriesComplete,
+ base::Unretained(this),
+ MessageLoopForUI::current()->QuitClosure()));
// Run message loop until a quit message is sent from
// OnQueryDownloadEntriesComplete().
@@ -197,66 +235,42 @@ class SavePageBrowserTest : public InProcessBrowserTest {
}
void OnQueryDownloadEntriesComplete(
- std::vector<DownloadPersistentStoreInfo>* entries) {
- history_entries_ = *entries;
-
- // Indicate thet we have received the history and can continue.
- MessageLoopForUI::current()->Quit();
+ const base::Closure& done_on_ui,
+ scoped_ptr<std::vector<DownloadPersistentStoreInfo> > entries) {
+ history_entries_ = entries.Pass();
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, done_on_ui);
}
- struct DownloadPersistentStoreInfoMatch
- : public std::unary_function<DownloadPersistentStoreInfo, bool> {
-
- DownloadPersistentStoreInfoMatch(const GURL& url,
- const FilePath& path,
- int64 num_files)
- : url_(url),
- path_(path),
- num_files_(num_files) {
- }
-
- bool operator() (const DownloadPersistentStoreInfo& info) const {
- return info.url == url_ &&
- info.path == path_ &&
- // For non-MHTML save packages, received_bytes is actually the
- // number of files.
- ((num_files_ < 0) ||
- (info.received_bytes == num_files_)) &&
- info.total_bytes == 0 &&
- info.state == DownloadItem::COMPLETE;
- }
-
- GURL url_;
- FilePath path_;
- int64 num_files_;
- };
-
void CheckDownloadHistory(const GURL& url,
const FilePath& path,
- int64 num_files) {
+ int64 num_files,
+ bool expect_found) {
QueryDownloadHistory();
-
- std::vector<DownloadPersistentStoreInfo>::iterator found =
- std::find_if(history_entries_.begin(), history_entries_.end(),
- DownloadPersistentStoreInfoMatch(url, path, num_files));
-
- if (found == history_entries_.end()) {
- LOG(ERROR) << "Missing url=" << url.spec()
- << " path=" << path.value()
- << " received=" << num_files;
- for (size_t index = 0; index < history_entries_.size(); ++index) {
- LOG(ERROR) << "History@" << index << ": url="
- << history_entries_[index].url.spec()
- << " path=" << history_entries_[index].path.value()
- << " received=" << history_entries_[index].received_bytes
- << " total=" << history_entries_[index].total_bytes
- << " state=" << history_entries_[index].state;
+ bool found = false;
+ for (std::vector<DownloadPersistentStoreInfo>::iterator it =
+ history_entries_->begin();
+ it != history_entries_->end(); ++it) {
+ if (it->url != url) {
+ LOG(WARNING) << it->url.spec() << " != " << url.spec();
+ } else if (it->path != path) {
+ LOG(WARNING) << it->path.value() << " != " << path.value();
+ } else if ((num_files >= 0) &&
+ (it->received_bytes != num_files)) {
+ LOG(WARNING) << it->received_bytes << " != " << num_files;
+ } else if ((num_files >= 0) &&
+ (it->total_bytes != num_files)) {
+ LOG(WARNING) << it->total_bytes << " != " << num_files;
+ } else if (it->state != DownloadItem::COMPLETE) {
+ LOG(WARNING) << it->state << " != " << DownloadItem::COMPLETE;
+ } else {
+ found = true;
+ break;
}
- EXPECT_TRUE(false);
}
+ EXPECT_EQ(found, expect_found);
}
- std::vector<DownloadPersistentStoreInfo> history_entries_;
+ scoped_ptr<std::vector<DownloadPersistentStoreInfo> > history_entries_;
// Path to directory containing test data.
FilePath test_dir_;
@@ -282,7 +296,7 @@ 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.
+ CheckDownloadHistory(url, full_file_name, 1, true); // a.htm is 1 file.
EXPECT_TRUE(file_util::PathExists(full_file_name));
EXPECT_FALSE(file_util::PathExists(dir));
@@ -311,7 +325,7 @@ IN_PROC_BROWSER_TEST_F(SavePageBrowserTest, SaveHTMLOnlyCancel) {
EXPECT_EQ(url, WaitForSavePackageToFinish());
EXPECT_TRUE(browser()->window()->IsDownloadShelfVisible());
- CheckDownloadHistory(url, full_file_name, 1); // a.htm is 1 file.
+ CheckDownloadHistory(url, full_file_name, 1, true); // a.htm is 1 file.
EXPECT_TRUE(file_util::PathExists(full_file_name));
EXPECT_FALSE(file_util::PathExists(dir));
@@ -360,7 +374,7 @@ 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.
+ CheckDownloadHistory(actual_page_url, full_file_name, 1, true);
EXPECT_TRUE(file_util::PathExists(full_file_name));
EXPECT_FALSE(file_util::PathExists(dir));
@@ -380,7 +394,7 @@ 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.
+ CheckDownloadHistory(url, full_file_name, 3, true); // b.htm is 3 files.
EXPECT_TRUE(file_util::PathExists(full_file_name));
EXPECT_TRUE(file_util::PathExists(dir));
@@ -413,7 +427,7 @@ 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.
+ CheckDownloadHistory(url, full_file_name, 3, true); // b.htm is 3 files.
EXPECT_TRUE(file_util::PathExists(full_file_name));
EXPECT_TRUE(file_util::PathExists(dir));
@@ -439,15 +453,13 @@ 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.
+ CheckDownloadHistory(url, full_file_name, 1, true); // a.htm is 1 file.
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)),
- history_entries_.end());
+ CheckDownloadHistory(url, full_file_name, 1, false);
EXPECT_TRUE(file_util::PathExists(full_file_name));
EXPECT_FALSE(file_util::PathExists(dir));
@@ -520,7 +532,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, true);
EXPECT_TRUE(file_util::PathExists(full_file_name));
int64 actual_file_size = -1;

Powered by Google App Engine
This is Rietveld 408576698