chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc - Issue 2629603002: [NTP::Downloads] Fetch assets once the manager is loaded.

Unified Diff: chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc

Issue 2629603002: [NTP::Downloads] Fetch assets once the manager is loaded. (Closed)

Patch Set: constructed DownloadHistory + 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

Index: chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc

diff --git a/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc b/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc

index 94d78f2ee078be2526a3a9cd570dbe7b34619cc0..939bbed424eae9f6971b86c1d2a743e8a0b8612c 100644

--- a/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc

+++ b/chrome/browser/ntp_snippets/download_suggestions_provider_unittest.cc

@@ -18,17 +18,18 @@

#include "components/prefs/testing_pref_service.h"

#include "content/public/test/mock_download_item.h"

#include "content/public/test/mock_download_manager.h"

+#include "content/public/test/test_browser_thread_bundle.h"

#include "testing/gtest/include/gtest/gtest.h"

using content::DownloadItem;

using content::MockDownloadManager;

using ntp_snippets::Category;

+using ntp_snippets::CategoryStatus;

using ntp_snippets::ContentSuggestion;

using ntp_snippets::ContentSuggestionsProvider;

using ntp_snippets::MockContentSuggestionsProviderObserver;

using ntp_snippets::test::CaptureDismissedSuggestions;

using ntp_snippets::test::FakeOfflinePageModel;

-using ntp_snippets::CategoryStatus;

using offline_pages::ClientId;

using offline_pages::OfflinePageItem;

using test::FakeDownloadItem;

@@ -227,12 +228,28 @@ class ObservedMockDownloadManager : public MockDownloadManager {

std::vector<std::unique_ptr<FakeDownloadItem>> items_;

};

+class DummyHistoryAdapter : public DownloadHistory::HistoryAdapter {

+ public:

+ DummyHistoryAdapter() : HistoryAdapter(nullptr) {}

+ void QueryDownloads(

+ const history::HistoryService::DownloadQueryCallback& callback) override {

+ }

+ void CreateDownload(const history::DownloadRow& info,

+ const history::HistoryService::DownloadCreateCallback&

+ callback) override {}

+ void UpdateDownload(const history::DownloadRow& data,

+ bool should_commit_immediately) override {}

+ void RemoveDownloads(const std::set<uint32_t>& ids) override {}

+};

} // namespace

