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

components/omnibox/autocomplete_match.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/omnibox/autocomplete_match.h"
 
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 413 matching lines @@
 
   // Remove the www. prefix from the host.
   static const char prefix[] = "www.";
   static const size_t prefix_len = arraysize(prefix) - 1;
   std::string host = stripped_destination_url.host();
   if (host.compare(0, prefix_len, prefix) == 0) {
     replacements.SetHostStr(base::StringPiece(host).substr(prefix_len));
     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,
                            url::Component(0, strlen(url::kHttpScheme)));
     needs_replacement = true;
   }
 
   if (needs_replacement)
     stripped_destination_url = stripped_destination_url.ReplaceComponents(
         replacements);
@@ skipping 98 matching lines @@
   return false;
 }
 
 void AutocompleteMatch::PossiblySwapContentsAndDescriptionForDisplay() {
   if (swap_contents_and_description) {
     std::swap(contents, description);
     std::swap(contents_class, description_class);
   }
 }
 
+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);
   ValidateClassifications(description, description_class);
 }
 
 void AutocompleteMatch::ValidateClassifications(
     const base::string16& text,
     const ACMatchClassifications& classifications) const {
   if (text.empty()) {
@@ skipping 18 matching lines @@
         << " is unsorted in relation to last offset of " << last_offset
         << ". Provider: " << provider_name << ".";
     DCHECK_LT(i->offset, text.length())
         << " Classification of [" << i->offset << "," << text.length()
         << "] is out of bounds for \"" << text << "\". Provider: "
         << provider_name << ".";
     last_offset = i->offset;
   }
 }
 #endif