Omnibox UI Experiments: Implement elide-after-host experiment.

components/url_formatter/url_formatter.cc

 // Copyright 2015 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/url_formatter/url_formatter.h"
 
 #include <algorithm>
 #include <utility>
 #include <vector>
 
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_offset_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_local_storage.h"
 #include "components/url_formatter/idn_spoof_checker.h"
 #include "third_party/icu/source/common/unicode/uidna.h"
 #include "third_party/icu/source/common/unicode/utypes.h"
+#include "ui/gfx/text_elider.h"
 #include "url/gurl.h"
 #include "url/third_party/mozilla/url_parse.h"
 
 namespace url_formatter {
 
 namespace {
 
 base::string16 IDNToUnicodeWithAdjustments(
     base::StringPiece host,
     base::OffsetAdjuster::Adjustments* adjustments);
@@ skipping 321 matching lines @@
 
 }  // namespace
 
 const FormatUrlType kFormatUrlOmitNothing = 0;
 const FormatUrlType kFormatUrlOmitUsernamePassword = 1 << 0;
 const FormatUrlType kFormatUrlOmitHTTP = 1 << 1;
 const FormatUrlType kFormatUrlOmitTrailingSlashOnBareHostname = 1 << 2;
 const FormatUrlType kFormatUrlOmitAll =
     kFormatUrlOmitUsernamePassword | kFormatUrlOmitHTTP |
     kFormatUrlOmitTrailingSlashOnBareHostname;
+const FormatUrlType kFormatUrlExperimentalEllipsizePath = 1 << 3;
 
 base::string16 FormatUrl(const GURL& url,
                          FormatUrlTypes format_types,
                          net::UnescapeRule::Type unescape_rules,
                          url::Parsed* new_parsed,
                          size_t* prefix_end,
                          size_t* offset_for_adjustment) {
   std::vector<size_t> offsets;
   if (offset_for_adjustment)
     offsets.push_back(*offset_for_adjustment);
@@ skipping 126 matching lines @@
     url_string.push_back(':');
     new_parsed->port.begin = url_string.length();
     url_string.insert(url_string.end(), spec.begin() + parsed.port.begin,
                       spec.begin() + parsed.port.end());
     new_parsed->port.len = url_string.length() - new_parsed->port.begin;
   } else {
     new_parsed->port.reset();
   }
 
   // Path & query.  Both get the same general unescape & convert treatment.
-  if (!(format_types & kFormatUrlOmitTrailingSlashOnBareHostname) ||
-      !CanStripTrailingSlash(url)) {
-    AppendFormattedComponent(spec, parsed.path,
-                             NonHostComponentTransform(unescape_rules),
-                             &url_string, &new_parsed->path, adjustments);
-  } else {
+  if ((format_types & kFormatUrlOmitTrailingSlashOnBareHostname) &&
+      CanStripTrailingSlash(url)) {
+    // Case when it's just a single trailing slash we can strip.

[Peter Kasting, 2017/06/28 20:54:34]

Nit: "Omit the path, which is a single trailing slash.  There is no query or
ref."

[tommycli, 2017/06/28 21:21:39]

Done.

     if (parsed.path.len > 0) {
       adjustments->push_back(base::OffsetAdjuster::Adjustment(
           parsed.path.begin, parsed.path.len, 0));
     }
+  } else if ((format_types & kFormatUrlExperimentalEllipsizePath) &&
+             url.IsStandard() && !url.SchemeIsFile() &&
+             !url.SchemeIsFileSystem()) {
+    // Case when we replace the path with an ellipsis.

[Peter Kasting, 2017/06/28 20:54:34]

Nit: "Replace everything after the host with a slash + ellipsis."

[tommycli, 2017/06/28 21:21:39]

Done.

+    url_string.push_back('/');
+    url_string.append(gfx::kEllipsisUTF16);
+
+    // Start at begin + 1, since we are preserving the first forward slash.
+    // Subtract one from the length to account for the first forward slash.
+    // New length is one to account for the new ellipsis character.

[Peter Kasting, 2017/06/28 20:54:34]

Nit: These comments actually refer, respectively, to line 534, 528, and 534.  I
had a really hard time understanding this reading them.  How about spreading
them out?  I also tried to explain why I thought you added "+1"s on the
query/ref, correct me if I'm wrong.

    // Compute the length of everything we're replacing.  For the path, we're
    // removing all but the first slash.
    size_t old_length = parsed.path.len - 1;

    // We're also removing any query, plus the delimiting '?'.
    if (parsed.query.is_valid())
      old_length += parsed.query.len + 1;

    // And any ref, plus delimiting '#'.
    if (parsed.ref.is_valid())
      old_length += parsed.ref.len + 1;

    // We're replacing all these with a single character (an ellipsis).
    adjustments->push_back(
        base::OffsetAdjuster::Adjustment(parsed.path.begin + 1, old_length, 1));

[tommycli, 2017/06/28 21:21:39]

Done. This makes it make much more sense!

+    size_t old_length = parsed.path.len - 1;
+    if (parsed.query.is_valid())
+      old_length += parsed.query.len + 1;
+    if (parsed.ref.is_valid())
+      old_length += parsed.ref.len + 1;
+    adjustments->push_back(
+        base::OffsetAdjuster::Adjustment(parsed.path.begin + 1, old_length, 1));
+  } else {
+    // Default case.

[Peter Kasting, 2017/06/28 20:54:34]

Nit: "Append formatted path, query, and ref."

[tommycli, 2017/06/28 21:21:39]

Done.

+    AppendFormattedComponent(spec, parsed.path,
+                             NonHostComponentTransform(unescape_rules),
+                             &url_string, &new_parsed->path, adjustments);
+
+    if (parsed.query.is_valid())
+      url_string.push_back('?');
+    AppendFormattedComponent(spec, parsed.query,
+                             NonHostComponentTransform(unescape_rules),
+                             &url_string, &new_parsed->query, adjustments);
+
+    // Ref.  This is valid, unescaped UTF-8, so we can just convert.
+    if (parsed.ref.is_valid())
+      url_string.push_back('#');
+    AppendFormattedComponent(spec, parsed.ref,
+                             NonHostComponentTransform(net::UnescapeRule::NONE),
+                             &url_string, &new_parsed->ref, adjustments);
   }
-  if (parsed.query.is_valid())
-    url_string.push_back('?');
-  AppendFormattedComponent(spec, parsed.query,
-                           NonHostComponentTransform(unescape_rules),
-                           &url_string, &new_parsed->query, adjustments);
-
-  // Ref.  This is valid, unescaped UTF-8, so we can just convert.
-  if (parsed.ref.is_valid())
-    url_string.push_back('#');
-  AppendFormattedComponent(spec, parsed.ref,
-                           NonHostComponentTransform(net::UnescapeRule::NONE),
-                           &url_string, &new_parsed->ref, adjustments);
 
   // If we need to strip out http do it after the fact.
   if (omit_http && base::StartsWith(url_string, base::ASCIIToUTF16(kHTTP),
                                     base::CompareCase::SENSITIVE)) {
     const size_t kHTTPSize = arraysize(kHTTP) - 1;
     url_string = url_string.substr(kHTTPSize);
     // Because offsets in the |adjustments| are already calculated with respect
     // to the string with the http:// prefix in it, those offsets remain correct
     // after stripping the prefix.  The only thing necessary is to add an
     // adjustment to reflect the stripped prefix.
@@ skipping 35 matching lines @@
   return base::StartsWith(text, www, base::CompareCase::SENSITIVE)
       ? text.substr(www.length()) : text;
 }
 
 base::string16 StripWWWFromHost(const GURL& url) {
   DCHECK(url.is_valid());
   return StripWWW(base::ASCIIToUTF16(url.host_piece()));
 }
 
 }  // namespace url_formatter