Add field to monitor last visited time for each search engine

chrome/browser/search_engines/template_url_service_unittest.cc

 // 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 "components/search_engines/template_url_service.h"
 
 #include <stddef.h>
 
 #include <memory>
 #include <utility>
@@ skipping 56 matching lines @@
     const std::string& keyword,
     const std::string& url,
     const std::string& suggest_url,
     const std::string& alternate_url,
     const std::string& favicon_url,
     bool safe_for_autoreplace,
     int prepopulate_id,
     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));
   data.SetKeyword(base::UTF8ToUTF16(keyword));
   data.SetURL(url);
   data.suggestions_url = suggest_url;
   if (!alternate_url.empty())
     data.alternate_urls.push_back(alternate_url);
   data.favicon_url = GURL(favicon_url);
   data.safe_for_autoreplace = safe_for_autoreplace;
   data.prepopulate_id = prepopulate_id;
   data.input_encodings = base::SplitString(
       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);
 }
 
 TemplateURL* AddKeywordWithDate(
     TemplateURLService* model,
     const std::string& short_name,
     const std::string& keyword,
     const std::string& url,
     const std::string& suggest_url,
     const std::string& alternate_url,
     const std::string& favicon_url,
     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 check the id or any
time-related fields."

[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);
   EXPECT_EQ(expected->short_name(), actual->short_name());
   EXPECT_EQ(expected->keyword(), actual->keyword());
   EXPECT_EQ(expected->url(), actual->url());
   EXPECT_EQ(expected->suggestions_url(), actual->suggestions_url());
   EXPECT_EQ(expected->favicon_url(), actual->favicon_url());
   EXPECT_EQ(expected->alternate_urls(), actual->alternate_urls());
   EXPECT_EQ(expected->safe_for_autoreplace(), actual->safe_for_autoreplace());
@@ skipping 21 matching lines @@
 
   TemplateURL* AddKeywordWithDate(const std::string& short_name,
                                   const std::string& keyword,
                                   const std::string& url,
                                   const std::string& suggest_url,
                                   const std::string& alternate_url,
                                   const std::string& favicon_url,
                                   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);
 
   // Verifies the two timestamps are equal, within the expected degree of
   // precision.
   void AssertTimesEqual(const base::Time& expected, const base::Time& actual);
 
   // Create an URL that appears to have been prepopulated, but won't be in the
   // current data.
@@ skipping 47 matching lines @@
 TemplateURL* TemplateURLServiceTest::AddKeywordWithDate(
     const std::string& short_name,
     const std::string& keyword,
     const std::string& url,
     const std::string& suggest_url,
     const std::string& alternate_url,
     const std::string& favicon_url,
     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,
                                           const TemplateURL& actual) {
   ASSERT_EQ(expected.short_name(), actual.short_name());
   ASSERT_EQ(expected.keyword(), actual.keyword());
   ASSERT_EQ(expected.url(), actual.url());
   ASSERT_EQ(expected.suggestions_url(), actual.suggestions_url());
   ASSERT_EQ(expected.favicon_url(), actual.favicon_url());
   ASSERT_EQ(expected.alternate_urls(), actual.alternate_urls());
   ASSERT_EQ(expected.prepopulate_id(), actual.prepopulate_id());
   ASSERT_EQ(expected.safe_for_autoreplace(), actual.safe_for_autoreplace());
   ASSERT_EQ(expected.input_encodings(), actual.input_encodings());
   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());
 }
 
 void TemplateURLServiceTest::AssertTimesEqual(const base::Time& expected,
                                               const base::Time& actual) {
   // Because times are stored with a granularity of one second, there is a loss
   // of precision when serializing and deserializing the timestamps. Hence, only
   // expect timestamps to be equal to within one second of one another.
   ASSERT_LT((expected - actual).magnitude(), base::TimeDelta::FromSeconds(1));
 }
 
 std::unique_ptr<TemplateURL> TemplateURLServiceTest::CreatePreloadedTemplateURL(
     bool safe_for_autoreplace,
     int prepopulate_id) {
   TemplateURLData data;
   data.SetShortName(ASCIIToUTF16("unittest"));
   data.SetKeyword(ASCIIToUTF16("unittest"));
   data.SetURL("http://www.unittest.com/{searchTerms}");
   data.favicon_url = GURL("http://favicon.url");
   data.safe_for_autoreplace = safe_for_autoreplace;
   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);
 }
 
 void TemplateURLServiceTest::VerifyObserverCount(int expected_changed_count) {
   EXPECT_EQ(expected_changed_count, test_util_->GetObserverCount());
   test_util_->ResetObserverCount();
 }
 
 void TemplateURLServiceTest::VerifyObserverFired() {
@@ skipping 14 matching lines @@
   const size_t initial_count = model()->GetTemplateURLs().size();
 
   TemplateURLData data;
   data.SetShortName(ASCIIToUTF16("google"));
   data.SetKeyword(ASCIIToUTF16("keyword"));
   data.SetURL("http://www.google.com/foo/bar");
   data.favicon_url = GURL("http://favicon.url");
   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"),
                                                   GURL(), NULL));
   VerifyObserverCount(1);
   base::RunLoop().RunUntilIdle();
   ASSERT_EQ(initial_count + 1, model()->GetTemplateURLs().size());
   ASSERT_EQ(t_url, model()->GetTemplateURLForKeyword(t_url->keyword()));
   // We need to make a second copy as the model takes ownership of |t_url| and
   // will delete it.  We have to do this after calling Add() since that gives
