Simplified domain: fix some test oddities

This CL fixes a few oddities in the omnibox tests for the simplified
domain field trials.

- The main fix is to make ExpectElidedToSimplifiedDomain() not
  sensitive to the rounding behavior of GetSubstringBounds(), which is
  changing in a follow-up CL
  (https://chromium-review.googlesource.com/c/chromium/src/+/2317820).
  Previously we were checking that the display offset is correct by
  comparing it to the width of the string that should be scrolled
  offscreen, but that won't work when GetSubstringBounds() is
  subsequently changed to round outwards so that adjacent substrings'
  bounds can overlap slightly -- this makes the width of the portion
  scrolled offscreen slightly larger than the actual display offset.
  Instead, we now compare the x value of what should be visible with
  the x value of what should be offscreen to check that the display
  offset is correct.

- Along the way I noticed that one of the checks in
  ExpectElidedToSimplifiedDomain() was bogus, so I fixed that.

- And I noticed that the URL wasn't set properly (it had an extra
  scheme appended to it) in SetUpSimplifiedDomainTest(). This didn't
  really matter because all the code looks at the display URL (which
  was set properly) rather than the actual URL, but I fixed it anyway.

Bug: 1108045
Change-Id: I4c233ce8485b5e09613cac7f268f4b3abc87b142
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2332581Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#793669}

chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc

