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

Unified Diff: components/offline_pages/core/downloads/download_ui_adapter_unittest.cc

Issue 2631933002: Adding status info to DownloadUIItem and piping it through. (Closed)
Patch Set: fix dependency for tests Created 3 years, 11 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: components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
diff --git a/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc b/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
index c65dc37d075320fa2d36345da55abcef813f3af7..3e57d43d4340244286d3aa0db07d960ec949ce17 100644
--- a/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
+++ b/components/offline_pages/core/downloads/download_ui_adapter_unittest.cc
@@ -16,10 +16,13 @@
#include "base/files/file_path.h"
#include "base/run_loop.h"
#include "base/single_thread_task_runner.h"
+#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/test_mock_time_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
+#include "components/offline_pages/core/background/offliner_stub.h"
+#include "components/offline_pages/core/background/request_coordinator_stub_taco.h"
#include "components/offline_pages/core/client_namespace_constants.h"
#include "components/offline_pages/core/client_policy_controller.h"
#include "components/offline_pages/core/stub_offline_page_model.h"
@@ -31,7 +34,6 @@ namespace {
// Constants for a test OfflinePageItem.
static const int kTestOfflineId1 = 1;
static const int kTestOfflineId2 = 2;
-static const int kTestOfflineId3 = 3;
static const char kTestUrl[] = "http://foo.com/bar.mhtml";
static const char kTestGuid1[] = "cccccccc-cccc-4ccc-0ccc-ccccccccccc1";
static const char kTestGuid2[] = "cccccccc-cccc-4ccc-0ccc-ccccccccccc2";
@@ -47,23 +49,39 @@ static const base::Time kTestCreationTime = base::Time::Now();
static const base::string16 kTestTitle = base::ASCIIToUTF16("test title");
} // namespace
+// Mock DownloadUIAdapter::Delegate
+class DownloadUIAdapterDelegate : public DownloadUIAdapter::Delegate {
+ public:
+ DownloadUIAdapterDelegate() {}
+ ~DownloadUIAdapterDelegate() override {}
+
+ // DownloadUIAdapter::Delegate
+ bool IsVisibleInUI(const ClientId& client_id) override { return is_visible; }
+ bool IsTemporarilyHiddenInUI(const ClientId& client_id) override {
+ return is_temporarily_hidden;
+ }
+
+ bool is_visible = true;
+ bool is_temporarily_hidden = false;
+};
+
// Mock OfflinePageModel for testing the SavePage calls.
class MockOfflinePageModel : public StubOfflinePageModel {
public:
MockOfflinePageModel(base::TestMockTimeTaskRunner* task_runner)
: observer_(nullptr),
task_runner_(task_runner),
- policy_controller_(new ClientPolicyController()) {
- adapter.reset(new DownloadUIAdapter(this));
- // Add one page.
+ policy_controller_(new ClientPolicyController()) {}
+
+ ~MockOfflinePageModel() override {}
+
+ void AddInitialPage() {
OfflinePageItem page(GURL(kTestUrl), kTestOfflineId1, kTestClientId1,
kTestFilePath, kFileSize, kTestCreationTime);
page.title = kTestTitle;
pages[kTestOfflineId1] = page;
}
- ~MockOfflinePageModel() override {}
-
// OfflinePageModel overrides.
void AddObserver(Observer* observer) override {
EXPECT_TRUE(observer != nullptr);
@@ -111,9 +129,6 @@ class MockOfflinePageModel : public StubOfflinePageModel {
return policy_controller_.get();
}
- // Normally, OfflinePageModel owns this adapter, so lets test it this way.
- std::unique_ptr<DownloadUIAdapter> adapter;
-
std::map<int64_t, OfflinePageItem> pages;
private:
@@ -144,22 +159,49 @@ class DownloadUIAdapterTest : public testing::Test,
// queue.
void PumpLoop();
+ int64_t AddRequest(const GURL& url, const ClientId& client_id);
+
+ RequestCoordinator* request_coordinator() {
+ return request_coordinator_taco_->request_coordinator();
+ }
+
bool items_loaded;
std::vector<std::string> added_guids, updated_guids, deleted_guids;
std::unique_ptr<MockOfflinePageModel> model;
+ DownloadUIAdapterDelegate* adapter_delegate;
+ std::unique_ptr<DownloadUIAdapter> adapter;
+ OfflinerStub* offliner_stub;
private:
+ std::unique_ptr<RequestCoordinatorStubTaco> request_coordinator_taco_;
scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
+ base::ThreadTaskRunnerHandle task_runner_handle_;
};
DownloadUIAdapterTest::DownloadUIAdapterTest()
- : items_loaded(false), task_runner_(new base::TestMockTimeTaskRunner) {}
+ : items_loaded(false),
+ task_runner_(new base::TestMockTimeTaskRunner),
+ task_runner_handle_(task_runner_) {}
DownloadUIAdapterTest::~DownloadUIAdapterTest() {}
void DownloadUIAdapterTest::SetUp() {
- model.reset(new MockOfflinePageModel(task_runner_.get()));
- model->adapter->AddObserver(this);
+ model = base::MakeUnique<MockOfflinePageModel>(task_runner_.get());
+ std::unique_ptr<DownloadUIAdapterDelegate> delegate =
+ base::MakeUnique<DownloadUIAdapterDelegate>();
+ adapter_delegate = delegate.get();
+ request_coordinator_taco_ = base::MakeUnique<RequestCoordinatorStubTaco>();
+
+ std::unique_ptr<OfflinerStub> offliner = base::MakeUnique<OfflinerStub>();
+ offliner_stub = offliner.get();
+ request_coordinator_taco_->SetOffliner(std::move(offliner));
+
+ request_coordinator_taco_->CreateRequestCoordinator();
+ adapter = base::MakeUnique<DownloadUIAdapter>(
+ model.get(), request_coordinator_taco_->request_coordinator(),
+ std::move(delegate));
+
+ adapter->AddObserver(this);
}
void DownloadUIAdapterTest::ItemsLoaded() {
@@ -182,22 +224,33 @@ void DownloadUIAdapterTest::PumpLoop() {
task_runner_->RunUntilIdle();
}
+int64_t DownloadUIAdapterTest::AddRequest(const GURL& url,
+ const ClientId& client_id) {
+ return request_coordinator()->SavePageLater(
+ url, client_id, /* user_requested */ true,
+ RequestCoordinator::RequestAvailability::ENABLED_FOR_OFFLINER);
+}
+
TEST_F(DownloadUIAdapterTest, InitialLoad) {
- EXPECT_NE(nullptr, model->adapter);
+ EXPECT_NE(nullptr, adapter.get());
+ model->AddInitialPage();
EXPECT_FALSE(items_loaded);
PumpLoop();
EXPECT_TRUE(items_loaded);
- const DownloadUIItem* item = model->adapter->GetItem(kTestGuid1);
+ const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
EXPECT_NE(nullptr, item);
}
TEST_F(DownloadUIAdapterTest, InitialItemConversion) {
+ model->AddInitialPage();
EXPECT_EQ(1UL, model->pages.size());
EXPECT_EQ(kTestGuid1, model->pages[kTestOfflineId1].client_id.id);
PumpLoop();
- const DownloadUIItem* item = model->adapter->GetItem(kTestGuid1);
+ const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
EXPECT_EQ(kTestGuid1, item->guid);
EXPECT_EQ(kTestUrl, item->url.spec());
+ EXPECT_EQ(DownloadUIItem::DownloadState::COMPLETE, item->download_state);
+ EXPECT_EQ(0, item->download_progress_bytes);
EXPECT_EQ(kTestFilePath, item->target_path);
EXPECT_EQ(kTestCreationTime, item->start_time);
EXPECT_EQ(kFileSize, item->total_bytes);
@@ -205,6 +258,7 @@ TEST_F(DownloadUIAdapterTest, InitialItemConversion) {
}
TEST_F(DownloadUIAdapterTest, ItemDeletedAdded) {
+ model->AddInitialPage();
PumpLoop();
// Add page, notify adapter.
OfflinePageItem page(GURL(kTestUrl), kTestOfflineId2, kTestClientId2,
@@ -223,26 +277,54 @@ TEST_F(DownloadUIAdapterTest, ItemDeletedAdded) {
EXPECT_EQ(kTestGuid2, deleted_guids[1]);
}
-TEST_F(DownloadUIAdapterTest, ItemWithWrongNamespace) {
+TEST_F(DownloadUIAdapterTest, NotVisibleItem) {
+ model->AddInitialPage();
PumpLoop();
+ adapter_delegate->is_visible = false;
OfflinePageItem page1(
GURL(kTestUrl), kTestOfflineId2, kTestClientIdOtherNamespace,
base::FilePath(kTestFilePath), kFileSize, kTestCreationTime);
model->AddPageAndNotifyAdapter(page1);
PumpLoop();
- // Should not add the page with wrong namespace.
+ // Should not add the page.
EXPECT_EQ(0UL, added_guids.size());
+}
- OfflinePageItem page2(GURL(kTestUrl), kTestOfflineId3, kTestClientIdOtherGuid,
- base::FilePath(kTestFilePath), kFileSize,
- kTestCreationTime);
- model->AddPageAndNotifyAdapter(page2);
+TEST_F(DownloadUIAdapterTest, TemporarilyNotVisibleItem) {
+ adapter_delegate->is_temporarily_hidden = true;
+ model->AddInitialPage();
PumpLoop();
- // Should not add the page with wrong guid.
+ // Initial Item should be invisible in the collection now.
+ EXPECT_EQ(nullptr, adapter->GetItem(kTestGuid1));
+ EXPECT_EQ(0UL, adapter->GetAllItems().size());
EXPECT_EQ(0UL, added_guids.size());
+ EXPECT_EQ(0UL, deleted_guids.size());
+
+ adapter_delegate->is_temporarily_hidden = false;
+ // Notify adapter about visibility change for the clientId of initial page.
+ adapter->TemporaryHiddenStatusChanged(kTestClientId1);
+ PumpLoop();
+
+ // There should be OnAdded simulated.
+ EXPECT_EQ(1UL, added_guids.size());
+ EXPECT_EQ(0UL, deleted_guids.size());
+ // Also the item should be visible in the collection of items now.
+ EXPECT_NE(nullptr, adapter->GetItem(kTestGuid1));
+ EXPECT_EQ(1UL, adapter->GetAllItems().size());
+
+ // Switch visibility back to hidden
+ adapter_delegate->is_temporarily_hidden = true;
+ adapter->TemporaryHiddenStatusChanged(kTestClientId1);
+ // There should be OnDeleted fired.
+ EXPECT_EQ(1UL, added_guids.size());
+ EXPECT_EQ(1UL, deleted_guids.size());
+ // Also the item should be visible in the collection of items now.
+ EXPECT_EQ(nullptr, adapter->GetItem(kTestGuid1));
+ EXPECT_EQ(0UL, adapter->GetAllItems().size());
}
TEST_F(DownloadUIAdapterTest, ItemAdded) {
+ model->AddInitialPage();
PumpLoop();
// Clear the initial page and replace it with updated one.
model->pages.clear();
@@ -261,16 +343,95 @@ TEST_F(DownloadUIAdapterTest, ItemAdded) {
}
TEST_F(DownloadUIAdapterTest, NoHangingLoad) {
- EXPECT_NE(nullptr, model->adapter);
+ model->AddInitialPage();
+ EXPECT_NE(nullptr, adapter.get());
EXPECT_FALSE(items_loaded);
// Removal of last observer causes cache unload of not-yet-loaded cache.
- model->adapter->RemoveObserver(this);
+ adapter->RemoveObserver(this);
// This will complete async fetch of items, but...
PumpLoop();
// items should not be loaded when there is no observers!
EXPECT_FALSE(items_loaded);
// This should not crash.
- model->adapter->AddObserver(this);
+ adapter->AddObserver(this);
+}
+
+TEST_F(DownloadUIAdapterTest, LoadExistingRequest) {
+ AddRequest(GURL(kTestUrl), kTestClientId1);
+ PumpLoop();
+ EXPECT_TRUE(items_loaded);
+ const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
+ EXPECT_NE(nullptr, item);
+}
+
+TEST_F(DownloadUIAdapterTest, AddRequest) {
+ PumpLoop();
+ EXPECT_TRUE(items_loaded);
+ EXPECT_EQ(0UL, added_guids.size());
+ AddRequest(GURL(kTestUrl), kTestClientId1);
+ PumpLoop();
+ EXPECT_EQ(1UL, added_guids.size());
+ EXPECT_EQ(kTestClientId1.id, added_guids[0]);
+ const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
+ EXPECT_NE(nullptr, item);
+}
+
+TEST_F(DownloadUIAdapterTest, RemoveRequest) {
+ int64_t id = AddRequest(GURL(kTestUrl), kTestClientId1);
+ PumpLoop();
+ // No added requests, the initial one is loaded.
+ EXPECT_EQ(0UL, added_guids.size());
+ EXPECT_NE(nullptr, adapter->GetItem(kTestGuid1));
+ EXPECT_EQ(0UL, deleted_guids.size());
+
+ std::vector<int64_t> requests_to_remove = {id};
+ request_coordinator()->RemoveRequests(
+ requests_to_remove,
+ base::Bind(
+ [](int64_t id, const MultipleItemStatuses& statuses) {
+ EXPECT_EQ(1UL, statuses.size());
+ EXPECT_EQ(id, statuses[0].first);
+ EXPECT_EQ(ItemActionStatus::SUCCESS, statuses[0].second);
+ },
+ id));
+ PumpLoop();
+
+ EXPECT_EQ(0UL, added_guids.size());
+ EXPECT_EQ(1UL, deleted_guids.size());
+ EXPECT_EQ(kTestClientId1.id, deleted_guids[0]);
+ EXPECT_EQ(nullptr, adapter->GetItem(kTestGuid1));
+}
+
+TEST_F(DownloadUIAdapterTest, RequestBecomesPage) {
+ // This will cause requests to be 'offlined' all the way and removed.
+ offliner_stub->enable_callback(true);
+ AddRequest(GURL(kTestUrl), kTestClientId1);
+ PumpLoop();
+
+ const DownloadUIItem* item = adapter->GetItem(kTestGuid1);
+ EXPECT_NE(nullptr, item);
+ // The item is still IN_PROGRESS, since we did not delete it when
+ // request is competed successfully, waiting for the page with the
+ // same client_id to come in.
+ EXPECT_EQ(DownloadUIItem::DownloadState::IN_PROGRESS, item->download_state);
+ // Add a new saved page with the same client id.
+ // This simulates what happens when the request is completed.
+ // It should not fire and OnAdded or OnDeleted, just OnUpdated.
+ OfflinePageItem page(GURL(kTestUrl), kTestOfflineId1, kTestClientId1,
+ base::FilePath(kTestFilePath), kFileSize,
+ kTestCreationTime);
+ model->AddPageAndNotifyAdapter(page);
+ PumpLoop();
+
+ // No added or deleted items, the one existing item should be 'updated'.
+ EXPECT_EQ(0UL, added_guids.size());
+ EXPECT_EQ(0UL, deleted_guids.size());
+
+ EXPECT_GE(updated_guids.size(), 1UL);
+ std::string last_updated_guid = updated_guids[updated_guids.size() - 1];
+ item = adapter->GetItem(last_updated_guid);
+ EXPECT_NE(nullptr, item);
+ EXPECT_EQ(DownloadUIItem::DownloadState::COMPLETE, item->download_state);
}
} // namespace offline_pages

Powered by Google App Engine
This is Rietveld 408576698