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

components/omnibox/autocomplete_match.cc

Index: components/omnibox/autocomplete_match.cc
diff --git a/components/omnibox/autocomplete_match.cc b/components/omnibox/autocomplete_match.cc
index 74d341071e3528f97f8f389b6df697244e261856..7690340ed4ea22aea1d240596d088199794c42f3 100644
--- a/components/omnibox/autocomplete_match.cc
+++ b/components/omnibox/autocomplete_match.cc
@@ -431,6 +431,21 @@ GURL AutocompleteMatch::GURLToStrippedGURL(
     needs_replacement = true;
   }
 
+  // Remove any trailing slash (if it's not a lone slash), or add a slash (to
+  // make a lone slash) if the path is empty.  (We can't unconditionally
+  // remove even lone slashes because for some schemes the path must consist
+  // of at least a slash.)
+  const std::string& path = stripped_destination_url.path();
+  if ((path.length() > 1) && (path[path.length() - 1] == '/')) {
+    replacements.SetPathStr(
+        base::StringPiece(path).substr(0, path.length() - 1));
+    needs_replacement = true;
+  } else if (path.empty()) {
+    static const char slash[] = "/";
+    replacements.SetPathStr(base::StringPiece(slash));
+    needs_replacement = true;
+  }
+
   // Replace https protocol with http protocol.
   if (stripped_destination_url.SchemeIs(url::kHttpsScheme)) {
     replacements.SetScheme(url::kHttpScheme,
@@ -549,6 +564,15 @@ void AutocompleteMatch::PossiblySwapContentsAndDescriptionForDisplay() {
   }
 }
 
+void AutocompleteMatch::StripLoneSlashOnInlineAutocompletion() {
+  DCHECK(!IsSearchType(type));
+  static const base::char16 slash_utf8 = 0x002F;

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

Why declare a variable for this?  And use 0x002f instead of '/', which is
equivalent?

It seems more readable to just use '/' directly in the conditional below, and I
don't think there should be a problem with char16 vs. char type mismatches,
we'll just auto-promote the char.

+  if ((inline_autocompletion.length() == 1) &&

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

I know you may argue that we should simply remove a trailing slash, not merely
lone slashes.  However, I sometimes find the trailing slash suggestions useful;
with them, I can type "www", see www.host.com", press the right arrow to accept
the text, type "/f", see the completion to www.host.com/foo/, press the right
arrow to accept the text, type "b", see the completion to www.host.com/foo/bar,
and press enter.  If we stripped trailing slashes, I'd have to type "/b" instead
of "b".  You may argue for consistency that we should remove the trailing
slashes everywhere, but I think I tend to believe this efficiency argument is
slightly stronger, and believe no one will complain that as they type they see
the trailing slash until they type the last character before the slash.

+      (inline_autocompletion[0] == slash_utf8)) {

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

Nit: No {}

+    inline_autocompletion.clear();
+  }
+}
+
 #ifndef NDEBUG
 void AutocompleteMatch::Validate() const {
   ValidateClassifications(contents, contents_class);