@@ skipping 46 matching lines @@
   test_util()->ResetModel(true);
   ASSERT_EQ(initial_count, model()->GetTemplateURLs().size());
   EXPECT_TRUE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("b")) == NULL);
 }
 
 TEST_F(TemplateURLServiceTest, AddSameKeyword) {
   test_util()->VerifyLoad();
 
   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
   // one in the model.
   TemplateURLData data;
   data.SetShortName(ASCIIToUTF16("second"));
   data.SetKeyword(ASCIIToUTF16("keyword"));
   data.SetURL("http://test2");
   data.safe_for_autoreplace = false;
   TemplateURL* t_url = model()->Add(base::MakeUnique<TemplateURL>(data));
@@ skipping 30 matching lines @@
   EXPECT_EQ(ASCIIToUTF16("keyword"), t_url2->keyword());
   EXPECT_EQ(ASCIIToUTF16("second"), t_url->short_name());
   EXPECT_EQ(ASCIIToUTF16("test2"), t_url->keyword());
 }
 
 TEST_F(TemplateURLServiceTest, AddExtensionKeyword) {
   test_util()->VerifyLoad();
 
   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 =
       model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword3"));
   ASSERT_TRUE(original3);
 
   // Extension keywords should override replaceable keywords.
   model()->RegisterOmniboxKeyword("id1", "test", "keyword1", "http://test4");
   TemplateURL* extension1 = model()->FindTemplateURLForExtension(
       "id1", TemplateURL::OMNIBOX_API_EXTENSION);
@@ skipping 25 matching lines @@
   // replaceable TemplateURL.  We should still do correct conflict resolution
   // between the non-template URLs.
   model()->RegisterOmniboxKeyword("test2", "extension", "keyword",
                                   "http://test2");
   TemplateURL* extension =
       model()->GetTemplateURLForKeyword(ASCIIToUTF16("keyword"));
   ASSERT_TRUE(extension);
   // 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.
   TemplateURLData data;
   data.SetShortName(ASCIIToUTF16("name1"));
   data.SetKeyword(ASCIIToUTF16("keyword"));
   data.SetURL("http://test3");
   data.safe_for_autoreplace = true;
   TemplateURL* t_url = model()->Add(base::MakeUnique<TemplateURL>(data));
   EXPECT_EQ(extension,
@@ skipping 30 matching lines @@
   Time now = Time::Now();
   TimeDelta one_day = TimeDelta::FromDays(1);
   Time month_ago = now - TimeDelta::FromDays(30);
 
   // Nothing has been added.
   EXPECT_EQ(0U, model()->GetTemplateURLs().size());
 
   // 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());
 
   // Try removing from current timestamp. This should delete the one in the
   // future and one very recent one.
   model()->RemoveAutoGeneratedSince(now);
   EXPECT_EQ(4U, model()->GetTemplateURLs().size());
 
   // Try removing from two months ago. This should only delete items that are
@@ skipping 26 matching lines @@
   Time now = Time::Now();
   TimeDelta one_day = TimeDelta::FromDays(1);
   Time month_ago = now - TimeDelta::FromDays(30);
 
   // Nothing has been added.
   EXPECT_EQ(0U, model()->GetTemplateURLs().size());
 
   // 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());
 
   // Try removing foo2. This should delete foo2, but leave foo1 and 3 untouched.
   GURL url2("http://foo2");
   model()->RemoveAutoGeneratedForUrlsBetween(
       base::Bind(static_cast<bool (*)(const GURL&, const GURL&)>(operator==),
                  url2),
       month_ago, now + one_day);
@@ skipping 31 matching lines @@
   // Add a new TemplateURL.
   test_util()->VerifyLoad();
   const size_t initial_count = model()->GetTemplateURLs().size();
   TemplateURLData data;
   data.SetShortName(ASCIIToUTF16("google"));
   data.SetKeyword(ASCIIToUTF16("keyword"));
   data.SetURL("http://www.google.com/foo/bar");
   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);
   base::RunLoop().RunUntilIdle();
 
   base::Time now = base::Time::Now();
   std::unique_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
   clock->SetNow(now);
   model()->set_clock(std::move(clock));
 
@@ skipping 22 matching lines @@
   AssertEquals(*cloned_url, *read_url);
   AssertTimesEqual(now, read_url->last_modified());
 }
 
 TEST_F(TemplateURLServiceTest, DefaultSearchProvider) {
   // Add a new TemplateURL.
   test_util()->VerifyLoad();
   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);
   ASSERT_EQ(t_url, model()->GetDefaultSearchProvider());
   ASSERT_TRUE(t_url->safe_for_autoreplace());
   ASSERT_TRUE(model()->ShowInDefaultList(t_url));
 
   // Setting the default search provider should have caused notification.
   VerifyObserverCount(1);
   base::RunLoop().RunUntilIdle();
 
   std::unique_ptr<TemplateURL> cloned_url(
       base::MakeUnique<TemplateURL>(t_url->data()));
 
   // Make sure when we reload we get a default search provider.
   test_util()->ResetModel(true);
   EXPECT_EQ(initial_count + 1, model()->GetTemplateURLs().size());
   ASSERT_TRUE(model()->GetDefaultSearchProvider());
   AssertEquals(*cloned_url, *model()->GetDefaultSearchProvider());
 }
 
 TEST_F(TemplateURLServiceTest, CantReplaceWithSameKeyword) {
   test_util()->ChangeModelToLoadState();
   ASSERT_TRUE(model()->CanAddAutogeneratedKeyword(ASCIIToUTF16("foo"), GURL(),
                                                   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"),
                                          GURL("http://foo2"), NULL));
 
   // ResetTemplateURL marks the TemplateURL as unsafe to replace, so it should
   // no longer be replaceable.
   model()->ResetTemplateURL(t_url, t_url->short_name(), t_url->keyword(),
                             t_url->url());
 
   ASSERT_FALSE(model()->CanAddAutogeneratedKeyword(ASCIIToUTF16("foo"),
                                           GURL("http://foo2"), NULL));
 }
 
 TEST_F(TemplateURLServiceTest, CantReplaceWithSameHosts) {
   test_util()->ChangeModelToLoadState();
   ASSERT_TRUE(model()->CanAddAutogeneratedKeyword(ASCIIToUTF16("foo"),
                                          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"),
                                          GURL("http://foo.com"), NULL));
 
   // ResetTemplateURL marks the TemplateURL as unsafe to replace, so it should
   // no longer be replaceable.
   model()->ResetTemplateURL(t_url, t_url->short_name(), t_url->keyword(),
                             t_url->url());
 
@@ skipping 15 matching lines @@
   test_util()->VerifyLoad();
 
   TemplateURLData data;
   data.SetShortName(ASCIIToUTF16("a"));
   data.safe_for_autoreplace = true;
   data.SetURL("http://url/{searchTerms}");
   data.suggestions_url = "http://url2";
   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();
 
   model()->SetUserSelectedDefaultSearchProvider(t_url);
   base::RunLoop().RunUntilIdle();
   std::unique_ptr<TemplateURL> cloned_url(
       base::MakeUnique<TemplateURL>(t_url->data()));
 
   // Reset the model and don't load it. The template url we set as the default
   // should be pulled from prefs now.
@@ skipping 25 matching lines @@
   ASSERT_TRUE(google);
   model()->ResetTemplateURL(google, ASCIIToUTF16("trash"), ASCIIToUTF16("xxx"),
                             "http://www.foo.com/s?q={searchTerms}");
   EXPECT_EQ(ASCIIToUTF16("trash"), google->short_name());
   EXPECT_EQ(ASCIIToUTF16("xxx"), google->keyword());
 
   // Add third-party default search engine.
   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());
 
   // Remove bing.
   TemplateURL* bing = model()->GetTemplateURLForKeyword(
       ASCIIToUTF16("bing.com"));
   ASSERT_TRUE(bing);
   model()->Remove(bing);
   EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("bing.com")));
 