class DownloadSuggestionsProviderTest : public testing::Test {

public:

DownloadSuggestionsProviderTest()

- : pref_service_(new TestingPrefServiceSimple()) {

+ : download_history_(&downloads_manager_for_history_,

+ base::MakeUnique<DummyHistoryAdapter>()),

+ pref_service_(new TestingPrefServiceSimple()) {

DownloadSuggestionsProvider::RegisterProfilePrefs(

pref_service()->registry());

}

@@ -251,13 +268,24 @@ class DownloadSuggestionsProviderTest : public testing::Test {

EXPECT_CALL(observer_, OnSuggestionInvalidated(_, _)).Times(AnyNumber());

}

+ DownloadSuggestionsProvider* CreateLoadedProvider(bool show_assets,

+ bool show_offline_pages) {

+ CreateProvider(show_assets, show_offline_pages);

+ FireHistoryQueryComplete();

+ return provider_.get();

+ }

DownloadSuggestionsProvider* CreateProvider(bool show_assets,

bool show_offline_pages) {

DCHECK(!provider_);

DCHECK(show_assets || show_offline_pages);

+ // TODO(crbug.com/681766): Extract DownloadHistory interface and move

+ // implementation into DownloadHistoryImpl. Then mock it.

provider_ = base::MakeUnique<DownloadSuggestionsProvider>(

&observer_, show_offline_pages ? &offline_pages_model_ : nullptr,

- show_assets ? &downloads_manager_ : nullptr, pref_service(),

+ show_assets ? &downloads_manager_ : nullptr, &download_history_,

+ pref_service(),

/*download_manager_ui_enabled=*/false);

return provider_.get();

}

@@ -297,6 +325,8 @@ class DownloadSuggestionsProviderTest : public testing::Test {

}

+ void FireHistoryQueryComplete() { provider_->OnHistoryQueryComplete(); }

ContentSuggestion::ID GetDummySuggestionId(int id, bool is_offline_page) {

return ContentSuggestion::ID(

downloads_category(),

@@ -324,6 +354,13 @@ class DownloadSuggestionsProviderTest : public testing::Test {

TestingPrefServiceSimple* pref_service() { return pref_service_.get(); }

private:

+ // DownloadHistory requires UI thread.

+ content::TestBrowserThreadBundle thread_bundle_;

+ // We do not use DownloadHistory functionality in the tests, so we provide an

+ // empty manager to ensure no notifications, so that it does not intervene.

+ ObservedMockDownloadManager downloads_manager_for_history_;

+ DownloadHistory download_history_;

ObservedMockDownloadManager downloads_manager_;

FakeOfflinePageModel offline_pages_model_;

StrictMock<MockContentSuggestionsProviderObserver> observer_;

@@ -350,7 +387,7 @@ TEST_F(DownloadSuggestionsProviderTest,

HasDownloadSuggestionExtra(

/*is_download_asset=*/false,

FILE_PATH_LITERAL(""), "")))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

}

TEST_F(DownloadSuggestionsProviderTest,

@@ -360,7 +397,7 @@ TEST_F(DownloadSuggestionsProviderTest,

EXPECT_CALL(*observer(),

OnNewSuggestions(_, downloads_category(), SizeIs(0)));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

std::vector<std::unique_ptr<FakeDownloadItem>> asset_downloads =

CreateDummyAssetDownloads({1, 2});

@@ -401,7 +438,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldMixInBothSources) {

UnorderedElementsAre(

HasUrl("http://dummy.com/1"),

HasUrl("http://dummy.com/2"))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

std::vector<std::unique_ptr<FakeDownloadItem>> asset_downloads =

CreateDummyAssetDownloads({1, 2});

@@ -442,7 +479,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldSortSuggestions) {

OnNewSuggestions(_, downloads_category(),

ElementsAre(HasUrl("http://dummy.com/2"),

HasUrl("http://dummy.com/1"))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

std::vector<std::unique_ptr<FakeDownloadItem>> asset_downloads;

asset_downloads.push_back(CreateDummyAssetDownload(3, next_week));

@@ -468,9 +505,8 @@ TEST_F(DownloadSuggestionsProviderTest,

ShouldDismissWithoutNotifyingObservers) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(4ul))))

- .Times(2);

+ *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

@@ -478,11 +514,7 @@ TEST_F(DownloadSuggestionsProviderTest,

HasUrl("http://dummy.com/2"),

HasUrl("http://download.com/1"),

HasUrl("http://download.com/2"))));

- *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

EXPECT_CALL(*observer(), OnNewSuggestions(_, _, _)).Times(0);

EXPECT_CALL(*observer(), OnSuggestionInvalidated(_, _)).Times(0);

@@ -499,9 +531,8 @@ TEST_F(DownloadSuggestionsProviderTest,

ShouldNotReportDismissedSuggestionsOnNewData) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(4ul))))

- .Times(2);

+ *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

@@ -509,10 +540,7 @@ TEST_F(DownloadSuggestionsProviderTest,

HasUrl("http://dummy.com/2"),

HasUrl("http://download.com/1"),

HasUrl("http://download.com/2"))));

- *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/true));

@@ -531,9 +559,8 @@ TEST_F(DownloadSuggestionsProviderTest,

TEST_F(DownloadSuggestionsProviderTest, ShouldReturnDismissedSuggestions) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(4ul))))

- .Times(2);

+ *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

@@ -541,10 +568,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldReturnDismissedSuggestions) {

HasUrl("http://dummy.com/2"),

HasUrl("http://download.com/1"),

HasUrl("http://download.com/2"))));

- *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/true));

@@ -559,9 +583,8 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldReturnDismissedSuggestions) {

TEST_F(DownloadSuggestionsProviderTest, ShouldClearDismissedSuggestions) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(4ul))))

- .Times(2);

+ *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

@@ -569,10 +592,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldClearDismissedSuggestions) {

HasUrl("http://dummy.com/2"),

HasUrl("http://download.com/1"),

HasUrl("http://download.com/2"))));

