chrome/browser/search_engines/template_url_service_unittest.cc - Issue 2498053002: Add field to monitor last visited time for each search engine

Unified Diff: chrome/browser/search_engines/template_url_service_unittest.cc

Issue 2498053002: Add field to monitor last visited time for each search engine (Closed)

Patch Set: Add unit test for last_visited field. Created 4 years, 1 month 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

« chrome/browser/search_engines/template_url_service_sync_unittest.cc ('K') | « chrome/browser/search_engines/template_url_service_sync_unittest.cc ('k') | components/search_engines/default_search_manager.h » ('j') | components/search_engines/default_search_manager_unittest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: chrome/browser/search_engines/template_url_service_unittest.cc

diff --git a/chrome/browser/search_engines/template_url_service_unittest.cc b/chrome/browser/search_engines/template_url_service_unittest.cc

index 3e86c1a9cdaf6479557021e3306207ff19f742bf..461a6f5f36fd82a71c29b360a2833ecdfcfb52c8 100644

--- a/chrome/browser/search_engines/template_url_service_unittest.cc

+++ b/chrome/browser/search_engines/template_url_service_unittest.cc

@@ -74,6 +74,7 @@ std::unique_ptr<TemplateURL> CreateKeywordWithDate(

const std::string& encodings,

Time date_created,

Time last_modified,

+ Time last_visited,

TemplateURL::Type type = TemplateURL::NORMAL) {

TemplateURLData data;

data.SetShortName(base::UTF8ToUTF16(short_name));

@@ -89,6 +90,7 @@ std::unique_ptr<TemplateURL> CreateKeywordWithDate(

encodings, ";", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);

data.date_created = date_created;

data.last_modified = last_modified;

+ data.last_visited = last_visited;

return base::MakeUnique<TemplateURL>(data, type);

}

@@ -103,16 +105,19 @@ TemplateURL* AddKeywordWithDate(

bool safe_for_autoreplace,

const std::string& encodings,

Time date_created,

- Time last_modified) {

+ Time last_modified,

+ Time last_visited) {

TemplateURL* t_url = model->Add(CreateKeywordWithDate(

- model, short_name, keyword, url, suggest_url, alternate_url, favicon_url,

- safe_for_autoreplace, 0, encodings, date_created, last_modified));

+ model, short_name, keyword, url, suggest_url, alternate_url,

+ favicon_url, safe_for_autoreplace, 0, encodings, date_created,

+ last_modified, last_visited));

EXPECT_NE(0, t_url->id());

return t_url;

}

// Checks that the two TemplateURLs are similar. It does not check the id, the

-// date_created or the last_modified time. Neither pointer should be NULL.

+// date_created, the last_modified and the last_visit time. Neither pointer

Peter Kasting 2016/11/21 03:35:08 Nit: Shorter, and more future-proof: "It does not

ltian 2016/11/28 22:08:01 Done.

+// should be NULL.

void ExpectSimilar(const TemplateURL* expected, const TemplateURL* actual) {

ASSERT_TRUE(expected != NULL);

ASSERT_TRUE(actual != NULL);

@@ -154,7 +159,8 @@ class TemplateURLServiceTest : public testing::Test {

bool safe_for_autoreplace,

const std::string& encodings,

Time date_created,

- Time last_modified);

+ Time last_modified,

+ Time last_visited);

// Verifies the two TemplateURLs are equal.

void AssertEquals(const TemplateURL& expected, const TemplateURL& actual);

@@ -222,10 +228,12 @@ TemplateURL* TemplateURLServiceTest::AddKeywordWithDate(

bool safe_for_autoreplace,

const std::string& encodings,

Time date_created,

- Time last_modified) {

+ Time last_modified,

+ Time last_visited) {

return ::AddKeywordWithDate(model(), short_name, keyword, url, suggest_url,

alternate_url, favicon_url, safe_for_autoreplace,

- encodings, date_created, last_modified);

+ encodings, date_created, last_modified,

+ last_visited);

}

