Cache SearchEngineType of TemplateURL

components/search_engines/template_url.h

Index: components/search_engines/template_url.h
diff --git a/components/search_engines/template_url.h b/components/search_engines/template_url.h
index 2acb146851887d0f8246dbf56c4b60071374d097..6063e8b6efab3ba694a63955d3175bf1164b52d3 100644
--- a/components/search_engines/template_url.h
+++ b/components/search_engines/template_url.h
@@ -644,6 +644,11 @@ class TemplateURL {
   // OMNIBOX_API_EXTENSION.
   std::string GetExtensionId() const;
 
+  // Returns the type of the provided engine, or SEARCH_ENGINE_OTHER if no

[Peter Kasting, 2016/05/17 04:43:04]

"the provided engine" is meaningless since this doesn't take an engine.

[Vitaly Baranov, 2016/06/03 15:31:45]

Done.

+  // engines match.
+  SearchEngineType GetEngineType(
+      const SearchTermsData& search_terms_data) const;
+
   // Use the alternate URLs and the search URL to match the provided |url|
   // and extract |search_terms| from it. Returns false and an empty
   // |search_terms| if no search terms can be matched. The order in which the
@@ -748,6 +753,7 @@ class TemplateURL {
   TemplateURLRef new_tab_url_ref_;
   TemplateURLRef contextual_search_url_ref_;
   std::unique_ptr<AssociatedExtensionInfo> extension_info_;
+  mutable SearchEngineType engine_type_;

[Peter Kasting, 2016/05/17 04:43:04]

I think it's confusing for there to be an engine_type_ field in both TemplateURL
and TemplateURLData.  I understand what you're using them for (the "cached" vs.
the "prepopulated, if any" value), but this doesn't really match how the rest of
the TemplateURL[Data] code works.  I think instead, TemplateURLData should have
the only copy of this info.

We shouldn't reset its value in InvalidateCachedValues(), either, since that
gets called for cases that shouldn't actually affect the engine type.  Part of
the issue here is whether it's important that this not be computed except on
demand.  If a single computation is cheap enough, or we ask for engine types
frequently enough that we'll need to compute them for every TemplateURL anyway,
then instead of setting this on demand, we can set it when the computed value
would actually change, e.g. in SetURL() and at construction.

[Vitaly Baranov, 2016/06/03 15:31:45]

Done. And field TemplateURLData::engine_type_ is mutable now to keep
TemplateURL::GetEngineType() a constant function.
Done.
A single computation costs less than a millisecond however we cannot easily
perform it in TemplateURLData::SetURL() because we need SearchTermsData for
this.

 
   // TODO(sky): Add date last parsed OSD file.