Omnibox UI Experiments: Add a GetURLForDisplay method with elisions.

Add a new GetURLForDisplay method to sit alongside GetFormattedFullURL.
This will be used in the Omnibox if the feature flag is on. The actual
implementation knows nothing about the feature flag, since the feature
flag is an Omnibox concern rather than a components/toolbar concern.

Bug: 797354
Change-Id: I61fe6a70a5d7bb990a52866a9a28da8bc50d8a36
Reviewed-on: https://chromium-review.googlesource.com/877040
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531055}

chrome/browser/ui/toolbar/toolbar_model_unittest.cc

@@ -33,46 +33,46 @@ namespace {
 
 struct TestItem {
   GURL url;
-  base::string16 expected_text;
+  const char* expected_formatted_full_url;
+  const char* expected_elided_url_for_display = expected_formatted_full_url;
 } test_items[] = {
     {
-        GURL("view-source:http://www.google.com"),
-        base::ASCIIToUTF16("view-source:www.google.com"),
+        GURL("view-source:http://www.google.com"), "view-source:www.google.com",
+        "view-source:google.com",
     },
     {
-        GURL("chrome://newtab/"), base::string16(),
+        GURL("chrome://newtab/"), "",
     },
     {
-        GURL("view-source:chrome://newtab/"),
-        base::ASCIIToUTF16("view-source:chrome://newtab"),
+        GURL("view-source:chrome://newtab/"), "view-source:chrome://newtab",
     },
     {
-        GURL("chrome-search://local-ntp/local-ntp.html"), base::string16(),
+        GURL("chrome-search://local-ntp/local-ntp.html"), "",
     },
     {
         GURL("view-source:chrome-search://local-ntp/local-ntp.html"),
-        base::ASCIIToUTF16(
-            "view-source:chrome-search://local-ntp/local-ntp.html"),
+        "view-source:chrome-search://local-ntp/local-ntp.html",
     },
     {
         GURL("chrome-extension://fooooooooooooooooooooooooooooooo/bar.html"),
-        base::ASCIIToUTF16(
-            "chrome-extension://fooooooooooooooooooooooooooooooo/bar.html"),
+        "chrome-extension://fooooooooooooooooooooooooooooooo/bar.html",
     },
     {
-        GURL(url::kAboutBlankURL), base::ASCIIToUTF16(url::kAboutBlankURL),
+        GURL(url::kAboutBlankURL), url::kAboutBlankURL,
     },
     {
         GURL("http://searchurl/?q=tractor+supply"),
-        base::ASCIIToUTF16("searchurl/?q=tractor+supply"),
+        "searchurl/?q=tractor+supply",
     },
     {
-        GURL("http://google.com/search?q=tractor+supply"),
-        base::ASCIIToUTF16("google.com/search?q=tractor+supply"),
+        GURL("http://www.google.com/search?q=tractor+supply"),
+        "www.google.com/search?q=tractor+supply",
+        "google.com/search?q=tractor+supply",
     },
     {
-        GURL("https://google.ca/search?q=tractor+supply"),
-        base::ASCIIToUTF16("https://google.ca/search?q=tractor+supply"),
+        GURL("https://m.google.ca/search?q=tractor+supply"),
+        "https://m.google.ca/search?q=tractor+supply",
+        "google.ca/search?q=tractor+supply",
     },
 };
 
@@ -90,8 +90,10 @@ class ToolbarModelTest : public BrowserWithTestWindowTest {
   void SetUp() override;
 
  protected:
-  void NavigateAndCheckText(const GURL& url,
-                            const base::string16& expected_text);
+  void NavigateAndCheckText(
+      const GURL& url,
+      const base::string16& expected_formatted_full_url,
+      const base::string16& expected_elided_url_for_display);
   void NavigateAndCheckElided(const GURL& https_url);
 
  private:
@@ -128,20 +130,25 @@ void ToolbarModelTest::SetUp() {
 
 void ToolbarModelTest::NavigateAndCheckText(
     const GURL& url,
-    const base::string16& expected_text) {
+    const base::string16& expected_formatted_full_url,
+    const base::string16& expected_elided_url_for_display) {
   // Check while loading.
   content::NavigationController* controller =
       &browser()->tab_strip_model()->GetActiveWebContents()->GetController();
   controller->LoadURL(url, content::Referrer(), ui::PAGE_TRANSITION_LINK,
                       std::string());
   ToolbarModel* toolbar_model = browser()->toolbar_model();
-  EXPECT_EQ(expected_text, toolbar_model->GetFormattedFullURL());
-  EXPECT_NE(expected_text.empty(), toolbar_model->ShouldDisplayURL());
+  EXPECT_EQ(expected_formatted_full_url, toolbar_model->GetFormattedFullURL());
+  EXPECT_NE(expected_formatted_full_url.empty(),
+            toolbar_model->ShouldDisplayURL());
+  EXPECT_EQ(expected_elided_url_for_display, toolbar_model->GetURLForDisplay());
 
   // Check after commit.
   CommitPendingLoad(controller);
-  EXPECT_EQ(expected_text, toolbar_model->GetFormattedFullURL());
-  EXPECT_NE(expected_text.empty(), toolbar_model->ShouldDisplayURL());
+  EXPECT_EQ(expected_formatted_full_url, toolbar_model->GetFormattedFullURL());
+  EXPECT_NE(expected_formatted_full_url.empty(),
+            toolbar_model->ShouldDisplayURL());
+  EXPECT_EQ(expected_elided_url_for_display, toolbar_model->GetURLForDisplay());
 }
 
 void ToolbarModelTest::NavigateAndCheckElided(const GURL& url) {
@@ -151,17 +158,29 @@ void ToolbarModelTest::NavigateAndCheckElided(const GURL& url) {
   controller->LoadURL(url, content::Referrer(), ui::PAGE_TRANSITION_LINK,
                       std::string());
   ToolbarModel* toolbar_model = browser()->toolbar_model();
-  const base::string16 toolbar_text_before(
+  const base::string16 formatted_full_url_before(
       toolbar_model->GetFormattedFullURL());
-  EXPECT_LT(toolbar_text_before.size(), url.spec().size());
-  EXPECT_TRUE(base::EndsWith(toolbar_text_before,
+  EXPECT_LT(formatted_full_url_before.size(), url.spec().size());
+  EXPECT_TRUE(base::EndsWith(formatted_full_url_before,
                              base::string16(gfx::kEllipsisUTF16),
                              base::CompareCase::SENSITIVE));
+  const base::string16 display_url_before(toolbar_model->GetURLForDisplay());
+  EXPECT_LT(display_url_before.size(), url.spec().size());
+  EXPECT_TRUE(base::EndsWith(display_url_before,
+                             base::string16(gfx::kEllipsisUTF16),
+                             base::CompareCase::SENSITIVE));
+
   // Check after commit.
   CommitPendingLoad(controller);
-  const base::string16 toolbar_text_after(toolbar_model->GetFormattedFullURL());
-  EXPECT_LT(toolbar_text_after.size(), url.spec().size());
-  EXPECT_TRUE(base::EndsWith(toolbar_text_after,
+  const base::string16 formatted_full_url_after(
+      toolbar_model->GetFormattedFullURL());
+  EXPECT_LT(formatted_full_url_after.size(), url.spec().size());
+  EXPECT_TRUE(base::EndsWith(formatted_full_url_after,
+                             base::string16(gfx::kEllipsisUTF16),
+                             base::CompareCase::SENSITIVE));
+  const base::string16 display_url_after(toolbar_model->GetURLForDisplay());
+  EXPECT_LT(display_url_after.size(), url.spec().size());
+  EXPECT_TRUE(base::EndsWith(display_url_after,
                              base::string16(gfx::kEllipsisUTF16),
                              base::CompareCase::SENSITIVE));
 }
@@ -173,7 +192,10 @@ TEST_F(ToolbarModelTest, ShouldDisplayURL) {
   AddTab(browser(), GURL(url::kAboutBlankURL));
 
   for (const TestItem& test_item : test_items) {
-    NavigateAndCheckText(test_item.url, test_item.expected_text);
+    NavigateAndCheckText(
+        test_item.url,
+        base::ASCIIToUTF16(test_item.expected_formatted_full_url),
+        base::ASCIIToUTF16(test_item.expected_elided_url_for_display));
   }
 }
 

components/toolbar/test_toolbar_model.cc

@@ -22,6 +22,10 @@ base::string16 TestToolbarModel::GetFormattedFullURL() const {
   return text_;
 }
 
+base::string16 TestToolbarModel::GetURLForDisplay() const {
+  return text_;
+}
+
 GURL TestToolbarModel::GetURL() const {
   return url_;
 }

components/toolbar/test_toolbar_model.h

@@ -24,6 +24,7 @@ class TestToolbarModel : public ToolbarModel {
   TestToolbarModel();
   ~TestToolbarModel() override;
   base::string16 GetFormattedFullURL() const override;
+  base::string16 GetURLForDisplay() const override;
   GURL GetURL() const override;
   security_state::SecurityLevel GetSecurityLevel(
       bool ignore_editing) const override;

components/toolbar/toolbar_model.h

@@ -32,6 +32,12 @@ class ToolbarModel {
   // applying any elisions that change the meaning of the URL.
   virtual base::string16 GetFormattedFullURL() const = 0;
 
+  // Returns a simplified URL for display (but not editing) on the toolbar.
+  // This formatting is generally a superset of GetFormattedFullURL, and may
+  // include some destructive elisions that change the meaning of the URL.
+  // The returned string is not suitable for editing, and is for display only.
+  virtual base::string16 GetURLForDisplay() const = 0;
+
   // Returns the URL of the current navigation entry.
   virtual GURL GetURL() const = 0;
 

components/toolbar/toolbar_model_impl.cc

@@ -47,17 +47,27 @@ base::string16 ToolbarModelImpl::GetFormattedFullURL() const {
           url, url_formatter::FormatUrl(
                    url, url_formatter::kFormatUrlOmitDefaults,
                    net::UnescapeRule::NORMAL, nullptr, nullptr, nullptr));
-  if (formatted_text.length() <= max_url_display_chars_)
-    return formatted_text;
 
   // Truncating the URL breaks editing and then pressing enter, but hopefully
   // people won't try to do much with such enormous URLs anyway. If this becomes
   // a real problem, we could perhaps try to keep some sort of different "elided
   // visible URL" where editing affects and reloads the "real underlying URL",
   // but this seems very tricky for little gain.
-  return gfx::TruncateString(formatted_text, max_url_display_chars_ - 1,
-                             gfx::CHARACTER_BREAK) +
-         gfx::kEllipsisUTF16;
+  return gfx::TruncateString(formatted_text, max_url_display_chars_,
+                             gfx::CHARACTER_BREAK);
+}
+
+base::string16 ToolbarModelImpl::GetURLForDisplay() const {
+  url_formatter::FormatUrlTypes format_types =
+      url_formatter::kFormatUrlOmitDefaults |
+      url_formatter::kFormatUrlOmitHTTPS |
+      url_formatter::kFormatUrlOmitTrivialSubdomains;
+  base::string16 result = url_formatter::FormatUrl(GetURL(), format_types,
+                                                   net::UnescapeRule::NORMAL,
+                                                   nullptr, nullptr, nullptr);
+
+  return gfx::TruncateString(result, max_url_display_chars_,
+                             gfx::CHARACTER_BREAK);
 }
 
 GURL ToolbarModelImpl::GetURL() const {

components/toolbar/toolbar_model_impl.h

@@ -29,6 +29,7 @@ class ToolbarModelImpl : public ToolbarModel {
  private:
   // ToolbarModel:
   base::string16 GetFormattedFullURL() const override;
+  base::string16 GetURLForDisplay() const override;
   GURL GetURL() const override;
   security_state::SecurityLevel GetSecurityLevel(
       bool ignore_editing) const override;