void TemplateURLServiceTest::AssertEquals(const TemplateURL& expected,

@@ -242,6 +250,7 @@ void TemplateURLServiceTest::AssertEquals(const TemplateURL& expected,

ASSERT_EQ(expected.id(), actual.id());

ASSERT_EQ(expected.date_created(), actual.date_created());

AssertTimesEqual(expected.last_modified(), actual.last_modified());

+ ASSERT_EQ(expected.last_visited(), actual.last_visited());

ASSERT_EQ(expected.sync_guid(), actual.sync_guid());

ASSERT_EQ(expected.search_terms_replacement_key(),

actual.search_terms_replacement_key());

@@ -267,6 +276,7 @@ std::unique_ptr<TemplateURL> TemplateURLServiceTest::CreatePreloadedTemplateURL(

data.input_encodings.push_back("UTF-8");

data.date_created = Time::FromTimeT(100);

data.last_modified = Time::FromTimeT(100);

+ data.last_visited = Time::FromTimeT(100);

data.prepopulate_id = prepopulate_id;

return base::MakeUnique<TemplateURL>(data);

}

@@ -301,6 +311,7 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) {

data.safe_for_autoreplace = true;

data.date_created = Time::FromTimeT(100);

data.last_modified = Time::FromTimeT(100);

+ data.last_visited = Time::FromTimeT(100);

data.sync_guid = "00000000-0000-0000-0000-000000000001";

TemplateURL* t_url = model()->Add(base::MakeUnique<TemplateURL>(data));

ASSERT_TRUE(model()->CanAddAutogeneratedKeyword(ASCIIToUTF16("keyword"),

@@ -367,7 +378,7 @@ TEST_F(TemplateURLServiceTest, AddSameKeyword) {

AddKeywordWithDate(

"first", "keyword", "http://test1", std::string(), std::string(),

- std::string(), true, "UTF-8", Time(), Time());

+ std::string(), true, "UTF-8", Time(), Time(), Time());

VerifyObserverCount(1);

// Test what happens when we try to add a TemplateURL with the same keyword as

@@ -418,10 +429,10 @@ TEST_F(TemplateURLServiceTest, AddExtensionKeyword) {

AddKeywordWithDate(

"replaceable", "keyword1", "http://test1", std::string(), std::string(),

- std::string(), true, "UTF-8", Time(), Time());

+ std::string(), true, "UTF-8", Time(), Time(), Time());

TemplateURL* original2 = AddKeywordWithDate(

"nonreplaceable", "keyword2", "http://test2", std::string(),

- std::string(), std::string(), false, "UTF-8", Time(), Time());

+ std::string(), std::string(), false, "UTF-8", Time(), Time(), Time());

model()->RegisterOmniboxKeyword("test3", "extension", "keyword3",

"http://test3");

TemplateURL* original3 =

@@ -467,7 +478,7 @@ TEST_F(TemplateURLServiceTest, AddSameKeywordWithExtensionPresent) {

// Adding a keyword that matches the extension.

AddKeywordWithDate(

"replaceable", "keyword", "http://test1", std::string(), std::string(),

- std::string(), true, "UTF-8", Time(), Time());

+ std::string(), true, "UTF-8", Time(), Time(), Time());

// Adding another replaceable keyword should remove the existing one, but

// leave the extension as is.

@@ -518,24 +529,24 @@ TEST_F(TemplateURLServiceTest, ClearBrowsingData_Keywords) {

// Create one with a 0 time.

AddKeywordWithDate("name1", "key1", "http://foo1", "http://suggest1",

std::string(), "http://icon1", true, "UTF-8;UTF-16",

- Time(), Time());

+ Time(), Time(), Time());

// Create one for now and +/- 1 day.

AddKeywordWithDate("name2", "key2", "http://foo2", "http://suggest2",

std::string(), "http://icon2", true, "UTF-8;UTF-16",

- now - one_day, Time());

+ now - one_day, Time(), Time());

AddKeywordWithDate("name3", "key3", "http://foo3", std::string(),

std::string(), std::string(), true, std::string(), now,

- Time());

+ Time(), Time());

AddKeywordWithDate("name4", "key4", "http://foo4", std::string(),

std::string(), std::string(), true, std::string(),

- now + one_day, Time());

+ now + one_day, Time(), Time());

// Try the other three states.

AddKeywordWithDate("name5", "key5", "http://foo5", "http://suggest5",

std::string(), "http://icon5", false, "UTF-8;UTF-16", now,

- Time());

+ Time(), Time());

AddKeywordWithDate("name6", "key6", "http://foo6", "http://suggest6",

std::string(), "http://icon6", false, "UTF-8;UTF-16",

- month_ago, Time());

+ month_ago, Time(), Time());

// We just added a few items, validate them.

EXPECT_EQ(6U, model()->GetTemplateURLs().size());

