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

Unified Diff: chrome/browser/download/download_history_unittest.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/download_history_unittest.cc
diff --git a/chrome/browser/download/download_history_unittest.cc b/chrome/browser/download/download_history_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..ca697bfbac3f63ad5a700efc7a6d8b6c9f313ade
--- /dev/null
+++ b/chrome/browser/download/download_history_unittest.cc
@@ -0,0 +1,512 @@
+// Copyright (c) 2012 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 "base/rand_util.h"
+#include "chrome/browser/download/download_history.h"
+#include "chrome/browser/history/download_persistent_store_info.h"
+#include "chrome/browser/history/history.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/test/mock_download_item.h"
+#include "content/public/test/mock_download_manager.h"
+#include "content/public/test/test_utils.h"
+
+using content::BrowserThread;
+using testing::DoAll;
+using testing::Return;
+using testing::ReturnRef;
+using testing::_;
+
+namespace {
+
+bool InfoEqual(const DownloadPersistentStoreInfo& left,
+ const DownloadPersistentStoreInfo& right) {
+ if (left.path != right.path) {
+ LOG(ERROR) << left.path.value() << " != " << right.path.value();
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 Wow, this is going to produce a lot of output, and
benjhayden 2012/09/21 20:45:46 Done.
+ return false;
+ } else if (left.url != right.url) {
+ LOG(ERROR) << left.url.spec() << " != " << right.url.spec();
+ return false;
+ } else if (left.referrer_url != right.referrer_url) {
+ LOG(ERROR) << left.referrer_url.spec() << " != "
+ << right.referrer_url.spec();
+ return false;
+ } else if (left.start_time != right.start_time) {
+ LOG(ERROR) << left.start_time.ToTimeT() << " != "
+ << right.start_time.ToTimeT();
+ return false;
+ } else if (left.end_time != right.end_time) {
+ LOG(ERROR) << left.end_time.ToTimeT() << " != " << right.end_time.ToTimeT();
+ return false;
+ } else if (left.received_bytes != right.received_bytes) {
+ LOG(ERROR) << left.received_bytes << " != " << right.received_bytes;
+ return false;
+ } else if (left.total_bytes != right.total_bytes) {
+ LOG(ERROR) << left.total_bytes << " != " << right.total_bytes;
+ return false;
+ } else if (left.state != right.state) {
+ LOG(ERROR) << left.state << " != " << right.state;
+ return false;
+ } else if (left.db_handle != right.db_handle) {
+ LOG(ERROR) << left.db_handle << " != " << right.db_handle;
+ return false;
+ } else if (left.opened != right.opened) {
+ LOG(ERROR) << left.opened << " != " << right.opened;
+ return false;
+ }
+ return true;
+}
+
+typedef std::set<int64> HandleSet;
+typedef std::vector<DownloadPersistentStoreInfo> InfoVector;
+
+class FakeHistoryService
+ : public HistoryService{
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 nit, suggestion: I think it's more customary to ha
benjhayden 2012/09/21 20:45:46 Done.
+ public:
+ FakeHistoryService()
+ : slow_create_download_(false),
+ handle_counter_(0) {
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 nit: Curlies on same line is allowed but not requi
benjhayden 2012/09/21 20:45:46 When the constructor spans multiple lines, I like
+ }
+
+ virtual void QueryDownloads(
+ const HistoryService::DownloadQueryCallback& callback) OVERRIDE {
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+ base::Bind(&FakeHistoryService::QueryDownloadsDone,
+ base::Unretained(this), callback));
+ }
+
+ void QueryDownloadsDone(
+ const HistoryService::DownloadQueryCallback& callback) {
+ CHECK(expect_query_downloads_.get());
+ callback.Run(expect_query_downloads_.Pass());
+ }
+
+ virtual void GetVisibleVisitCountToHostSimple(
+ const GURL& referrer_url,
+ const HistoryService::GetVisibleVisitCountToHostSimpleCallback&
+ callback) OVERRIDE {
+ NOTIMPLEMENTED();
+ }
+
+ void set_slow_create_download(bool slow) { slow_create_download_ = slow; }
+
+ virtual void CreateDownload(
+ const DownloadPersistentStoreInfo& info,
+ const HistoryService::DownloadCreateCallback& callback) OVERRIDE {
+ create_download_info_ = info;
+ create_download_callback_ = base::Bind(callback, handle_counter_++);
+ if (!slow_create_download_) {
+ FinishCreateDownload();
+ }
+ }
+
+ void FinishCreateDownload() {
+ create_download_callback_.Run();
+ create_download_callback_.Reset();
+ }
+
+ virtual void UpdateDownload(
+ const DownloadPersistentStoreInfo& info) OVERRIDE {
+ update_download_ = info;
+ }
+
+ virtual void RemoveDownloads(
+ const HandleSet& handles) OVERRIDE {
+ for (HandleSet::const_iterator it = handles.begin();
+ it != handles.end(); ++it) {
+ remove_downloads_.insert(*it);
+ }
+ }
+
+ void ExpectQueryDownloads(scoped_ptr<InfoVector> infos) {
+ expect_query_downloads_ = infos.Pass();
+ }
+
+ void ExpectQueryDownloadsDone() {
+ CHECK(NULL == expect_query_downloads_.get());
+ }
+
+ void ExpectCreateDownload(
+ const DownloadPersistentStoreInfo& info) {
+ content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+ CHECK(InfoEqual(info, create_download_info_));
+ create_download_info_ = DownloadPersistentStoreInfo();
+ }
+
+ void ExpectNoCreateDownload() {
+ content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+ CHECK(InfoEqual(DownloadPersistentStoreInfo(), create_download_info_));
+ }
+
+ void ExpectUpdateDownload(const DownloadPersistentStoreInfo& info) {
+ content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+ CHECK(InfoEqual(update_download_, info));
+ update_download_ = DownloadPersistentStoreInfo();
+ }
+
+ void ExpectNoUpdateDownload() {
+ content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+ CHECK(InfoEqual(DownloadPersistentStoreInfo(), update_download_));
+ }
+ void ExpectNoRemoveDownloads() {
+ content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+ CHECK_EQ(0, static_cast<int>(remove_downloads_.size()));
+ }
+
+ void ExpectRemoveDownloads(const HandleSet& handles) {
+ content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+ HandleSet difference;
+ std::insert_iterator<HandleSet> insert_it(
+ difference, difference.begin());
+ std::set_difference(handles.begin(), handles.end(),
+ remove_downloads_.begin(),
+ remove_downloads_.end(),
+ insert_it);
+ CHECK(difference.empty());
+ remove_downloads_.clear();
+ }
+
+ private:
+ virtual ~FakeHistoryService() {}
+
+ bool slow_create_download_;
+ base::Closure create_download_callback_;
+ int handle_counter_;
+ DownloadPersistentStoreInfo update_download_;
+ scoped_ptr<InfoVector> expect_query_downloads_;
+ HandleSet remove_downloads_;
+ DownloadPersistentStoreInfo create_download_info_;
+
+ DISALLOW_COPY_AND_ASSIGN(FakeHistoryService);
+};
+
+class DownloadHistoryTest : public InProcessBrowserTest {
+ public:
+ DownloadHistoryTest()
+ : manager_(new content::MockDownloadManager()),
+ download_history_(NULL) {
+ }
+ virtual ~DownloadHistoryTest() {}
+
+ protected:
+ virtual void CleanUpOnMainThread() OVERRIDE {
+ download_history_.reset();
+ }
+
+ DownloadHistory* download_history() { return download_history_.get(); }
+
+ content::MockDownloadManager& manager() { return *manager_.get(); }
+ content::MockDownloadItem& item() { return item_; }
+
+ void ExpectQueryDownloads(scoped_ptr<InfoVector> infos) {
+ CHECK(infos.get());
+ EXPECT_CALL(manager(), AddObserver(_));
+ EXPECT_CALL(manager(), RemoveObserver(_));
+ if (infos->size() != 0) {
+ EXPECT_EQ(1, static_cast<int>(infos->size()));
+ EXPECT_CALL(manager(), CreateDownloadItem(
+ infos->at(0).path,
+ infos->at(0).url,
+ infos->at(0).referrer_url,
+ infos->at(0).start_time,
+ infos->at(0).end_time,
+ infos->at(0).received_bytes,
+ infos->at(0).total_bytes,
+ infos->at(0).state,
+ infos->at(0).opened))
+ .WillOnce(DoAll(
+ InvokeWithoutArgs(
+ this, &DownloadHistoryTest::CallOnDownloadCreated),
+ Return(&item())));
+ }
+ EXPECT_CALL(manager(), CheckForHistoryFilesRemoval());
+ history_ = new FakeHistoryService();
+ history_->ExpectQueryDownloads(infos.Pass());
+ download_history_.reset(new DownloadHistory(&manager(), history_.get()));
+ content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+ history_->ExpectQueryDownloadsDone();
+ }
+
+ void CallOnDownloadCreated() {
+ download_history_->OnDownloadCreated(&manager(), &item());
+ }
+
+ void set_slow_create_download(bool slow) {
+ history_->set_slow_create_download(slow);
+ }
+
+ void FinishCreateDownload() {
+ history_->FinishCreateDownload();
+ }
+
+ void ExpectCreateDownload(
+ const DownloadPersistentStoreInfo& info) {
+ history_->ExpectCreateDownload(info);
+ }
+
+ void ExpectNoCreateDownload() {
+ history_->ExpectNoCreateDownload();
+ }
+
+ void ExpectUpdateDownload(const DownloadPersistentStoreInfo& info) {
+ history_->ExpectUpdateDownload(info);
+ }
+
+ void ExpectNoUpdateDownload() {
+ history_->ExpectNoUpdateDownload();
+ }
+
+ void ExpectNoRemoveDownloads() {
+ history_->ExpectNoRemoveDownloads();
+ }
+
+ void ExpectRemoveDownloads(const HandleSet& handles) {
+ history_->ExpectRemoveDownloads(handles);
+ }
+
+ void InitItem(
+ int32 id,
+ const FilePath& path,
+ const GURL& url,
+ const GURL& referrer,
+ const base::Time& start_time,
+ const base::Time& end_time,
+ int64 received_bytes,
+ int64 total_bytes,
+ content::DownloadItem::DownloadState state,
+ int64 db_handle,
+ bool opened,
+ DownloadPersistentStoreInfo* info) {
+ info->path = path;
+ info->url = url;
+ info->referrer_url = referrer;
+ info->start_time = start_time;
+ info->end_time = end_time;
+ info->received_bytes = received_bytes;
+ info->total_bytes = total_bytes;
+ info->state = state;
+ info->db_handle = db_handle;
+ info->opened = opened;
+ EXPECT_CALL(item(), GetId()).WillRepeatedly(Return(id));
+ EXPECT_CALL(item(), GetFullPath()).WillRepeatedly(ReturnRef(path));
+ EXPECT_CALL(item(), GetURL()).WillRepeatedly(ReturnRef(url));
+ EXPECT_CALL(item(), GetReferrerUrl()).WillRepeatedly(ReturnRef(referrer));
+ EXPECT_CALL(item(), GetStartTime()).WillRepeatedly(Return(start_time));
+ EXPECT_CALL(item(), GetEndTime()).WillRepeatedly(Return(end_time));
+ EXPECT_CALL(item(), GetReceivedBytes())
+ .WillRepeatedly(Return(received_bytes));
+ EXPECT_CALL(item(), GetTotalBytes()).WillRepeatedly(Return(total_bytes));
+ EXPECT_CALL(item(), GetState()).WillRepeatedly(Return(state));
+ EXPECT_CALL(item(), GetOpened()).WillRepeatedly(Return(opened));
+ EXPECT_CALL(item(), GetTargetDisposition()).WillRepeatedly(Return(
+ content::DownloadItem::TARGET_DISPOSITION_OVERWRITE));
+ EXPECT_CALL(manager(), GetDownload(id))
+ .WillRepeatedly(Return(&item()));
+ EXPECT_CALL(item(), AddObserver(_));
+ EXPECT_CALL(item(), RemoveObserver(_));
+ }
+
+ private:
+ testing::NiceMock<content::MockDownloadItem> item_;
+ scoped_refptr<FakeHistoryService> history_;
+ scoped_refptr<content::MockDownloadManager> manager_;
+ scoped_ptr<DownloadHistory> download_history_;
+
+ DISALLOW_COPY_AND_ASSIGN(DownloadHistoryTest);
+};
+
+} // anonymous namespace
+
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest,
+ DownloadHistoryTest_Load) {
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 nit, suggestion: It looks really weird to me to ha
benjhayden 2012/09/21 20:45:46 Done.
+ // Load a download from history, create the item, OnDownloadCreated,
+ // OnDownloadUpdated, OnDownloadRemoved, OnDownloadDestroyed.
+ DownloadPersistentStoreInfo info;
+ InitItem(base::RandInt(0, 1 << 20),
+ FilePath(FILE_PATH_LITERAL("/foo/bar.pdf")),
+ GURL("http://example.com/bar.pdf"),
+ GURL("http://example.com/referrer.html"),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(10)),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(1)),
+ 100,
+ 100,
+ content::DownloadItem::COMPLETE,
+ base::RandInt(0, 1 << 20),
+ false,
+ &info);
+ {
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 Why the extra curlies?
benjhayden 2012/09/21 20:45:46 To delineate the lifetime of infos.
+ scoped_ptr<InfoVector> infos(new InfoVector());
+ infos->push_back(info);
+ ExpectQueryDownloads(infos.Pass());
+ ExpectNoCreateDownload();
+ }
+
+ // Pretend that something changed on the item.
+ EXPECT_CALL(item(), GetOpened()).WillRepeatedly(Return(true));
+ download_history()->OnDownloadUpdated(&item());
+ info.opened = true;
+ ExpectUpdateDownload(info);
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 For what it's worth, I find it confusing to read t
benjhayden 2012/09/21 20:45:46 Done.
+
+ // Pretend that the user removed the item.
+ HandleSet handles;
+ handles.insert(info.db_handle);
+ download_history()->OnDownloadRemoved(&item());
+ ExpectRemoveDownloads(handles);
+
+ // Pretend that the browser is closing.
+ download_history()->ManagerGoingDown(&manager());
+ download_history()->OnDownloadDestroyed(&item());
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest,
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 Intro line or two before tests describing what you
benjhayden 2012/09/21 20:45:46 Done.
+ DownloadHistoryTest_Create) {
+ // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
+ // OnDownloadRemoved, OnDownloadDestroyed.
+ ExpectQueryDownloads(scoped_ptr<InfoVector>(new InfoVector()));
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 Huh; I wouldn't have thought this would compile (I
benjhayden 2012/09/21 20:45:46 I think this is what compilers call a "temporary".
+
+ // Note that db_handle must be -1 at first because it isn't in the db yet.
+ DownloadPersistentStoreInfo info;
+ InitItem(base::RandInt(0, 1 << 20),
+ FilePath(FILE_PATH_LITERAL("/foo/bar.pdf")),
+ GURL("http://example.com/bar.pdf"),
+ GURL("http://example.com/referrer.html"),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(10)),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(1)),
+ 100,
+ 100,
+ content::DownloadItem::COMPLETE,
+ -1,
+ false,
+ &info);
+
+ // Pretend the manager just created |item|.
+ download_history()->OnDownloadCreated(&manager(), &item());
+ // CreateDownload() always gets db_handle=-1.
+ ExpectCreateDownload(info);
+ info.db_handle = 0;
+
+ // Pretend that something changed on the item.
+ EXPECT_CALL(item(), GetOpened()).WillRepeatedly(Return(true));
+ download_history()->OnDownloadUpdated(&item());
+ info.opened = true;
+ ExpectUpdateDownload(info);
+
+ // Pretend that the user removed the item.
+ HandleSet handles;
+ handles.insert(info.db_handle);
+ download_history()->OnDownloadRemoved(&item());
+ ExpectRemoveDownloads(handles);
+
+ // Pretend that the browser is closing.
+ download_history()->ManagerGoingDown(&manager());
+ download_history()->OnDownloadDestroyed(&item());
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest,
+ DownloadHistoryTest_Temporary) {
+ // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
+ // OnDownloadRemoved, OnDownloadDestroyed.
+ ExpectQueryDownloads(scoped_ptr<InfoVector>(new InfoVector()));
+
+ // Note that db_handle must be -1 at first because it isn't in the db yet.
+ DownloadPersistentStoreInfo info;
+ InitItem(base::RandInt(0, 1 << 20),
+ FilePath(FILE_PATH_LITERAL("/foo/bar.pdf")),
+ GURL("http://example.com/bar.pdf"),
+ GURL("http://example.com/referrer.html"),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(10)),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(1)),
+ 100,
+ 100,
+ content::DownloadItem::COMPLETE,
+ -1,
+ false,
+ &info);
+
+ // Pretend the manager just created |item|.
+ download_history()->OnDownloadCreated(&manager(), &item());
+ // CreateDownload() always gets db_handle=-1.
+ ExpectCreateDownload(info);
+ info.db_handle = 0;
+
+ // Pretend the item was marked temporary. DownloadHistory should remove it
+ // from history and start ignoring it.
+ EXPECT_CALL(item(), IsTemporary()).WillRepeatedly(Return(true));
+ download_history()->OnDownloadUpdated(&item());
+ HandleSet handles;
+ handles.insert(info.db_handle);
+ ExpectRemoveDownloads(handles);
+
+ // Change something that would make DownloadHistory call UpdateDownload if the
+ // item weren't temporary.
+ EXPECT_CALL(item(), GetReceivedBytes()).WillRepeatedly(Return(4200));
+ download_history()->OnDownloadUpdated(&item());
+ ExpectNoUpdateDownload();
+
+ // Changing a temporary item back to a non-temporary item should make
+ // DownloadHistory call CreateDownload.
+ EXPECT_CALL(item(), IsTemporary()).WillRepeatedly(Return(false));
+ download_history()->OnDownloadUpdated(&item());
+ info.received_bytes = 4200;
+ info.db_handle = -1;
+ // CreateDownload() always gets db_handle=-1.
+ ExpectCreateDownload(info);
+ info.db_handle = 1;
+
+ EXPECT_CALL(item(), GetReceivedBytes()).WillRepeatedly(Return(100));
+ download_history()->OnDownloadUpdated(&item());
+ info.received_bytes = 100;
+ ExpectUpdateDownload(info);
+
+ // Pretend that the browser is closing.
+ download_history()->ManagerGoingDown(&manager());
+ download_history()->OnDownloadDestroyed(&item());
+}
+
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest,
+ DownloadHistoryTest_RemoveWhileAdding) {
Randy Smith (Not in Mondays) 2012/09/13 19:53:19 Do you test the straight remove path anywhere in h
benjhayden 2012/09/21 20:45:46 DownloadHistoryTest_Create tests OnDownloadRemoved
+ ExpectQueryDownloads(scoped_ptr<InfoVector>(new InfoVector()));
+
+ // Note that db_handle must be -1 at first because it isn't in the db yet.
+ DownloadPersistentStoreInfo info;
+ InitItem(base::RandInt(0, 1 << 20),
+ FilePath(FILE_PATH_LITERAL("/foo/bar.pdf")),
+ GURL("http://example.com/bar.pdf"),
+ GURL("http://example.com/referrer.html"),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(10)),
+ (base::Time::Now() - base::TimeDelta::FromMinutes(1)),
+ 100,
+ 100,
+ content::DownloadItem::COMPLETE,
+ -1,
+ false,
+ &info);
+
+ // Instruct CreateDownload() to not callback to DownloadHistory immediately,
+ // but to wait for FinishCreateDownload().
+ set_slow_create_download(true);
+
+ // Pretend the manager just created |item|.
+ download_history()->OnDownloadCreated(&manager(), &item());
+ // CreateDownload() always gets db_handle=-1.
+ ExpectCreateDownload(info);
+ info.db_handle = 0;
+
+ // Call OnDownloadRemoved before calling back to DownloadHistory::ItemAdded().
+ // Instead of calling RemoveDownloads() immediately, it should
+ download_history()->OnDownloadRemoved(&item());
+ EXPECT_CALL(manager(), GetDownload(item().GetId()))
+ .WillRepeatedly(Return(static_cast<content::DownloadItem*>(NULL)));
+ ExpectNoRemoveDownloads();
+
+ // Now callback to DownloadHistory::ItemAdded(), and expect a call to
+ // RemoveDownloads() for the item that was removed while it was being added.
+ FinishCreateDownload();
+ HandleSet handles;
+ handles.insert(info.db_handle);
+ ExpectRemoveDownloads(handles);
+
+ // Pretend that the browser is closing.
+ download_history()->ManagerGoingDown(&manager());
+ download_history()->OnDownloadDestroyed(&item());
+}

Powered by Google App Engine
This is Rietveld 408576698