@@ skipping 75 matching lines @@
     { "http://x/foo?q=b&q=xx", base::string16() },
     { "http://x/foo#query=xx", ASCIIToUTF16("xx") },
     { "http://x/foo?q=b#query=xx", ASCIIToUTF16("xx") },
     { "http://x/foo?q=b#q=xx", ASCIIToUTF16("b") },
     { "http://x/foo?query=b#q=xx", base::string16() },
   };
 
   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 format before uploading can help with this)

[ltian, 2016/11/28 22:08:02]

Done.

 
   for (size_t i = 0; i < arraysize(data); ++i) {
     TemplateURLService::URLVisitedDetails details = {
       GURL(data[i].url), false
     };
     model()->UpdateKeywordSearchTermsForURL(details);
     EXPECT_EQ(data[i].term, test_util()->GetAndClearSearchTerm());
   }
 }
 
 TEST_F(TemplateURLServiceTest, DontUpdateKeywordSearchForNonReplaceable) {
   struct TestData {
     const std::string url;
   } data[] = {
     { "http://foo/" },
     { "http://x/bar?q=xx" },
     { "http://x/foo?y=xx" },
   };
 
   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 = {
       GURL(data[i].url), false
     };
     model()->UpdateKeywordSearchTermsForURL(details);
     ASSERT_EQ(base::string16(), test_util()->GetAndClearSearchTerm());
   }
 }
 
 TEST_F(TemplateURLServiceWithoutFallbackTest, ChangeGoogleBaseValue) {
   // NOTE: Do not load the prepopulate data, which also has a {google:baseURL}
   // keyword in it and would confuse this test.
   test_util()->ChangeModelToLoadState();
 
   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());
 
   // Change the Google base url.
   test_util()->ResetObserverCount();
   test_util()->SetGoogleBaseURL(GURL("http://google.co.uk/"));
   VerifyObserverCount(1);
 
   // Make sure the host->TemplateURL map was updated appropriately.
   ASSERT_EQ(t_url, model()->GetTemplateURLForHost("google.co.uk"));
   EXPECT_TRUE(model()->GetTemplateURLForHost("google.com") == NULL);
   EXPECT_EQ("google.co.uk", t_url->url_ref().GetHost(search_terms_data()));
   EXPECT_EQ(ASCIIToUTF16("google.co.uk"), t_url->keyword());
   EXPECT_EQ("http://google.co.uk/?q=x", t_url->url_ref().ReplaceSearchTerms(
       TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("x")), search_terms_data()));
 
   // Now add a manual entry and then change the Google base URL such that the
   // autogenerated Google search keyword would conflict.
   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
   // has not changed.
   ASSERT_EQ(manual,
             model()->GetTemplateURLForKeyword(ASCIIToUTF16("google.de")));
   EXPECT_EQ("google.de", manual->url_ref().GetHost(search_terms_data()));
   ASSERT_EQ(t_url,
             model()->GetTemplateURLForKeyword(ASCIIToUTF16("google.co.uk")));
   EXPECT_EQ("google.de", t_url->url_ref().GetHost(search_terms_data()));
