Omnibox UI Refresh: Make the selection bounds vertically symmetric.

See the bug for screenshots.

Currently, the selection bounds in the Omnibox are not vertically
symmetric. This is becoming a lot more noticeable with the MD Refresh
redesign.

This CL extends RenderText to allow expanding the selection bounds to
be vertically symmetric with respect to the display rect.

This CL only turns on this flag for the OmniboxViewViews.

Bug: 249065
Change-Id: I031017335a0efb5e8842d137fc55efce5520e6cd
Reviewed-on: https://chromium-review.googlesource.com/1140901
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575887}

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

@@ -157,6 +157,7 @@ void OmniboxViewViews::Init() {
   set_controller(this);
   SetTextInputType(ui::TEXT_INPUT_TYPE_URL);
   GetRenderText()->SetElideBehavior(gfx::ELIDE_TAIL);
+  GetRenderText()->set_symmetric_selection_visual_bounds(true);
 
   if (popup_window_mode_)
     SetReadOnly(true);

ui/gfx/render_text.cc

@@ -1433,6 +1433,22 @@ int RenderText::DetermineBaselineCenteringText(const int display_height,
   return baseline + std::max(min_shift, std::min(max_shift, baseline_shift));
 }
 
+// static
+gfx::Rect RenderText::ExpandToBeVerticallySymmetric(
+    const gfx::Rect& rect,
+    const gfx::Rect& display_rect) {
+  // Mirror |rect| accross the horizontal line dividing |display_rect| in half.
+  gfx::Rect result = rect;
+  int mid_y = display_rect.CenterPoint().y();
+  // The top of the mirror rect must be equidistant with the bottom of the
+  // original rect from the mid-line.
+  result.set_y(mid_y + (mid_y - rect.bottom()));
+
+  // Now make a union with the original rect to ensure we are encompassing both.
+  result.Union(rect);
+  return result;
+}
+
 void RenderText::OnTextAttributeChanged() {
   layout_text_.clear();
   display_text_.clear();
@@ -1693,8 +1709,11 @@ void RenderText::UpdateCachedBoundsAndOffset() {
 }
 
 void RenderText::DrawSelection(Canvas* canvas) {
-  for (const Rect& s : GetSubstringBounds(selection()))
+  for (Rect s : GetSubstringBounds(selection())) {
+    if (symmetric_selection_visual_bounds() && !multiline())
+      s = ExpandToBeVerticallySymmetric(s, display_rect());
     canvas->FillRect(s, selection_background_focused_color_);
+  }
 }
 
 size_t RenderText::GetNearestWordStartBoundary(size_t index) const {

ui/gfx/render_text.h

@@ -244,6 +244,13 @@ class GFX_EXPORT RenderText {
     selection_background_focused_color_ = color;
   }
 
+  bool symmetric_selection_visual_bounds() const {
+    return symmetric_selection_visual_bounds_;
+  }
+  void set_symmetric_selection_visual_bounds(bool symmetric) {
+    symmetric_selection_visual_bounds_ = symmetric;
+  }
+
   bool focused() const { return focused_; }
   void set_focused(bool focused) { focused_ = focused; }
 
@@ -697,6 +704,11 @@ class GFX_EXPORT RenderText {
   static int DetermineBaselineCenteringText(const int display_height,
                                             const FontList& font_list);
 
+  // Returns an expanded version of |rect| that is vertically symmetric with
+  // respect to the center of |display_rect|.
+  static gfx::Rect ExpandToBeVerticallySymmetric(const gfx::Rect& rect,
+                                                 const gfx::Rect& display_rect);
+
  private:
   friend class test::RenderTextTestApi;
 
@@ -783,6 +795,11 @@ class GFX_EXPORT RenderText {
   // The background color used for drawing the selection when focused.
   SkColor selection_background_focused_color_;
 
+  // Whether the selection visual bounds should be expanded vertically to be
+  // vertically symmetric with respect to the display rect. Note this flag has
+  // no effect on multi-line text.
+  bool symmetric_selection_visual_bounds_ = false;
+
   // The focus state of the text.
   bool focused_;
 

ui/gfx/render_text_test_api.h

@@ -84,6 +84,12 @@ class RenderTextTestApi {
     render_text_->SetGlyphWidthForTest(test_width);
   }
 
+  static gfx::Rect ExpandToBeVerticallySymmetric(
+      const gfx::Rect& rect,
+      const gfx::Rect& display_rect) {
+    return RenderText::ExpandToBeVerticallySymmetric(rect, display_rect);
+  }
+
  private:
   RenderText* render_text_;
 

ui/gfx/render_text_unittest.cc

@@ -4986,6 +4986,36 @@ TEST_P(RenderTextTest, InvalidFont) {
   DrawVisualText();
 }
 
+TEST_P(RenderTextTest, ExpandToBeVerticallySymmetric) {
+  Rect test_display_rect(0, 0, 400, 100);
+
+  // Basic case.
+  EXPECT_EQ(Rect(20, 20, 400, 60),
+            test::RenderTextTestApi::ExpandToBeVerticallySymmetric(
+                Rect(20, 20, 400, 40), test_display_rect));
+
+  // Expand upwards.
+  EXPECT_EQ(Rect(20, 20, 400, 60),
+            test::RenderTextTestApi::ExpandToBeVerticallySymmetric(
+                Rect(20, 40, 400, 40), test_display_rect));
+
+  // Original rect is entirely above the center point.
+  EXPECT_EQ(Rect(10, 30, 200, 40),
+            test::RenderTextTestApi::ExpandToBeVerticallySymmetric(
+                Rect(10, 30, 200, 10), test_display_rect));
+
+  // Original rect is below the display rect entirely.
+  EXPECT_EQ(Rect(10, -10, 200, 120),
+            test::RenderTextTestApi::ExpandToBeVerticallySymmetric(
+                Rect(10, 100, 200, 10), test_display_rect));
+
+  // Sanity check that we can handle a display rect with a non-zero origin.
+  test_display_rect.Offset(10, 10);
+  EXPECT_EQ(Rect(20, 20, 400, 80),
+            test::RenderTextTestApi::ExpandToBeVerticallySymmetric(
+                Rect(20, 20, 400, 40), test_display_rect));
+}
+
 TEST_P(RenderTextHarfBuzzTest, LinesInvalidationOnElideBehaviorChange) {
   RenderTextHarfBuzz* render_text = GetRenderTextHarfBuzz();
   render_text->SetText(UTF8ToUTF16("This is an example"));