- *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/true));

@@ -594,9 +614,8 @@ TEST_F(DownloadSuggestionsProviderTest,

ShouldNotDismissOtherTypeWithTheSameID) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(4ul))))

- .Times(2);

+ *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

@@ -604,10 +623,7 @@ TEST_F(DownloadSuggestionsProviderTest,

HasUrl("http://dummy.com/2"),

HasUrl("http://download.com/1"),

HasUrl("http://download.com/2"))));

- *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/true));

@@ -625,9 +641,10 @@ TEST_F(DownloadSuggestionsProviderTest,

TEST_F(DownloadSuggestionsProviderTest, ShouldReplaceDismissedItemWithNewData) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(5ul))))

- .Times(5);

+ // Currently the provider stores five items in its internal cache, so six

+ // items are needed to check whether all downloads are fetched on dismissal.

+ *(downloads_manager()->mutable_items()) =

+ CreateDummyAssetDownloads({1, 2, 3, 4, 5, 6});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

@@ -636,12 +653,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldReplaceDismissedItemWithNewData) {

HasUrl("http://download.com/3"),

HasUrl("http://download.com/4"),

HasUrl("http://download.com/5"))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- // Currently the provider stores five items in its internal cache, so six

- // items are needed to check whether all downloads are fetched on dismissal.

- *(downloads_manager()->mutable_items()) =

- CreateDummyAssetDownloads({1, 2, 3, 4, 5, 6});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/false));

@@ -665,18 +677,15 @@ TEST_F(DownloadSuggestionsProviderTest,

ShouldInvalidateWhenUnderlyingItemDeleted) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(3ul))));

+ *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

UnorderedElementsAre(HasUrl("http://dummy.com/1"),

HasUrl("http://dummy.com/2"),

HasUrl("http://download.com/1"))));

- *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

// We add another item manually, so that when it gets deleted it is not

// present in DownloadsManager list.

@@ -706,9 +715,8 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldReplaceRemovedItemWithNewData) {

IgnoreOnCategoryStatusChangedToAvailable();

IgnoreOnSuggestionInvalidated();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(5ul))))

- .Times(5);

+ *(downloads_manager()->mutable_items()) =

+ CreateDummyAssetDownloads({1, 2, 3, 4, 5});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

@@ -717,10 +725,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldReplaceRemovedItemWithNewData) {

HasUrl("http://download.com/3"),

HasUrl("http://download.com/4"),

HasUrl("http://download.com/5"))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

- *(downloads_manager()->mutable_items()) =

- CreateDummyAssetDownloads({1, 2, 3, 4, 5});

- FireDownloadsCreated(downloads_manager()->items());

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

// Note that |CreateDummyAssetDownloads| creates items "downloaded" before

// |base::Time::Now()|, so for a new item the time is set in future to enforce

@@ -764,7 +769,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldPruneOfflinePagesDismissedIDs) {

HasUrl("http://dummy.com/1"),

HasUrl("http://dummy.com/2"),

HasUrl("http://dummy.com/3"))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/true));

@@ -788,12 +793,13 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldPruneAssetDownloadsDismissedIDs) {

IgnoreOnCategoryStatusChangedToAvailable();

IgnoreOnSuggestionInvalidated();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(Lt(3ul))))

- .Times(3);

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

*(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

- FireDownloadsCreated(downloads_manager()->items());

+ EXPECT_CALL(

+ *observer(),

+ OnNewSuggestions(_, downloads_category(),

+ UnorderedElementsAre(HasUrl("http://download.com/1"),

+ HasUrl("http://download.com/2"))));

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/false));