@@ -305,8 +305,7 @@ class OmniboxViewViewsTest : public OmniboxViewViewsTestBase {
 
   // Sets up tests for the simplified domain field trials.
   void SetUpSimplifiedDomainTest() {
-    location_bar_model()->set_url(
-        GURL(base::ASCIIToUTF16("https://") + kSimplifiedDomainDisplayUrl));
+    location_bar_model()->set_url(GURL(kSimplifiedDomainDisplayUrl));
     location_bar_model()->set_url_for_display(kSimplifiedDomainDisplayUrl);
     omnibox_view()->model()->ResetDisplayTexts();
     omnibox_view()->RevertAll();
@@ -1417,14 +1416,14 @@ TEST_F(OmniboxViewViewsSteadyStateElisionsTest, UnelideFromModel) {
 // |subdomain_and_scheme| and |subdomain| should include a trailing ".", and
 // |path| should include a leading "/".
 void ExpectElidedToSimplifiedDomain(gfx::RenderText* render_text,
-                                    const base::string16& subdomain_and_scheme,
+                                    const base::string16& scheme,
                                     const base::string16& subdomain,
                                     const base::string16& hostname_and_scheme,
                                     const base::string16& path,
                                     bool should_elide_to_registrable_domain) {
   gfx::Rect subdomain_and_scheme_rect;
   for (const auto& rect : render_text->GetSubstringBounds(
-           gfx::Range(0, subdomain_and_scheme.size()))) {
+           gfx::Range(0, scheme.size() + subdomain.size()))) {
     subdomain_and_scheme_rect.Union(rect);
   }
   gfx::Rect path_rect;
@@ -1437,20 +1436,38 @@ void ExpectElidedToSimplifiedDomain(gfx::RenderText* render_text,
   if (should_elide_to_registrable_domain) {
     EXPECT_FALSE(
         render_text->display_rect().Contains(subdomain_and_scheme_rect));
-    EXPECT_EQ(subdomain_and_scheme_rect.width(),
+    gfx::Rect registrable_domain_rect;
+    for (const auto& rect : render_text->GetSubstringBounds(gfx::Range(
+             scheme.size() + subdomain.size(), hostname_and_scheme.size()))) {
+      registrable_domain_rect.Union(rect);
+    }
+    EXPECT_TRUE(render_text->display_rect().Contains(registrable_domain_rect));
+    // The text should be scrolled to push the scheme and subdomain offscreen,
+    // so that the text starts at the registrable domain. Note that this code
+    // computes the expected offset by comparing x() values rather than
+    // comparing based on widths (for example, it wouldn't work to check that
+    // the display offset is equal to |subdomain_and_scheme_rect|'s width). This
+    // is because GetSubstringBounds() rounds outward, so the width of
+    // |subdomain_and_scheme_rect| could slightly overlap
+    // |registrable_domain_rect|.
+    EXPECT_EQ(registrable_domain_rect.x() - subdomain_and_scheme_rect.x(),
               -1 * render_text->GetUpdatedDisplayOffset().x());
   } else {
     // When elision to registrable domain is disabled, the scheme should be
     // hidden but the subdomain should not be.
     EXPECT_FALSE(
         render_text->display_rect().Contains(subdomain_and_scheme_rect));
-    gfx::Rect subdomain_rect;
-    for (const auto& rect :
-         render_text->GetSubstringBounds(gfx::Range(0, subdomain.size()))) {
-      subdomain_rect.Union(rect);
+    gfx::Rect hostname_rect;
+    for (const auto& rect : render_text->GetSubstringBounds(
+             gfx::Range(scheme.size(), hostname_and_scheme.size()))) {
+      hostname_rect.Union(rect);
     }
-    EXPECT_EQ(subdomain_rect.x() - render_text->display_rect().x(),
-              render_text->GetUpdatedDisplayOffset().x());
+    // The text should be scrolled to push the scheme offscreen, so that the
+    // text starts at the subdomain. As above, it's important to compute the
+    // expected offset with x() values instead of width()s, since the width()s
+    // of different adjacent substring bounds could overlap.
+    EXPECT_EQ(hostname_rect.x() - subdomain_and_scheme_rect.x(),
+              -1 * render_text->GetUpdatedDisplayOffset().x());
   }
 }
 
@@ -1573,7 +1590,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, HoverAndExit) {
   SetUpSimplifiedDomainTest();
   gfx::RenderText* render_text = omnibox_view()->GetRenderText();
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1598,7 +1615,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, HoverAndExit) {
   // After the extended hover threshold has elapsed, the display text shouldn't
   // have changed yet.
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1626,7 +1643,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, HoverAndExit) {
   hover_animation_as_element->Step(base::TimeTicks() +
                                    base::TimeDelta::FromSeconds(1));
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1698,7 +1715,7 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
   elide_as_element->SetStartTime(base::TimeTicks());
   elide_as_element->Step(base::TimeTicks() + base::TimeDelta::FromSeconds(1));
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1804,7 +1821,7 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest, BoundsChanged) {
   EXPECT_TRUE(elide_animation->IsAnimating());
   omnibox_view()->OnBoundsChanged(gfx::Rect());
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1817,7 +1834,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, BoundsChanged) {
   gfx::RenderText* render_text = omnibox_view()->GetRenderText();
 
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1825,7 +1842,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, BoundsChanged) {
   // After the bounds change, the URL should remain elided.
   omnibox_view()->OnBoundsChanged(gfx::Rect());
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1840,7 +1857,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, BoundsChanged) {
   EXPECT_TRUE(unelide_animation->IsAnimating());
   omnibox_view()->OnBoundsChanged(gfx::Rect());
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -1915,11 +1932,14 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
       elide_animation->GetAnimationForTesting());
   elide_as_element->SetStartTime(base::TimeTicks());
   elide_as_element->Step(base::TimeTicks() + base::TimeDelta::FromSeconds(2));
+  // Use should_elide_to_registrable_domain=true here regardless of how the
+  // field trial is set because the "www." should be elided as a trivial
+  // subdomain.
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, base::ASCIIToUTF16("https://www."),
-      base::ASCIIToUTF16("www."),
+      render_text, base::ASCIIToUTF16("https://"), base::ASCIIToUTF16("www."),
       base::ASCIIToUTF16("https://www.example.test"),
-      base::ASCIIToUTF16("/foo"), ShouldElideToRegistrableDomain()));
+      base::ASCIIToUTF16("/foo"),
+      /* should_elide_to_registrable_domain=*/true));
 
   // Do another hover and check that the URL gets unelided to the full URL.
   omnibox_view()->OnMouseMoved(CreateMouseEvent(ui::ET_MOUSE_MOVED, {0, 0}));