@@ -582,13 +593,13 @@ TEST_F(TemplateURLServiceTest, ClearBrowsingData_KeywordsForUrls) {

// Create one for now and +/- 1 day.

AddKeywordWithDate("name1", "key1", "http://foo1", "http://suggest1",

std::string(), "http://icon2", true, "UTF-8;UTF-16",

- now - one_day, Time());

+ now - one_day, Time(), Time());

AddKeywordWithDate("name2", "key2", "http://foo2", std::string(),

std::string(), std::string(), true, std::string(), now,

- Time());

+ Time(), Time());

AddKeywordWithDate("name3", "key3", "http://foo3", std::string(),

std::string(), std::string(), true, std::string(),

- now + one_day, Time());

+ now + one_day, Time(), Time());

// We just added a few items, validate them.

EXPECT_EQ(3U, model()->GetTemplateURLs().size());

@@ -640,6 +651,7 @@ TEST_F(TemplateURLServiceTest, Reset) {

data.favicon_url = GURL("http://favicon.url");

data.date_created = Time::FromTimeT(100);

data.last_modified = Time::FromTimeT(100);

+ data.last_visited = Time::FromTimeT(100);

TemplateURL* t_url = model()->Add(base::MakeUnique<TemplateURL>(data));

VerifyObserverCount(1);

@@ -682,7 +694,7 @@ TEST_F(TemplateURLServiceTest, DefaultSearchProvider) {

const size_t initial_count = model()->GetTemplateURLs().size();

TemplateURL* t_url = AddKeywordWithDate(

"name1", "key1", "http://foo1/{searchTerms}", "http://sugg1",

- std::string(), "http://icon1", true, "UTF-8;UTF-16", Time(), Time());

+ std::string(), "http://icon1", true, "UTF-8;UTF-16", Time(), Time(), Time());

test_util()->ResetObserverCount();

model()->SetUserSelectedDefaultSearchProvider(t_url);

@@ -710,7 +722,7 @@ TEST_F(TemplateURLServiceTest, CantReplaceWithSameKeyword) {

NULL));

TemplateURL* t_url = AddKeywordWithDate(

"name1", "foo", "http://foo1", "http://sugg1", std::string(),

- "http://icon1", true, "UTF-8;UTF-16", Time(), Time());

+ "http://icon1", true, "UTF-8;UTF-16", Time(), Time(), Time());

// Can still replace, newly added template url is marked safe to replace.

ASSERT_TRUE(model()->CanAddAutogeneratedKeyword(ASCIIToUTF16("foo"),

@@ -731,7 +743,7 @@ TEST_F(TemplateURLServiceTest, CantReplaceWithSameHosts) {

GURL("http://foo.com"), NULL));

TemplateURL* t_url = AddKeywordWithDate(

"name1", "foo", "http://foo.com", "http://sugg1", std::string(),

- "http://icon1", true, "UTF-8;UTF-16", Time(), Time());

+ "http://icon1", true, "UTF-8;UTF-16", Time(), Time(), Time());

// Can still replace, newly added template url is marked safe to replace.

