Add field to monitor last visited time for each search engine

components/search_engines/template_url_service.cc

Index: components/search_engines/template_url_service.cc
diff --git a/components/search_engines/template_url_service.cc b/components/search_engines/template_url_service.cc
index 664fa3390a3a2ec1775d070a925493347201018e..fcafca5dc8393dc6973fa37f804a33f19bfd83b3 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -725,6 +725,15 @@ void TemplateURLService::RepairPrepopulatedSearchEngines() {
   }
 }
 
+bool TemplateURLService::UpdateTemplateURLVisitTime(TemplateURL* url) {
+  TemplateURLData data(url->data());
+  data.last_visited = clock_->Now();
+  const bool updated = UpdateNoNotify(url, TemplateURL(data));
+  if (updated)
+    NotifyObservers();
+  return updated;
+}
+
 void TemplateURLService::AddObserver(TemplateURLServiceObserver* observer) {
   model_observers_.AddObserver(observer);
 }
@@ -1777,6 +1786,7 @@ void TemplateURLService::UpdateKeywordSearchTermsForURL(
   if (!urls_for_host)
     return;
 
+  TemplateURL* visited_url = nullptr;
   for (TemplateURLSet::const_iterator i = urls_for_host->begin();
        i != urls_for_host->end(); ++i) {
     base::string16 search_terms;
@@ -1793,8 +1803,12 @@ void TemplateURLService::UpdateKeywordSearchTermsForURL(
         client_->SetKeywordSearchTermsForURL(
             details.url, (*i)->id(), search_terms);
       }
+      visited_url = *i;

[Peter Kasting, 2016/12/01 07:38:24]

Why copy this into a temp and then make the call below?  Why not just do that
directly here?

[ltian, 2016/12/01 10:02:58]

Directly call it here will cause the error "attempt to increment a singular
iterator." which seems mean the container of iterator is modified (maybe by
UpdateNoNotify()?). Caching it and calling it later could avoid this problem. 

[Peter Kasting, 2016/12/01 19:32:35]

I understand this now.  You're correct that ultimately UpdateNoNotify will ask
the host-to-url map to remove an entry, which will modify the container as we
touch it.

This can be fixed in a way that will save underlying work.  If
UpdateTemplateURLVisitTime() just changes the last visited time directly
(instead of using UpdateNoNotify()), you should be fine putting that call here. 
This is legal because TemplateURLService is a friend of TemplateURL, and safe
because you're not touching any fields that require the map updates
UpdateNoNotify() performs.

     }
   }
+  if (visited_url != nullptr) {

[Peter Kasting, 2016/12/01 07:38:25]

Nit: No {}

I would elide the "!= nullptr" as well.

[ltian, 2016/12/01 10:02:58]

Done.

+    UpdateTemplateURLVisitTime(visited_url);
+  }
 }
 
 void TemplateURLService::AddTabToSearchVisit(const TemplateURL& t_url) {