@@ -1946,10 +1966,10 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
   elide_as_element->SetStartTime(base::TimeTicks());
   elide_as_element->Step(base::TimeTicks() + base::TimeDelta::FromSeconds(2));
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, base::ASCIIToUTF16("https://www."),
-      base::ASCIIToUTF16("www."),
+      render_text, base::ASCIIToUTF16("https://"), base::ASCIIToUTF16("www."),
       base::ASCIIToUTF16("https://www.example.test"),
-      base::ASCIIToUTF16("/foo"), ShouldElideToRegistrableDomain()));
+      base::ASCIIToUTF16("/foo"),
+      /* should_elide_to_registrable_domain=*/true));
   EXPECT_EQ(SK_ColorTRANSPARENT, omnibox_view()->GetLatestColorForRange(
                                      gfx::Range(0, kSchemeAndSubdomainSize)));
 }
@@ -2124,8 +2144,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverAndMaybeHideOnInteractionTest,
     // to the registrable domain because the www subdomain is considered
     // trivial.
     ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-        render_text, base::ASCIIToUTF16("https://www."),
-        base::ASCIIToUTF16("www."),
+        render_text, base::ASCIIToUTF16("https://"), base::ASCIIToUTF16("www."),
         base::ASCIIToUTF16("https://www.example.test"),
         base::ASCIIToUTF16("/foo"),
         true /* should elide to registrable domain */));
@@ -2338,7 +2357,7 @@ TEST_P(OmniboxViewViewsHideOnInteractionTest, SameDocNavigations) {
   elide_as_element->SetStartTime(base::TimeTicks());
   elide_as_element->Step(base::TimeTicks() + base::TimeDelta::FromSeconds(1));
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -2350,7 +2369,7 @@ TEST_P(OmniboxViewViewsHideOnInteractionTest, SameDocNavigations) {
     navigation.set_is_same_document(true);
     omnibox_view()->DidFinishNavigation(&navigation);
     ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-        render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+        render_text, kSimplifiedDomainDisplayUrlScheme,
         kSimplifiedDomainDisplayUrlSubdomain,
         kSimplifiedDomainDisplayUrlHostnameAndScheme,
         kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -2508,7 +2527,7 @@ TEST_P(OmniboxViewViewsHideOnInteractionTest, SubframeNavigations) {
   elide_as_element->SetStartTime(base::TimeTicks());
   elide_as_element->Step(base::TimeTicks() + base::TimeDelta::FromSeconds(1));
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -2528,7 +2547,7 @@ TEST_P(OmniboxViewViewsHideOnInteractionTest, SubframeNavigations) {
     navigation.set_render_frame_host(subframe);
     omnibox_view()->DidFinishNavigation(&navigation);
     ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-        render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+        render_text, kSimplifiedDomainDisplayUrlScheme,
         kSimplifiedDomainDisplayUrlSubdomain,
         kSimplifiedDomainDisplayUrlHostnameAndScheme,
         kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -2555,7 +2574,7 @@ TEST_P(OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest,
   elide_as_element->SetStartTime(base::TimeTicks());
   elide_as_element->Step(base::TimeTicks() + base::TimeDelta::FromSeconds(1));
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      render_text, kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      render_text, kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));
@@ -2602,8 +2621,7 @@ TEST_P(OmniboxViewViewsRevealOnHoverTest, AfterBlur) {
   omnibox_view()->OnFocus();
   omnibox_view()->OnBlur();
   ASSERT_NO_FATAL_FAILURE(ExpectElidedToSimplifiedDomain(
-      omnibox_view()->GetRenderText(),
-      kSimplifiedDomainDisplayUrlSubdomainAndScheme,
+      omnibox_view()->GetRenderText(), kSimplifiedDomainDisplayUrlScheme,
       kSimplifiedDomainDisplayUrlSubdomain,
       kSimplifiedDomainDisplayUrlHostnameAndScheme,
       kSimplifiedDomainDisplayUrlPath, ShouldElideToRegistrableDomain()));