@@ skipping 16 matching lines @@
 // KEYWORD visits.
 TEST_F(TemplateURLServiceTest, GenerateVisitOnKeyword) {
   test_util()->profile()->CreateBookmarkModel(false);
   ASSERT_TRUE(test_util()->profile()->CreateHistoryService(true, false));
   test_util()->ResetModel(true);
 
   // Create a keyword.
   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()?

Because if not, then I think otherwise we basically always pass Time() for this
arg, and we could just eliminate it from both AddKeywordWithDate() and
CreateKeywordWithDate().

[ltian, 2016/11/28 22:08:02]

Done.

 
   // Add a visit that matches the url of the keyword.
   history::HistoryService* history = HistoryServiceFactory::GetForProfile(
       test_util()->profile(), ServiceAccessType::EXPLICIT_ACCESS);
   history->AddPage(
       GURL(t_url->url_ref().ReplaceSearchTerms(
           TemplateURLRef::SearchTermsArgs(ASCIIToUTF16("blah")),
           search_terms_data())),
       base::Time::Now(), NULL, 0, GURL(), history::RedirectList(),
       ui::PAGE_TRANSITION_KEYWORD, history::SOURCE_BROWSED, false);
@@ skipping 190 matching lines @@
 // the default search as managed.  Also check that we are getting the right
 // values.
 TEST_F(TemplateURLServiceTest, TestManagedDefaultSearch) {
   test_util()->VerifyLoad();
   const size_t initial_count = model()->GetTemplateURLs().size();
   test_util()->ResetObserverCount();
 
   // 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
   // notifications.
   VerifyObserverCount(1);
   EXPECT_FALSE(model()->is_default_search_managed());
   EXPECT_EQ(initial_count + 1, model()->GetTemplateURLs().size());
 
   // Set a managed preference that establishes a default search provider.
   const char kName[] = "test1";
@@ skipping 175 matching lines @@
   ASSERT_FALSE(loaded_url == NULL);
   EXPECT_EQ(4U, loaded_url->input_encodings().size());
 }
 
 TEST_F(TemplateURLServiceTest, DefaultExtensionEngine) {
   test_util()->VerifyLoad();
   // Add third-party default search engine.
   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;
   TemplateURL* ext_dse_ptr = model()->AddExtensionControlledTURL(
       std::move(ext_dse), std::move(extension_info));
   EXPECT_EQ(ext_dse_ptr, model()->GetDefaultSearchProvider());
 
   model()->RemoveExtensionControlledTURL(
       "ext", TemplateURL::NORMAL_CONTROLLED_BY_EXTENSION);
   ExpectSimilar(user_dse, model()->GetDefaultSearchProvider());
 }
 
 TEST_F(TemplateURLServiceTest, ExtensionEnginesNotPersist) {
   test_util()->VerifyLoad();
   // Add third-party default search engine.
   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;
   model()->AddExtensionControlledTURL(std::move(ext_dse),
                                       std::move(extension_info));
   EXPECT_EQ(user_dse, model()->GetDefaultSearchProvider());
 
   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(
       std::move(ext_dse), std::move(extension_info));
   EXPECT_EQ(ext_dse_ptr, model()->GetDefaultSearchProvider());
 
   test_util()->ResetModel(true);
   user_dse = model()->GetTemplateURLForKeyword(ASCIIToUTF16("user"));
   ExpectSimilar(user_dse, model()->GetDefaultSearchProvider());
   EXPECT_FALSE(model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext1")));
@@ skipping 25 matching lines @@
   data.search_terms_replacement_key = kSearchTermsReplacementKey;
   std::unique_ptr<TemplateURL> expected_managed_default(new TemplateURL(data));
   EXPECT_TRUE(model()->is_default_search_managed());
   const TemplateURL* actual_managed_default =
       model()->GetDefaultSearchProvider();
   ExpectSimilar(expected_managed_default.get(), actual_managed_default);
 
   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;
   TemplateURL* ext_dse_ptr = model()->AddExtensionControlledTURL(
       std::move(ext_dse), std::move(extension_info));
   EXPECT_EQ(ext_dse_ptr,
             model()->GetTemplateURLForKeyword(ASCIIToUTF16("ext1")));
   EXPECT_TRUE(model()->is_default_search_managed());
   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 could be inlined into the next
statement, although feel free not to do this if it decreases readability overall
(2 places)

[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 wrapping, but about replacing
the lone use of |modified_url| in the subsequent statement with this expression
that's used to initialize it.

+      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 the midst of the parameter
list (I suspect git cl format would do this too, not sure)

[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 explaining AssertTimesEqual() itself)

[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.

+
+}