@@ -809,28 +815,41 @@ TEST_F(DownloadSuggestionsProviderTest,

ShouldFetchAssetDownloadsOnStartupButOnlyOnce) {

IgnoreOnCategoryStatusChangedToAvailable();

- // Downloads manager was created before the provider, so |OnDownloadCreated|

- // calls "were" missed, but the provider must show missed items anyway.

*(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

EXPECT_CALL(

*observer(),

OnNewSuggestions(_, downloads_category(),

UnorderedElementsAre(HasUrl("http://download.com/1"),

HasUrl("http://download.com/2"))));

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+TEST_F(DownloadSuggestionsProviderTest,

+ ShouldFetchAssetDownloadsOnHistoryQueryComplete) {

+ IgnoreOnCategoryStatusChangedToAvailable();

+ EXPECT_CALL(*observer(), OnNewSuggestions(_, _, _)).Times(0);

CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1, 2});

+ EXPECT_CALL(

+ *observer(),

+ OnNewSuggestions(_, downloads_category(),

+ UnorderedElementsAre(HasUrl("http://download.com/1"),

+ HasUrl("http://download.com/2"))));

+ FireHistoryQueryComplete();

}

TEST_F(DownloadSuggestionsProviderTest,

ShouldInvalidateAssetDownloadWhenItsFileRemoved) {

IgnoreOnCategoryStatusChangedToAvailable();

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), IsEmpty()));

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(), SizeIs(1)));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

