TemplateURL Performance - don't call replace search terms

components/search_engines/template_url.cc

 // 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.
 
 #include "components/search_engines/template_url.h"
 
 #include <string>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 444 matching lines @@
   DCHECK(search_terms);
   search_terms->clear();
 
   ParseIfNecessary(search_terms_data);
 
   // We need a search term in the template URL to extract something.
   if (search_term_key_.empty() &&
       (search_term_key_location_ != url::Parsed::PATH))
     return false;
 
-  // Fill-in the replacements. We don't care about search terms in the pattern,
-  // so we use the empty string.
-  // Currently we assume the search term only shows in URL, not in post params.
-  GURL pattern(ReplaceSearchTerms(SearchTermsArgs(base::string16()),
-                                  search_terms_data, NULL));
-  // Host, path and port must match.
-  if ((url.port() != pattern.port()) ||
-      (url.host() != host_) ||
+  // Host, port, and path must match.
+  if ((url.host() != host_) ||
+      (url.port() != port_) ||
       ((url.path() != path_) &&
           (search_term_key_location_ != url::Parsed::PATH))) {
     return false;
   }
 
   std::string source;
   url::Component position;
 
   if (search_term_key_location_ == url::Parsed::PATH) {
     source = url.path();
@@ skipping 36 matching lines @@
   if (search_terms_component)
     *search_terms_component = search_term_key_location_;
   if (search_terms_position)
     *search_terms_position = position;
   return true;
 }
 
 void TemplateURLRef::InvalidateCachedValues() const {
   supports_replacements_ = valid_ = parsed_ = false;
   host_.clear();
+  port_.clear();
   path_.clear();
   search_term_key_.clear();
   search_term_position_in_path_ = std::string::npos;
   search_term_key_location_ = url::Parsed::QUERY;
   replacements_.clear();
   post_params_.clear();
 }
 
 bool TemplateURLRef::ParseParameter(size_t start,
                                     size_t end,
@@ skipping 231 matching lines @@
   base::ReplaceSubstringsAfterOffset(
       &url_string, 0, "{google:baseURL}",
       search_terms_data.GoogleBaseURLValue());
   base::ReplaceSubstringsAfterOffset(
       &url_string, 0, "{google:baseSuggestURL}",
       search_terms_data.GoogleBaseSuggestURLValue());
 
   search_term_key_.clear();
   search_term_position_in_path_ = std::string::npos;
   host_.clear();
+  port_.clear();
   path_.clear();
   search_term_key_location_ = url::Parsed::QUERY;

[Peter Kasting, 2016/01/20 01:42:56]

While you're touching this stuff, I don't think any of this block is needed.

ParseHostAndSearchTermKey() is only called one place: the end of
ParseIfNecessary().  That function, in turn, is guaranteed to have called
InvalidateCachedValues() shortly before.  So I think all these have already been
reset.

The only possible wrench here is that in between the two calls,
ParseIfNecessary() calls ParseURL(), which is non-static, and I didn't trace
deeply through that to figure out why it's non-static and what it touches.

So ideally you could verify this block can be nuked, although if you want that
could be done separately.

[Mark P, 2016/01/20 05:18:06]

I looked closer.  ParseURL() cannot be static because it calls ParseParameter(),
which uses prepopulated_ and calls GetPostParamsString(), which uses owner_.

That said, it looks safe to remove the block you suggested.  Done.

 
   GURL url(url_string);
   if (!url.is_valid())
     return;
 
   std::string query_key = FindSearchTermsKey(url.query());
   std::string ref_key = FindSearchTermsKey(url.ref());
   url::Component parameter_position;
   const bool in_query = !query_key.empty();
   const bool in_ref = !ref_key.empty();
   const bool in_path = FindSearchTermsInPath(url.path(), &parameter_position);
   if (in_query ? (in_ref || in_path) : (in_ref == in_path))
     return;  // No key or multiple keys found.  We only handle having one key.
 
   host_ = url.host();
+  port_ = url.port();
   path_ = url.path();
   if (in_query) {
     search_term_key_ = query_key;
     search_term_key_location_ = url::Parsed::QUERY;
   } else if (in_ref) {
     search_term_key_ = ref_key;
     search_term_key_location_ = url::Parsed::REF;
   } else {
     DCHECK(in_path);
     DCHECK_GE(parameter_position.begin, 1);  // Path must start with '/'.
@@ skipping 671 matching lines @@
       // patterns. This means that given patterns
       //    [ "http://foo/#q={searchTerms}", "http://foo/?q={searchTerms}" ],
       // calling ExtractSearchTermsFromURL() on "http://foo/?q=bar#q=' would
       // return false. This is important for at least Google, where such URLs
       // are invalid.
       return !search_terms->empty();
     }
   }
   return false;
 }