Reland - Omnibox - Mark As Duplicates URLs that only differ by a trailing slash

components/omnibox/autocomplete_match.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_OMNIBOX_AUTOCOMPLETE_MATCH_H_
 #define COMPONENTS_OMNIBOX_AUTOCOMPLETE_MATCH_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 161 matching lines @@
   // A static version GetTemplateURL() that takes the match's keyword and
   // match's hostname as parameters.  In short, returns the TemplateURL
   // associated with |keyword| if it exists; otherwise returns the TemplateURL
   // associated with |host| if it exists.
   static TemplateURL* GetTemplateURLWithKeyword(
       TemplateURLService* template_url_service,
       const base::string16& keyword,
       const std::string& host);
 
   // Returns |url| altered by stripping off "www.", converting https protocol
-  // to http, and stripping excess query parameters.  These conversions are
-  // merely to allow comparisons to remove likely duplicates; these URLs are
-  // not used as actual destination URLs.  If |template_url_service| is not
-  // NULL, it is used to get a template URL corresponding to this match.  If
-  // the match's keyword is known, it can be passed in.  Otherwise, it can be
-  // left empty and the template URL (if any) is determined from the
-  // destination's hostname.  The template URL is used to strip off query args
-  // other than the search terms themselves that would otherwise prevent doing
-  // proper deduping.
+  // to http, normalizing trailing slashes, and stripping excess query
+  // parameters.  These conversions are merely to allow comparisons to remove
+  // likely duplicates; these URLs are not used as actual destination URLs.
+  // If |template_url_service| is not NULL, it is used to get a template URL
+  // corresponding to this match.  If the match's keyword is known, it can be
+  // passed in.  Otherwise, it can be left empty and the template URL (if any)
+  // is determined from the destination's hostname.  The template URL is used
+  // to strip off query args other than the search terms themselves that would
+  // otherwise prevent doing proper deduping.
   static GURL GURLToStrippedGURL(const GURL& url,
                                  TemplateURLService* template_url_service,
                                  const base::string16& keyword);
 
   // Computes the stripped destination URL (via GURLToStrippedGURL()) and
   // stores the result in |stripped_destination_url|.
   void ComputeStrippedDestinationURL(TemplateURLService* template_url_service);
 
   // Sets |allowed_to_be_default_match| to true if this match is effectively
   // the URL-what-you-typed match (i.e., would be dupped against the UWYT
@@ skipping 64 matching lines @@
 
   // Swaps the contents and description fields, and their associated
   // classifications, if this is a match for which we should emphasize the
   // title (stored in the description field) over the URL (in the contents
   // field).  Intended to only be used at the UI level before displaying, lest
   // other omnibox systems get confused about which is which.  See the code
   // that sets |swap_contents_and_description| for conditions under which
   // it is true.
   void PossiblySwapContentsAndDescriptionForDisplay();
 
+  // If |inline_autocompletion| is "/", clears the inline autocompletion.
+  // Inline autocompletions of "/" have extremely limited utility because
+  // the omnibox treats two URLs that only differ by whether they end in a
+  // slash as duplicates.  This means that one can type one of those URLs
+  // and hit enter and end up navigating to the other depending on which
+  // URL suggestion scores better.  Displaying an inline autocompletion
+  // of a slash isn't very useful in this context and can cause issues if the

[Peter Kasting, 2015/07/01 22:01:36]

I would stop after "isn't very useful in this context".  I think that's a
sufficiently good argument for this, and the further arguments you make apply
equally to any inline autocompletion, not just to trailing slashes, so they just
raise the question for the reader of what's different about this case.

+  // user made a typo and wants to backspace.  With an inline autocompletion
+  // of a slash, the user first has to delete the completion and then delete
+  // the typed text to correct it, taking longer than if the completion was
+  // not there.  Hence, it's not worth displaying the inline autocompletion
+  // in this context.  Intended to be used for URL suggestions.
+  void StripLoneSlashOnInlineAutocompletion();

[Mark P, 2015/06/30 23:47:30]

I know this function is trivial and could be inlined in the places that call it.
 However, I want a common place for a comment arguing why this is the right
thing to do.  This is a decent place for it.

+
   // 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).
   AutocompleteProvider* provider;
 
   // The relevance of this match. See table in autocomplete.h for scores
   // returned by various providers. This is used to rank matches among all
   // responding providers, so different providers must be carefully tuned to
   // supply matches with appropriate relevance.
   //
@@ skipping 28 matching lines @@
   // should only set this flag if ".com" will be inline autocompleted;
   // and a navigation to "foo/" (an intranet host) or search for "foo"
   // should set this flag.
   bool allowed_to_be_default_match;
 
   // The URL to actually load when the autocomplete item is selected. This URL
   // should be canonical so we can compare URLs with strcmp to avoid dupes.
   // It may be empty if there is no possible navigation.
   GURL destination_url;
 
-  // The destination URL with "www." stripped off for better dupe finding.
+  // The destination URL, somewhat normalized for better dupe finding.
   GURL stripped_destination_url;
 
   // The main text displayed in the address bar dropdown.
   base::string16 contents;
   ACMatchClassifications contents_class;
 
   // Additional helper text for each entry, such as a title or description.
   base::string16 description;
   ACMatchClassifications description_class;
 
@@ skipping 68 matching lines @@
       const base::string16& text,
       const ACMatchClassifications& classifications) const;
 #endif
 };
 
 typedef AutocompleteMatch::ACMatchClassification ACMatchClassification;
 typedef std::vector<ACMatchClassification> ACMatchClassifications;
 typedef std::vector<AutocompleteMatch> ACMatches;
 
 #endif  // COMPONENTS_OMNIBOX_AUTOCOMPLETE_MATCH_H_