Initial implementation of dedupping search provider's URLs.

chrome/browser/autocomplete/autocomplete_match.h

 // 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.
 
 #ifndef CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_MATCH_H_
 #define CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_MATCH_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 170 matching lines @@
   // Convenience function to check if |type| is a search (as opposed to a URL or
   // an extension).
   static bool IsSearchType(Type type);
 
   // Copies the destination_url with "www." stripped off to
   // |stripped_destination_url| and also converts https protocol to
   // http.  These two conversions are merely to allow comparisons to
   // remove likely duplicates; these URLs are not used as actual
   // destination URLs.  This method is invoked internally by the
   // AutocompleteResult and does not normally need to be invoked.
-  void ComputeStrippedDestinationURL();
+  // If |profile| is not NULL, it is used to get a template URL corresponding
+  // to this match.  The template is used to strip off time/position sensitive
+  // data that would otherwise prevent from proper dedupping.

[Peter Kasting, 2012/10/26 22:13:49]

Nit: dedupping -> deduping (and in other files)

I'd also replace "time/position sensitive data" with "query args other than the
search terms themselves" since that's more accurate (even though the prior
phrase is the core motivation here).

Finally, if we need to pass a Profile here and in GetTemplateURL(), we should
consider simply forcing AutocompleteMatch() to take a Profile* and store it as a
member.  Optionally, perhaps the right fix is to leave these signatures but make
AutocompleteResult take a Profile*?  I'm not sure without trying both.

It would be fine to do this as a follow up.

[Bart N., 2012/10/26 23:36:55]

Done.
Yes, I'll have a follow-up CL to support proper destination URL normalization.
Deferred.

+  void ComputeStrippedDestinationURL(Profile* profile);
 
   // Gets data relevant to whether there should be any special keyword-related
   // UI shown for this match.  If this match represents a selected keyword, i.e.
   // the UI should be "in keyword mode", |keyword| will be set to the keyword
   // and |is_keyword_hint| will be set to false.  If this match has a non-NULL
   // |associated_keyword|, i.e. we should show a "Press [tab] to search ___"
   // hint and allow the user to toggle into keyword mode, |keyword| will be set
   // to the associated keyword and |is_keyword_hint| will be set to true.  Note
   // that only one of these states can be in effect at once.  In all other
   // cases, |keyword| will be cleared, even when our member variable |keyword|
   // is non-empty -- such as with non-substituting keywords or matches that
   // represent searches using the default search engine.  See also
   // GetSubstitutingExplicitlyInvokedKeyword().
   void GetKeywordUIState(Profile* profile,
                          string16* keyword,
                          bool* is_keyword_hint) const;
 
   // Returns |keyword|, but only if it represents a substituting keyword that
   // the user has explicitly invoked.  If for example this match represents a
   // search with the default search engine (and the user didn't explicitly
   // invoke its keyword), this returns the empty string.  The result is that
   // this function returns a non-empty string in the same cases as when the UI
   // should show up as being "in keyword mode".
   string16 GetSubstitutingExplicitlyInvokedKeyword(Profile* profile) const;
 
   // Returns the TemplateURL associated with this match.  This may be NULL if
   // the match has no keyword OR if the keyword no longer corresponds to a valid
   // TemplateURL.  See comments on |keyword| below.
+  // Simply delegates to GetTemplateURL(profile, false) below.

[Peter Kasting, 2012/10/26 22:13:49]

Nit: Having two functions here is just another way of writing default arguments,
which are banned by the style guide.  Remove this function and make all callers
use the two-arg form.

[Bart N., 2012/10/26 23:36:55]

Done.

   TemplateURL* GetTemplateURL(Profile* profile) const;
 
+  // Same as above except it may peform additional template lookup using
+  // |destination_url|'s host if |check_host| is true.

[Peter Kasting, 2012/10/26 22:13:49]

Nit: Be more clear.  Something like "If |use_host| is true and the keyword does
not map to a valid TemplateURL, we'll then check for a TemplateURL that
corresponds to the destination_url's hostname."  You may also want to rename the
arg |allow_fallback_to_destination_host| or something.

[Bart N., 2012/10/26 23:36:55]

Done.

+  TemplateURL* GetTemplateURL(Profile* profile, bool use_host) const;
+
   // Adds optional information to the |additional_info| dictionary.
   void RecordAdditionalInfo(const std::string& property,
                             const std::string& value);
   void RecordAdditionalInfo(const std::string& property, int value);
   void RecordAdditionalInfo(const std::string& property,
                             const base::Time& value);
 
   // The provider of this match, used to remember which provider the user had
   // selected when the input changes. This may be NULL, in which case there is
   // no provider (or memory of the user's selection).
@@ skipping 101 matching lines @@
       const string16& text,
       const ACMatchClassifications& classifications) const;
 #endif
 };
 
 typedef AutocompleteMatch::ACMatchClassification ACMatchClassification;
 typedef std::vector<ACMatchClassification> ACMatchClassifications;
 typedef std::vector<AutocompleteMatch> ACMatches;
 
 #endif  // CHROME_BROWSER_AUTOCOMPLETE_AUTOCOMPLETE_MATCH_H_