ASSERT_TRUE(model()->CanAddAutogeneratedKeyword(ASCIIToUTF16("bar"),

@@ -767,6 +779,7 @@ TEST_F(TemplateURLServiceTest, DefaultSearchProviderLoadedFromPrefs) {

data.instant_url = "http://instant";

data.date_created = Time::FromTimeT(100);

data.last_modified = Time::FromTimeT(100);

+ data.last_visited = Time::FromTimeT(100);

TemplateURL* t_url = model()->Add(base::MakeUnique<TemplateURL>(data));

const TemplateURLID id = t_url->id();

@@ -812,7 +825,7 @@ TEST_F(TemplateURLServiceTest, RepairPrepopulatedSearchEngines) {

TemplateURL* user_dse = AddKeywordWithDate(

"malware", "google.com", "http://www.goo.com/s?q={searchTerms}",

std::string(), std::string(), std::string(),

- true, "UTF-8", Time(), Time());

+ true, "UTF-8", Time(), Time(), Time());

model()->SetUserSelectedDefaultSearchProvider(user_dse);

EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());

@@ -908,7 +921,7 @@ TEST_F(TemplateURLServiceTest, UpdateKeywordSearchTermsForURL) {

test_util()->ChangeModelToLoadState();

AddKeywordWithDate("name", "x", "http://x/foo?q={searchTerms}",

"http://sugg1", "http://x/foo#query={searchTerms}",

- "http://icon1", false, "UTF-8;UTF-16", Time(), Time());

+ "http://icon1", false, "UTF-8;UTF-16", Time(), Time(), Time());

Peter Kasting 2016/11/21 03:35:08 Nit: 80 columns (several places) (Running git cl

ltian 2016/11/28 22:08:02 Done.

for (size_t i = 0; i < arraysize(data); ++i) {

TemplateURLService::URLVisitedDetails details = {

@@ -930,7 +943,7 @@ TEST_F(TemplateURLServiceTest, DontUpdateKeywordSearchForNonReplaceable) {

test_util()->ChangeModelToLoadState();

AddKeywordWithDate("name", "x", "http://x/foo", "http://sugg1", std::string(),

- "http://icon1", false, "UTF-8;UTF-16", Time(), Time());

+ "http://icon1", false, "UTF-8;UTF-16", Time(), Time(), Time());

for (size_t i = 0; i < arraysize(data); ++i) {

TemplateURLService::URLVisitedDetails details = {

@@ -949,7 +962,7 @@ TEST_F(TemplateURLServiceWithoutFallbackTest, ChangeGoogleBaseValue) {

test_util()->SetGoogleBaseURL(GURL("http://google.com/"));

const TemplateURL* t_url = AddKeywordWithDate(

"name", "google.com", "{google:baseURL}?q={searchTerms}", "http://sugg1",

- std::string(), "http://icon1", false, "UTF-8;UTF-16", Time(), Time());

+ std::string(), "http://icon1", false, "UTF-8;UTF-16", Time(), Time(), Time());

ASSERT_EQ(t_url, model()->GetTemplateURLForHost("google.com"));

EXPECT_EQ("google.com", t_url->url_ref().GetHost(search_terms_data()));

EXPECT_EQ(ASCIIToUTF16("google.com"), t_url->keyword());

@@ -972,7 +985,7 @@ TEST_F(TemplateURLServiceWithoutFallbackTest, ChangeGoogleBaseValue) {

TemplateURL* manual = AddKeywordWithDate(

"manual", "google.de", "http://google.de/search?q={searchTerms}",

std::string(), std::string(), std::string(), false, "UTF-8", Time(),

- Time());

+ Time(), Time());

test_util()->SetGoogleBaseURL(GURL("http://google.de"));

// Verify that the manual entry is untouched, and the autogenerated keyword

@@ -1009,7 +1022,7 @@ TEST_F(TemplateURLServiceTest, GenerateVisitOnKeyword) {

TemplateURL* t_url = AddKeywordWithDate(

"keyword", "keyword", "http://foo.com/foo?query={searchTerms}",

"http://sugg1", std::string(), "http://icon1", true, "UTF-8;UTF-16",

- base::Time::Now(), base::Time::Now());

+ base::Time::Now(), base::Time::Now(), base::Time::Now());

Peter Kasting 2016/11/21 03:35:08 Is it important that we set this to Time::Now()?

ltian 2016/11/28 22:08:02 Done.

// Add a visit that matches the url of the keyword.

history::HistoryService* history = HistoryServiceFactory::GetForProfile(

@@ -1220,7 +1233,7 @@ TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) {

// Set a regular default search provider.

TemplateURL* regular_default = AddKeywordWithDate(

"name1", "key1", "http://foo1/{searchTerms}", "http://sugg1",

- std::string(), "http://icon1", true, "UTF-8;UTF-16", Time(), Time());

+ std::string(), "http://icon1", true, "UTF-8;UTF-16", Time(), Time(), Time());

VerifyObserverCount(1);

model()->SetUserSelectedDefaultSearchProvider(regular_default);

// Adding the URL and setting the default search provider should have caused

@@ -1416,14 +1429,15 @@ TEST_F(TemplateURLServiceTest, DefaultExtensionEngine) {

TemplateURL* user_dse = AddKeywordWithDate(

"user", "user", "http://www.goo.com/s?q={searchTerms}",

std::string(), std::string(), std::string(),

- true, "UTF-8", Time(), Time());

+ true, "UTF-8", Time(), Time(), Time());

model()->SetUserSelectedDefaultSearchProvider(user_dse);

EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());

std::unique_ptr<TemplateURL> ext_dse = CreateKeywordWithDate(

model(), "ext", "ext", "http://www.search.com/s?q={searchTerms}",

std::string(), std::string(), std::string(), true, kPrepopulatedId,

- "UTF-8", Time(), Time(), TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

+ "UTF-8", Time(), Time(), Time(),

+ TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

std::unique_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(

new TemplateURL::AssociatedExtensionInfo("ext"));

extension_info->wants_to_be_default_engine = true;

@@ -1442,14 +1456,14 @@ TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {

TemplateURL* user_dse = AddKeywordWithDate(

"user", "user", "http://www.goo.com/s?q={searchTerms}",

std::string(), std::string(), std::string(),

- true, "UTF-8", Time(), Time());

+ true, "UTF-8", Time(), Time(), Time());

model()->SetUserSelectedDefaultSearchProvider(user_dse);

EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());

std::unique_ptr<TemplateURL> ext_dse = CreateKeywordWithDate(

model(), "ext1", "ext1", "http://www.ext1.com/s?q={searchTerms}",

std::string(), std::string(), std::string(), true, 0, "UTF-8", Time(),

- Time(), TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

+ Time(), Time(), TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

std::unique_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(

new TemplateURL::AssociatedExtensionInfo("ext1"));

extension_info->wants_to_be_default_engine = false;

@@ -1460,7 +1474,8 @@ TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {

ext_dse = CreateKeywordWithDate(

model(), "ext2", "ext2", "http://www.ext2.com/s?q={searchTerms}",

std::string(), std::string(), std::string(), true, kPrepopulatedId,

- "UTF-8", Time(), Time(), TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

+ "UTF-8", Time(), Time(), Time(),

+ TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

extension_info.reset(new TemplateURL::AssociatedExtensionInfo("ext2"));

extension_info->wants_to_be_default_engine = true;

TemplateURL* ext_dse_ptr = model()->AddExtensionControlledTURL(

@@ -1506,7 +1521,8 @@ TEST_F(TemplateURLServiceTest, ExtensionEngineVsPolicy) {

std::unique_ptr<TemplateURL> ext_dse = CreateKeywordWithDate(

model(), "ext1", "ext1", "http://www.ext1.com/s?q={searchTerms}",

std::string(), std::string(), std::string(), true, kPrepopulatedId,

- "UTF-8", Time(), Time(), TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

+ "UTF-8", Time(), Time(), Time(),

+ TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);

std::unique_ptr<TemplateURL::AssociatedExtensionInfo> extension_info(

new TemplateURL::AssociatedExtensionInfo("ext1"));

extension_info->wants_to_be_default_engine = true;

@@ -1518,3 +1534,24 @@ TEST_F(TemplateURLServiceTest, ExtensionEngineVsPolicy) {

actual_managed_default = model()->GetDefaultSearchProvider();

ExpectSimilar(expected_managed_default.get(), actual_managed_default);

}

+TEST_F(TemplateURLServiceTest, LastVisitedTimeUpdate) {

+ test_util()->VerifyLoad();

+ TemplateURL* original_url = AddKeywordWithDate(

+ "name1", "key1", "http://foo1","http://suggest1",

+ std::string(), "http://icon1", true, "UTF-8;UTF-16",

+ Time(), Time(), Time());

+ base::Time original_last_visited = original_url->last_visited();

Peter Kasting 2016/11/21 03:35:08 Nit: Can be const (2 places)

ltian 2016/11/28 22:08:02 Done.

+ model()->UpdateTemplateURLVisitTime(original_url);

+ TemplateURL* modified_url = model()->GetTemplateURLForKeyword(

Peter Kasting 2016/11/21 03:35:08 Nit: If you follow my next comment, then this coul

ltian 2016/11/28 22:08:01 git cl format does as you suggest.

Peter Kasting 2016/12/01 07:38:24 "Inlined into the next statement" isn't about line

+ ASCIIToUTF16("key1"));

+ base::Time modifed_last_visited = modified_url->last_visited();

+ EXPECT_NE(original_last_visited, modified_url->last_visited());

Peter Kasting 2016/11/21 03:35:08 Nit: Use |modifed_last_visited|

ltian 2016/11/28 22:08:01 Done.

+ test_util()->ResetModel(true);

+ TemplateURL* reloaded_url = model()->GetTemplateURLForKeyword(

Peter Kasting 2016/11/21 03:35:08 Nit: Prefer to linebreak after '=' rather than in

ltian 2016/11/28 22:08:02 Done.

+ ASCIIToUTF16("key1"));

+ // Times are stored with a granularity of one second, there is a loss

+ // of precision when serializing and deserializing the timestamps.

Peter Kasting 2016/11/21 03:35:08 Nit: Comment not necessary (this is just explainin

ltian 2016/11/28 22:08:02 Done.

+ AssertTimesEqual(modifed_last_visited, reloaded_url ->last_visited());

Peter Kasting 2016/11/21 03:35:08 Nit: No space before ->

ltian 2016/11/28 22:08:02 Done.