Make DownloadHistory observe manager, items

chrome/browser/download/download_history_unittest.cc

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..8249f31e53d4a22e92db3d9f72fe6e72701dd54b
--- /dev/null
+++ b/chrome/browser/download/download_history_unittest.cc
@@ -0,0 +1,527 @@
+// 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::Invoke;
+using testing::Return;
+using testing::ReturnRef;
+using testing::SetArgPointee;
+using testing::WithArg;
+using testing::_;
+
+namespace {
+
+void CheckInfoEqual(const DownloadPersistentStoreInfo& left,
+                    const DownloadPersistentStoreInfo& right) {
+  CHECK_EQ(left.path.value(), right.path.value());
+  CHECK_EQ(left.url.spec(), right.url.spec());
+  CHECK_EQ(left.referrer_url.spec(), right.referrer_url.spec());
+  CHECK_EQ(left.start_time.ToTimeT(), right.start_time.ToTimeT());
+  CHECK_EQ(left.end_time.ToTimeT(), right.end_time.ToTimeT());
+  CHECK_EQ(left.received_bytes, right.received_bytes);
+  CHECK_EQ(left.total_bytes, right.total_bytes);
+  CHECK_EQ(left.state, right.state);
+  CHECK_EQ(left.db_handle, right.db_handle);
+  CHECK_EQ(left.opened, right.opened);
+}
+
+typedef std::set<int64> HandleSet;
+typedef std::vector<DownloadPersistentStoreInfo> InfoVector;
+
+class FakeHistoryService : public HistoryService {
+ public:
+  FakeHistoryService()
+    : slow_create_download_(false),
+      handle_counter_(0) {
+  }
+
+  virtual Handle QueryDownloads(
+      CancelableRequestConsumerBase* consumer,
+      const HistoryService::DownloadQueryCallback& callback) OVERRIDE {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(&FakeHistoryService::QueryDownloadsDone,
+            base::Unretained(this), callback));
+    // DownloadHistory ignores the handle.
+    return -1;
+  }
+
+  void QueryDownloadsDone(
+      const HistoryService::DownloadQueryCallback& callback) {
+    CHECK(expect_query_downloads_.get());
+    callback.Run(expect_query_downloads_.get());
+    expect_query_downloads_.reset();
+  }
+
+  virtual Handle GetVisibleVisitCountToHost(
+      const GURL& referrer_url,
+      CancelableRequestConsumerBase* consumer,
+      const HistoryService::GetVisibleVisitCountToHostCallback&
+        callback) OVERRIDE {
+    NOTIMPLEMENTED();
+    return -1;
+  }
+
+  void set_slow_create_download(bool slow) { slow_create_download_ = slow; }
+
+  virtual Handle CreateDownload(
+      const DownloadPersistentStoreInfo& info,
+      CancelableRequestConsumerBase* consumer,
+      const HistoryService::DownloadCreateCallback& callback) OVERRIDE {
+    create_download_info_ = info;
+    create_download_callback_ = base::Bind(callback, handle_counter_++);
+    if (!slow_create_download_)
+      FinishCreateDownload();
+    return -1;
+  }
+
+  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 ExpectWillQueryDownloads(scoped_ptr<InfoVector> infos) {
+    expect_query_downloads_ = infos.Pass();
+  }
+
+  void ExpectQueryDownloadsDone() {
+    CHECK(NULL == expect_query_downloads_.get());
+  }
+
+  void ExpectDownloadCreated(
+      const DownloadPersistentStoreInfo& info) {
+    content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+    CheckInfoEqual(info, create_download_info_);
+    create_download_info_ = DownloadPersistentStoreInfo();
+  }
+
+  void ExpectNoDownloadCreated() {
+    content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+    CheckInfoEqual(DownloadPersistentStoreInfo(), create_download_info_);
+  }
+
+  void ExpectDownloadUpdated(const DownloadPersistentStoreInfo& info) {
+    content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+    CheckInfoEqual(update_download_, info);
+    update_download_ = DownloadPersistentStoreInfo();
+  }
+
+  void ExpectNoDownloadUpdated() {
+    content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+    CheckInfoEqual(DownloadPersistentStoreInfo(), update_download_);
+  }
+
+  void ExpectNoDownloadsRemoved() {
+    content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+    CHECK_EQ(0, static_cast<int>(remove_downloads_.size()));
+  }
+
+  void ExpectDownloadsRemoved(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),
+      manager_observer_(NULL),
+      item_observer_(NULL) {
+  }
+  virtual ~DownloadHistoryTest() {}
+
+ protected:
+  virtual void CleanUpOnMainThread() OVERRIDE {
+    download_history_.reset();
+  }
+
+  content::MockDownloadManager& manager() { return *manager_.get(); }
+  content::MockDownloadItem& item() { return item_; }
+
+  void SetManagerObserver(
+      content::DownloadManager::Observer* manager_observer) {
+    manager_observer_ = manager_observer;
+  }
+  content::DownloadManager::Observer* manager_observer() {
+    return manager_observer_;
+  }
+  void SetItemObserver(
+      content::DownloadItem::Observer* item_observer) {
+    item_observer_ = item_observer;
+  }
+  content::DownloadItem::Observer* item_observer() {
+    return item_observer_;
+  }
+
+  void ExpectWillQueryDownloads(scoped_ptr<InfoVector> infos) {
+    CHECK(infos.get());
+    EXPECT_CALL(manager(), AddObserver(_)).WillOnce(WithArg<0>(Invoke(
+        this, &DownloadHistoryTest::SetManagerObserver)));
+    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_->ExpectWillQueryDownloads(infos.Pass());
+    EXPECT_CALL(*manager_.get(), GetAllDownloads(_)).WillRepeatedly(Return());
+    download_history_.reset(new DownloadHistory(&manager(), history_.get()));
+    content::RunAllPendingInMessageLoop(content::BrowserThread::UI);
+    history_->ExpectQueryDownloadsDone();
+  }
+
+  void CallOnDownloadCreated() {
+    manager_observer()->OnDownloadCreated(&manager(), &item());
+  }
+
+  void set_slow_create_download(bool slow) {
+    history_->set_slow_create_download(slow);
+  }
+
+  void FinishCreateDownload() {
+    history_->FinishCreateDownload();
+  }
+
+  void ExpectDownloadCreated(
+      const DownloadPersistentStoreInfo& info) {
+    history_->ExpectDownloadCreated(info);
+  }
+
+  void ExpectNoDownloadCreated() {
+    history_->ExpectNoDownloadCreated();
+  }
+
+  void ExpectDownloadUpdated(const DownloadPersistentStoreInfo& info) {
+    history_->ExpectDownloadUpdated(info);
+  }
+
+  void ExpectNoDownloadUpdated() {
+    history_->ExpectNoDownloadUpdated();
+  }
+
+  void ExpectNoDownloadsRemoved() {
+    history_->ExpectNoDownloadsRemoved();
+  }
+
+  void ExpectDownloadsRemoved(const HandleSet& handles) {
+    history_->ExpectDownloadsRemoved(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(_)).WillOnce(WithArg<0>(Invoke(
+        this, &DownloadHistoryTest::SetItemObserver)));
+    EXPECT_CALL(item(), RemoveObserver(_));
+    std::vector<content::DownloadItem*> items;
+    items.push_back(&item());
+    EXPECT_CALL(*manager_.get(), GetAllDownloads(_))
+      .WillRepeatedly(SetArgPointee<0>(items));
+  }
+
+ private:
+  testing::NiceMock<content::MockDownloadItem> item_;
+  scoped_refptr<FakeHistoryService> history_;
+  scoped_refptr<content::MockDownloadManager> manager_;
+  scoped_ptr<DownloadHistory> download_history_;
+  content::DownloadManager::Observer* manager_observer_;
+  content::DownloadItem::Observer* item_observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(DownloadHistoryTest);
+};
+
+}  // anonymous namespace
+
+// Test loading an item from the database, changing it, saving it back, removing
+// it.
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest, DownloadHistoryTest_Load) {
+  // Load a download from history, create the item, OnDownloadCreated,
+  // OnDownloadUpdated, OnDownloadRemoved.
+  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);
+  {
+    scoped_ptr<InfoVector> infos(new InfoVector());
+    infos->push_back(info);
+    ExpectWillQueryDownloads(infos.Pass());
+    ExpectNoDownloadCreated();
+  }
+
+  // Pretend that something changed on the item.
+  EXPECT_CALL(item(), GetOpened()).WillRepeatedly(Return(true));
+  item_observer()->OnDownloadUpdated(&item());
+  info.opened = true;
+  ExpectDownloadUpdated(info);
+
+  // Pretend that the user removed the item.
+  HandleSet handles;
+  handles.insert(info.db_handle);
+  item_observer()->OnDownloadRemoved(&item());
+  ExpectDownloadsRemoved(handles);
+
+  // Pretend that the browser is closing.
+  manager_observer()->ManagerGoingDown(&manager());
+  item_observer()->OnDownloadDestroyed(&item());
+}
+
+// Test creating an item, saving it to the database, changing it, saving it
+// back, removing it.
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest, DownloadHistoryTest_Create) {
+  // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
+  // OnDownloadRemoved.
+  ExpectWillQueryDownloads(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|.
+  CallOnDownloadCreated();
+  // CreateDownload() always gets db_handle=-1.
+  ExpectDownloadCreated(info);
+  info.db_handle = 0;
+
+  // Pretend that something changed on the item.
+  EXPECT_CALL(item(), GetOpened()).WillRepeatedly(Return(true));
+  item_observer()->OnDownloadUpdated(&item());
+  info.opened = true;
+  ExpectDownloadUpdated(info);
+
+  // Pretend that the user removed the item.
+  HandleSet handles;
+  handles.insert(info.db_handle);
+  item_observer()->OnDownloadRemoved(&item());
+  ExpectDownloadsRemoved(handles);
+
+  // Pretend that the browser is closing.
+  manager_observer()->ManagerGoingDown(&manager());
+  item_observer()->OnDownloadDestroyed(&item());
+}
+
+// Test creating a new item, saving it, removing it by setting it Temporary,
+// changing it without saving it back because it's Temporary, clearing
+// IsTemporary, saving it back, changing it, saving it back because it isn't
+// Temporary anymore.
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest, DownloadHistoryTest_Temporary) {
+  // Create a fresh item not from history, OnDownloadCreated, OnDownloadUpdated,
+  // OnDownloadRemoved.
+  ExpectWillQueryDownloads(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|.
+  CallOnDownloadCreated();
+  // CreateDownload() always gets db_handle=-1.
+  ExpectDownloadCreated(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));
+  item_observer()->OnDownloadUpdated(&item());
+  HandleSet handles;
+  handles.insert(info.db_handle);
+  ExpectDownloadsRemoved(handles);
+
+  // Change something that would make DownloadHistory call UpdateDownload if the
+  // item weren't temporary.
+  EXPECT_CALL(item(), GetReceivedBytes()).WillRepeatedly(Return(4200));
+  item_observer()->OnDownloadUpdated(&item());
+  ExpectNoDownloadUpdated();
+
+  // Changing a temporary item back to a non-temporary item should make
+  // DownloadHistory call CreateDownload.
+  EXPECT_CALL(item(), IsTemporary()).WillRepeatedly(Return(false));
+  item_observer()->OnDownloadUpdated(&item());
+  info.received_bytes = 4200;
+  info.db_handle = -1;
+  // CreateDownload() always gets db_handle=-1.
+  ExpectDownloadCreated(info);
+  info.db_handle = 1;
+
+  EXPECT_CALL(item(), GetReceivedBytes()).WillRepeatedly(Return(100));
+  item_observer()->OnDownloadUpdated(&item());
+  info.received_bytes = 100;
+  ExpectDownloadUpdated(info);
+
+  // Pretend that the browser is closing.
+  manager_observer()->ManagerGoingDown(&manager());
+  item_observer()->OnDownloadDestroyed(&item());
+}
+
+// Test removing downloads while they're still being added.
+IN_PROC_BROWSER_TEST_F(DownloadHistoryTest,
+    DownloadHistoryTest_RemoveWhileAdding) {
+  ExpectWillQueryDownloads(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|.
+  CallOnDownloadCreated();
+  // CreateDownload() always gets db_handle=-1.
+  ExpectDownloadCreated(info);
+  info.db_handle = 0;
+
+  // Call OnDownloadRemoved before calling back to DownloadHistory::ItemAdded().
+  // Instead of calling RemoveDownloads() immediately, it should
+  item_observer()->OnDownloadRemoved(&item());
+  EXPECT_CALL(manager(), GetDownload(item().GetId()))
+    .WillRepeatedly(Return(static_cast<content::DownloadItem*>(NULL)));
+  ExpectNoDownloadsRemoved();
+
+  // 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);
+  ExpectDownloadsRemoved(handles);
+
+  // Pretend that the browser is closing.
+  manager_observer()->ManagerGoingDown(&manager());
+  item_observer()->OnDownloadDestroyed(&item());
+}

[Randy Smith (Not in Mondays), 2012/09/24 18:03:25]

Any tests to test multiple downloads in flight at the same time?  Probing maybe
specifically at the things where we batch updates?

[benjhayden, 2012/09/24 20:12:11]

Made a note to do that before committing.