Cache SearchEngineType of TemplateURL

components/search_engines/template_url.h

 // Copyright 2014 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.
 
 #ifndef COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_H_
 #define COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_H_
 
 #include <stddef.h>
 
 #include <memory>
@@ skipping 626 matching lines @@
   bool HasSameKeywordAs(const TemplateURLData& other,
                         const SearchTermsData& search_terms_data) const;
 
   Type GetType() const;
 
   // Returns the id of the extension that added this search engine. Only call
   // this for TemplateURLs of type NORMAL_CONTROLLED_BY_EXTENSION or
   // 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
   // alternate URLs are listed dictates their priority, the URL at index 0 is
   // treated as the highest priority and the primary search URL is treated as
   // the lowest priority. For example, if a TemplateURL has alternate URL
   // "http://foo/#q={searchTerms}" and search URL "http://foo/?q={searchTerms}",
   // and the URL to be decoded is "http://foo/?q=a#q=b", the alternate URL will
   // match first and the decoded search term will be "b".
   bool ExtractSearchTermsFromURL(const GURL& url,
@@ skipping 84 matching lines @@
   // Points to the TemplateURLRef in |url_refs_| which corresponds to the search
   // URL.
   TemplateURLRef* url_ref_;
 
   TemplateURLRef suggestions_url_ref_;
   TemplateURLRef instant_url_ref_;
   TemplateURLRef image_url_ref_;
   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.
 
   DISALLOW_COPY_AND_ASSIGN(TemplateURL);
 };
 
 #endif  // COMPONENTS_SEARCH_ENGINES_TEMPLATE_URL_H_