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

components/url_formatter/url_formatter.cc

Index: components/url_formatter/url_formatter.cc
diff --git a/components/url_formatter/url_formatter.cc b/components/url_formatter/url_formatter.cc
index 298b1c18ff42bfae9102433a16200dd4a65d642b..1dbffd786804df06d4fc0972894967191bd15ec8 100644
--- a/components/url_formatter/url_formatter.cc
+++ b/components/url_formatter/url_formatter.cc
@@ -19,6 +19,7 @@
 #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"
 
@@ -360,6 +361,7 @@ 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,
@@ -506,29 +508,49 @@ base::string16 FormatUrlWithAdjustments(
   }
 
   // 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),