*(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

- FireDownloadsCreated(downloads_manager()->items());

+ EXPECT_CALL(

+ *observer(),

+ OnNewSuggestions(_, downloads_category(),

+ UnorderedElementsAre(HasUrl("http://download.com/1"))));

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

EXPECT_CALL(*observer(),

OnSuggestionInvalidated(

@@ -847,7 +866,7 @@ TEST_F(DownloadSuggestionsProviderTest,

*(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

EXPECT_CALL(*observer(),

OnNewSuggestions(_, downloads_category(), IsEmpty()));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

std::vector<std::unique_ptr<FakeDownloadItem>> asset_downloads =

CreateDummyAssetDownloads({1});

@@ -885,7 +904,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldLoadOfflinePagesOnModelLoaded) {

offline_pages_model()->set_is_loaded(false);

EXPECT_CALL(*observer(),

OnNewSuggestions(_, downloads_category(), IsEmpty()));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateProvider(/*show_assets=*/false, /*show_offline_pages=*/true);

*(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

offline_pages_model()->set_is_loaded(true);

@@ -907,7 +926,7 @@ TEST_F(DownloadSuggestionsProviderTest,

UnorderedElementsAre(

HasUrl("http://dummy.com/1"),

HasUrl("http://dummy.com/2"))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateProvider(/*show_assets=*/false, /*show_offline_pages=*/true);

}

TEST_F(DownloadSuggestionsProviderTest,

@@ -921,35 +940,21 @@ TEST_F(DownloadSuggestionsProviderTest,

OnNewSuggestions(_, downloads_category(),

UnorderedElementsAre(HasUrl("http://download.com/1"),

HasUrl("http://download.com/2"))));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

}

TEST_F(DownloadSuggestionsProviderTest,

- ShouldNotPruneDismissedSuggestionsOnStartup) {

+ ShouldLoadAndSubmitOfflinePagesEvenIfAssetDownloadsAreTurnedOff) {

IgnoreOnCategoryStatusChangedToAvailable();

IgnoreOnSuggestionInvalidated();

- // We dismiss an item to store it in the list of dismissed items.

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

- EXPECT_CALL(*observer(), OnNewSuggestions(_, downloads_category(), _));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

- provider()->DismissSuggestion(

- GetDummySuggestionId(1, /*is_offline_page=*/false));

- DestroyProvider();

- // We simulate current DownloadManager behaviour;

- // The download manager has not started reading the list yet, so it is empty.

- downloads_manager()->mutable_items()->clear();

- EXPECT_CALL(*observer(), OnNewSuggestions(_, downloads_category(), _));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

- Mock::VerifyAndClearExpectations(observer());

- // The first download is being read.

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

- EXPECT_CALL(*observer(), OnNewSuggestions(_, downloads_category(), _))

- .Times(0);

- FireDownloadCreated(downloads_manager()->items()[0].get());

- // The first download should not be reported, because it is dismissed.

+ *(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1, 2});

+ offline_pages_model()->set_is_loaded(true);

+ EXPECT_CALL(*observer(), OnNewSuggestions(_, downloads_category(),

+ UnorderedElementsAre(

+ HasUrl("http://dummy.com/1"),

+ HasUrl("http://dummy.com/2"))));

+ CreateProvider(/*show_assets=*/false, /*show_offline_pages=*/true);

}

TEST_F(DownloadSuggestionsProviderTest, ShouldStoreDismissedSuggestions) {

@@ -960,7 +965,7 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldStoreDismissedSuggestions) {

*(offline_pages_model()->mutable_items()) = CreateDummyOfflinePages({1});

*(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

EXPECT_CALL(*observer(), OnNewSuggestions(_, downloads_category(), _));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

provider()->DismissSuggestion(

GetDummySuggestionId(1, /*is_offline_page=*/true));

provider()->DismissSuggestion(

@@ -969,41 +974,45 @@ TEST_F(DownloadSuggestionsProviderTest, ShouldStoreDismissedSuggestions) {

DestroyProvider();

EXPECT_CALL(*observer(), OnNewSuggestions(_, downloads_category(), _));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/true);

EXPECT_THAT(GetDismissedSuggestions(),

UnorderedElementsAre(HasUrl("http://dummy.com/1"),

HasUrl("http://download.com/1")));

}

-// TODO(vitaliii): Remove this test once the dismissed ids are pruned. See

-// crbug.com/672758.

-TEST_F(DownloadSuggestionsProviderTest, ShouldRemoveOldDismissedIdsIfTooMany) {

+TEST_F(DownloadSuggestionsProviderTest,

+ ShouldNotPruneDismissedAssetDownloadsBeforeHistoryQueryComplete) {

IgnoreOnCategoryStatusChangedToAvailable();

IgnoreOnSuggestionInvalidated();

- const int kMaxDismissedIdCount =

- DownloadSuggestionsProvider::GetMaxDismissedCountForTesting();

- std::vector<int> ids;

- for (int i = 0; i < kMaxDismissedIdCount + 1; ++i) {

- ids.push_back(i);

- }

- *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads(ids);

+ // Dismiss items to store them in the list of dismissed items.

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

EXPECT_CALL(*observer(), OnNewSuggestions(_, downloads_category(), _));

- CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

+ CreateLoadedProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

+ provider()->DismissSuggestion(

+ GetDummySuggestionId(1, /*is_offline_page=*/false));

+ ASSERT_THAT(GetDismissedSuggestions(),

+ UnorderedElementsAre(HasUrl("http://download.com/1")));

+ // Destroy and create provider to simulate turning off Chrome.

+ DestroyProvider();

- for (int i = 0; i < static_cast<int>(ids.size()); ++i) {

- provider()->DismissSuggestion(

- GetDummySuggestionId(i, /*is_offline_page=*/false));

- }

+ downloads_manager()->mutable_items()->clear();

- EXPECT_THAT(GetDismissedSuggestions(), SizeIs(kMaxDismissedIdCount));

- DestroyProvider();

- // The oldest dismissed suggestion must become undismissed now. This is a

- // temporary workaround and not what we want in long term. This test must be

- // removed once we start pruning dismissed asset downloads on startup.

- EXPECT_CALL(*observer(),

- OnNewSuggestions(_, downloads_category(),

- ElementsAre(HasUrl("http://download.com/0"))));

+ EXPECT_CALL(*observer(), OnNewSuggestions(_, _, _)).Times(0);

CreateProvider(/*show_assets=*/true, /*show_offline_pages=*/false);

+ // Dismissed IDs should not be pruned yet, because the downloads list at the

+ // manager is not complete.

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

+ EXPECT_THAT(GetDismissedSuggestions(),

+ UnorderedElementsAre(HasUrl("http://download.com/1")));

+ EXPECT_CALL(*observer(), OnNewSuggestions(_, _, _));

+ downloads_manager()->mutable_items()->clear();

+ FireHistoryQueryComplete();

+ *(downloads_manager()->mutable_items()) = CreateDummyAssetDownloads({1});

+ // Once the manager has been loaded, the ids should be pruned.

+ EXPECT_THAT(GetDismissedSuggestions(), IsEmpty());

}

« no previous file with comments | « chrome/browser/ntp_snippets/download_suggestions_provider.cc ('k') | no next file » | no next file with comments »