Fix cursor positioning regression from r201136. GetCursorPos() shouldn't assume

vertically-centered text.

BUG=242801
TEST=Arrow through the omnibox in 200% mode and ensure the cursor does not appear vertically too low
R=asvitkine@chromium.org, msw@chromium.org

Review URL: https://codereview.chromium.org/15746013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@201944 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 20 insertions, 40 deletions

diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index 1f380dc..5348f8a 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -664,19 +664,17 @@ Rect RenderText::GetCursorBounds(const SelectionModel& caret,
   // overtype the next character.
   LogicalCursorDirection caret_affinity =
       insert_mode ? caret.caret_affinity() : CURSOR_FORWARD;
-  int x = 0, width = 1, height = 0;
+  int x = 0, width = 1;
+  Size size = GetStringSize();
   if (caret_pos == (caret_affinity == CURSOR_BACKWARD ? 0 : text().length())) {
     // The caret is attached to the boundary. Always return a 1-dip width caret,
     // since there is nothing to overtype.
-    Size size = GetStringSize();
     if ((GetTextDirection() == base::i18n::RIGHT_TO_LEFT) == (caret_pos == 0))
       x = size.width();
-    height = size.height();
   } else {
     size_t grapheme_start = (caret_affinity == CURSOR_FORWARD) ?
         caret_pos : IndexOfAdjacentGrapheme(caret_pos, CURSOR_BACKWARD);
-    ui::Range xspan;
-    GetGlyphBounds(grapheme_start, &xspan, &height);
+    ui::Range xspan(GetGlyphBounds(grapheme_start));
     if (insert_mode) {
       x = (caret_affinity == CURSOR_BACKWARD) ? xspan.end() : xspan.start();
     } else {  // overtype mode
@@ -684,9 +682,7 @@ Rect RenderText::GetCursorBounds(const SelectionModel& caret,
       width = xspan.length();
     }
   }
-  height = std::min(height, display_rect().height());
-  int y = (display_rect().height() - height) / 2;
-  return Rect(ToViewPoint(Point(x, y)), Size(width, height));
+  return Rect(ToViewPoint(Point(x, 0)), Size(width, size.height()));
 }
 
 const Rect& RenderText::GetUpdatedCursorBounds() {
diff --git a/ui/gfx/render_text.h b/ui/gfx/render_text.h
index eeacb62..41577c8 100644
--- a/ui/gfx/render_text.h
+++ b/ui/gfx/render_text.h
@@ -368,11 +368,11 @@ class UI_EXPORT RenderText {
   // Sets the selection model, the argument is assumed to be valid.
   virtual void SetSelectionModel(const SelectionModel& model);
 
-  // Get the height and horizontal bounds (relative to the left of the text, not
-  // the view) of the glyph starting at |index|. If the glyph is RTL then
-  // xspan->is_reversed(). This does not return a Rect because a Rect can't have
-  // a negative width.
-  virtual void GetGlyphBounds(size_t index, ui::Range* xspan, int* height) = 0;
+  // Get the horizontal bounds (relative to the left of the text, not the view)
+  // of the glyph starting at |index|. If the glyph is RTL then the returned
+  // Range will have is_reversed() true.  (This does not return a Rect because a
+  // Rect can't have a negative width.)
+  virtual ui::Range GetGlyphBounds(size_t index) = 0;
 
   // Get the visual bounds containing the logical substring within the |range|.
   // If |range| is empty, the result is empty. These bounds could be visually
diff --git a/ui/gfx/render_text_linux.cc b/ui/gfx/render_text_linux.cc
index 2958317..279c40f 100644
--- a/ui/gfx/render_text_linux.cc
+++ b/ui/gfx/render_text_linux.cc
@@ -206,14 +206,11 @@ SelectionModel RenderTextLinux::AdjacentWordSelectionModel(
   return cur;
 }
 
-void RenderTextLinux::GetGlyphBounds(size_t index,
-                                     ui::Range* xspan,
-                                     int* height) {
+ui::Range RenderTextLinux::GetGlyphBounds(size_t index) {
   PangoRectangle pos;
   pango_layout_index_to_pos(layout_, TextIndexToLayoutIndex(index), &pos);
   // TODO(derat): Support fractional ranges for subpixel positioning?
-  *xspan = ui::Range(PANGO_PIXELS(pos.x), PANGO_PIXELS(pos.x + pos.width));
-  *height = PANGO_PIXELS(pos.height);
+  return ui::Range(PANGO_PIXELS(pos.x), PANGO_PIXELS(pos.x + pos.width));
 }
 
 std::vector<Rect> RenderTextLinux::GetSubstringBounds(const ui::Range& range) {
diff --git a/ui/gfx/render_text_linux.h b/ui/gfx/render_text_linux.h
index c7e0173..3e0a209 100644
--- a/ui/gfx/render_text_linux.h
+++ b/ui/gfx/render_text_linux.h
@@ -32,9 +32,7 @@ class RenderTextLinux : public RenderText {
   virtual SelectionModel AdjacentWordSelectionModel(
       const SelectionModel& selection,
       VisualCursorDirection direction) OVERRIDE;
-  virtual void GetGlyphBounds(size_t index,
-                              ui::Range* xspan,
-                              int* height) OVERRIDE;
+  virtual ui::Range GetGlyphBounds(size_t index) OVERRIDE;
   virtual std::vector<Rect> GetSubstringBounds(const ui::Range& range) OVERRIDE;
   virtual size_t TextIndexToLayoutIndex(size_t index) const OVERRIDE;
   virtual size_t LayoutIndexToTextIndex(size_t index) const OVERRIDE;
diff --git a/ui/gfx/render_text_mac.cc b/ui/gfx/render_text_mac.cc
index 880823f..67a547d 100644
--- a/ui/gfx/render_text_mac.cc
+++ b/ui/gfx/render_text_mac.cc
@@ -68,10 +68,9 @@ SelectionModel RenderTextMac::AdjacentWordSelectionModel(
   return SelectionModel();
 }
 
-void RenderTextMac::GetGlyphBounds(size_t index,
-                                   ui::Range* xspan,
-                                   int* height) {
+ui::Range RenderTextMac::GetGlyphBounds(size_t index) {
   // TODO(asvitkine): Implement this. http://crbug.com/131618
+  return ui::Range();
 }
 
 std::vector<Rect> RenderTextMac::GetSubstringBounds(const ui::Range& range) {
diff --git a/ui/gfx/render_text_mac.h b/ui/gfx/render_text_mac.h
index ba73945..b89737b 100644
--- a/ui/gfx/render_text_mac.h
+++ b/ui/gfx/render_text_mac.h
@@ -39,9 +39,7 @@ class RenderTextMac : public RenderText {
   virtual SelectionModel AdjacentWordSelectionModel(
       const SelectionModel& selection,
       VisualCursorDirection direction) OVERRIDE;
-  virtual void GetGlyphBounds(size_t index,
-                              ui::Range* xspan,
-                              int* height) OVERRIDE;
+  virtual ui::Range GetGlyphBounds(size_t index) OVERRIDE;
   virtual std::vector<Rect> GetSubstringBounds(const ui::Range& range) OVERRIDE;
   virtual size_t TextIndexToLayoutIndex(size_t index) const OVERRIDE;
   virtual size_t LayoutIndexToTextIndex(size_t index) const OVERRIDE;
diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index 32a0db2..17f8ec4 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -285,11 +285,8 @@ TEST_F(RenderTextTest, ObscuredText) {
   }
 
   // GetGlyphBounds() should yield the entire string bounds for text index 0.
-  int height = 0;
-  ui::Range bounds;
-  render_text->GetGlyphBounds(0U, &bounds, &height);
   EXPECT_EQ(render_text->GetStringSize().width(),
-            static_cast<int>(bounds.length()));
+            static_cast<int>(render_text->GetGlyphBounds(0U).length()));
 
   // Cursoring is independent of underlying characters when text is obscured.
   const wchar_t* const texts[] = {
diff --git a/ui/gfx/render_text_win.cc b/ui/gfx/render_text_win.cc
index bd9e25f..4cf0833 100644
--- a/ui/gfx/render_text_win.cc
+++ b/ui/gfx/render_text_win.cc
@@ -342,17 +342,14 @@ void RenderTextWin::SetSelectionModel(const SelectionModel& model) {
   ResetLayout();
 }
 
-void RenderTextWin::GetGlyphBounds(size_t index,
-                                   ui::Range* xspan,
-                                   int* height) {
+ui::Range RenderTextWin::GetGlyphBounds(size_t index) {
   const size_t run_index =
       GetRunContainingCaret(SelectionModel(index, CURSOR_FORWARD));
   DCHECK_LT(run_index, runs_.size());
   internal::TextRun* run = runs_[run_index];
   const size_t layout_index = TextIndexToLayoutIndex(index);
-  xspan->set_start(GetGlyphXBoundary(run, layout_index, false));
-  xspan->set_end(GetGlyphXBoundary(run, layout_index, true));
-  *height = run->font.GetHeight();
+  return ui::Range(GetGlyphXBoundary(run, layout_index, false),
+                   GetGlyphXBoundary(run, layout_index, true));
 }
 
 std::vector<Rect> RenderTextWin::GetSubstringBounds(const ui::Range& range) {
diff --git a/ui/gfx/render_text_win.h b/ui/gfx/render_text_win.h
index e8ec253..704829d 100644
--- a/ui/gfx/render_text_win.h
+++ b/ui/gfx/render_text_win.h
@@ -80,9 +80,7 @@ class RenderTextWin : public RenderText {
       const SelectionModel& selection,
       VisualCursorDirection direction) OVERRIDE;
   virtual void SetSelectionModel(const SelectionModel& model) OVERRIDE;
-  virtual void GetGlyphBounds(size_t index,
-                              ui::Range* xspan,
-                              int* height) OVERRIDE;
+  virtual ui::Range GetGlyphBounds(size_t index) OVERRIDE;
   virtual std::vector<Rect> GetSubstringBounds(const ui::Range& range) OVERRIDE;
   virtual size_t TextIndexToLayoutIndex(size_t index) const OVERRIDE;
   virtual size_t LayoutIndexToTextIndex(size_t index) const OVERRIDE;