Fix RenderText cached bounds and offset logic; update clients.

Ensure the display offset and cursor bounds are valid on use.
Build in offset logic to RenderText display and hit-testing.
Implement simpler temporary GetStringWidth and GetCursorBounds.
Fix SetDisplayRect signature, code file ordering, invalidation.
Rename and refactor a bit, update comments.

Fixes adornment display & hit testing on text field overflows.
Increases abstraction/encapsulation for easier client use.

BUG=90426
TEST=--use-pure-views / touch_ui textfield use with overflow.

Review URL: http://codereview.chromium.org/7466048

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

4 files changed, 92 insertions, 93 deletions

diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index fce00b5..2855a07 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -130,10 +130,6 @@ void SelectionModel::Init(size_t start,
 RenderText::~RenderText() {
 }
 
-void RenderText::set_display_rect(const Rect& r) {
-  display_rect_ = r;
-}
-
 void RenderText::SetText(const string16& text) {
   size_t old_text_length = text_.length();
   text_ = text;
@@ -161,6 +157,7 @@ void RenderText::SetText(const string16& text) {
 #ifndef NDEBUG
   CheckStyleRanges(style_ranges_, text_.length());
 #endif
+  cached_bounds_and_offset_valid_ = false;
 }
 
 void RenderText::SetSelectionModel(const SelectionModel& sel) {
@@ -171,7 +168,12 @@ void RenderText::SetSelectionModel(const SelectionModel& sel) {
   selection_model_.set_caret_pos(std::min(sel.caret_pos(), text().length()));
   selection_model_.set_caret_placement(sel.caret_placement());
 
-  cursor_bounds_valid_ = false;
+  cached_bounds_and_offset_valid_ = false;
+}
+
+void RenderText::SetDisplayRect(const Rect& r) {
+  display_rect_ = r;
+  cached_bounds_and_offset_valid_ = false;
 }
 
 size_t RenderText::GetCursorPosition() const {
@@ -328,8 +330,8 @@ void RenderText::ApplyStyleRange(StyleRange style_range) {
 #ifndef NDEBUG
   CheckStyleRanges(style_ranges_, text_.length());
 #endif
-  // TODO(xji): only invalidate cursor_bounds if font or underline change.
-  cursor_bounds_valid_ = false;
+  // TODO(xji): only invalidate if font or underline changes.
+  cached_bounds_and_offset_valid_ = false;
 }
 
 void RenderText::ApplyDefaultStyle() {
@@ -337,7 +339,7 @@ void RenderText::ApplyDefaultStyle() {
   StyleRange style = StyleRange(default_style_);
   style.range.set_end(text_.length());
   style_ranges_.push_back(style);
-  cursor_bounds_valid_ = false;
+  cached_bounds_and_offset_valid_ = false;
 }
 
 base::i18n::TextDirection RenderText::GetTextDirection() const {
@@ -347,7 +349,7 @@ base::i18n::TextDirection RenderText::GetTextDirection() const {
 }
 
 int RenderText::GetStringWidth() {
-  return GetSubstringBounds(0, text_.length())[0].width();
+  return default_style_.font.GetStringWidth(text());
 }
 
 void RenderText::Draw(Canvas* canvas) {
@@ -363,10 +365,6 @@ void RenderText::Draw(Canvas* canvas) {
   for (std::vector<Rect>::const_iterator i = selection.begin();
        i < selection.end(); ++i) {
     Rect r(*i);
-    r.Offset(display_rect_.origin());
-    r.Offset(display_offset_);
-    // Center the rect vertically in |display_rect_|.
-    r.Offset(Point(0, (display_rect_.height() - r.height()) / 2));
     canvas->FillRectInt(selection_color, r.x(), r.y(), r.width(), r.height());
   }
 
@@ -376,7 +374,7 @@ void RenderText::Draw(Canvas* canvas) {
 
   // Draw the text.
   Rect bounds(display_rect_);
-  bounds.Offset(display_offset_);
+  bounds.Offset(GetUpdatedDisplayOffset());
   for (StyleRanges::const_iterator i = style_ranges.begin();
        i < style_ranges.end(); ++i) {
     const Font& font = !i->underline ? i->font :
@@ -403,16 +401,10 @@ void RenderText::Draw(Canvas* canvas) {
   }
 
   // Paint cursor. Replace cursor is drawn as rectangle for now.
-  if (cursor_visible() && focused()) {
-    bounds = CursorBounds();
-    bounds.Offset(display_offset_);
-    if (!bounds.IsEmpty())
-      canvas->DrawRectInt(kCursorColor,
-                          bounds.x(),
-                          bounds.y(),
-                          bounds.width(),
-                          bounds.height());
-  }
+  Rect cursor(GetUpdatedCursorBounds());
+  if (cursor_visible() && focused() && !cursor.IsEmpty())
+    canvas->DrawRectInt(kCursorColor, cursor.x(), cursor.y(),
+                        cursor.width(), cursor.height());
 }
 
 SelectionModel RenderText::FindCursorPosition(const Point& point) {
@@ -422,7 +414,7 @@ SelectionModel RenderText::FindCursorPosition(const Point& point) {
   int right = font.GetStringWidth(text());
   int right_pos = text().length();
 
-  int x = point.x();
+  int x = point.x() - (display_rect_.x() + GetUpdatedDisplayOffset().x());
   if (x <= left) return SelectionModel(left_pos);
   if (x >= right) return SelectionModel(right_pos);
   // binary searching the cursor position.
@@ -444,36 +436,29 @@ SelectionModel RenderText::FindCursorPosition(const Point& point) {
   return SelectionModel(left_pos);
 }
 
-std::vector<Rect> RenderText::GetSubstringBounds(
-    size_t from, size_t to) const {
+std::vector<Rect> RenderText::GetSubstringBounds(size_t from, size_t to) {
   size_t start = std::min(from, to);
   size_t end = std::max(from, to);
   const Font& font = default_style_.font;
   int start_x = font.GetStringWidth(text().substr(0, start));
   int end_x = font.GetStringWidth(text().substr(0, end));
-  std::vector<Rect> bounds;
-  bounds.push_back(Rect(start_x, 0, end_x - start_x, font.GetHeight()));
-  return bounds;
+  Rect rect(start_x, 0, end_x - start_x, font.GetHeight());
+  rect.Offset(display_rect_.origin());
+  rect.Offset(GetUpdatedDisplayOffset());
+  // Center the rect vertically in |display_rect_|.
+  rect.Offset(Point(0, (display_rect_.height() - rect.height()) / 2));
+  return std::vector<Rect>(1, rect);
 }
 
 Rect RenderText::GetCursorBounds(const SelectionModel& selection,
                                  bool insert_mode) {
-  size_t cursor_pos = selection.selection_end();
-  const Font& font = default_style_.font;
-  int x = font.GetStringWidth(text_.substr(0U, cursor_pos));
-  DCHECK_GE(x, 0);
-  int h = std::min(display_rect_.height(), font.GetHeight());
-  Rect bounds(x, (display_rect_.height() - h) / 2, 1, h);
-  if (!insert_mode && text_.length() != cursor_pos)
-    bounds.set_width(font.GetStringWidth(text_.substr(0, cursor_pos + 1)) - x);
-  return bounds;
-}
-
-const Rect& RenderText::CursorBounds() {
-  if (cursor_bounds_valid_ == false) {
-    UpdateCursorBoundsAndDisplayOffset();
-    cursor_bounds_valid_ = true;
-  }
+  size_t from = selection.selection_end();
+  size_t to = insert_mode ? from : std::min(text_.length(), from + 1);
+  return GetSubstringBounds(from, to)[0];
+}
+
+const Rect& RenderText::GetUpdatedCursorBounds() {
+  UpdateCachedBoundsAndOffset();
   return cursor_bounds_;
 }
 
@@ -481,14 +466,19 @@ RenderText::RenderText()
     : text_(),
       selection_model_(),
       cursor_bounds_(),
-      cursor_bounds_valid_(false),
       cursor_visible_(false),
       insert_mode_(true),
       composition_range_(),
       style_ranges_(),
       default_style_(),
       display_rect_(),
-      display_offset_() {
+      display_offset_(),
+      cached_bounds_and_offset_valid_(false) {
+}
+
+const Point& RenderText::GetUpdatedDisplayOffset() {
+  UpdateCachedBoundsAndOffset();
+  return display_offset_;
 }
 
 SelectionModel RenderText::GetLeftSelectionModel(const SelectionModel& current,
@@ -584,21 +574,31 @@ bool RenderText::IsPositionAtWordSelectionBoundary(size_t pos) {
       (!u_isalnum(text()[pos - 1]) && u_isalnum(text()[pos]));
 }
 
-void RenderText::UpdateCursorBoundsAndDisplayOffset() {
-  cursor_bounds_ = GetCursorBounds(selection_model_, insert_mode());
+void RenderText::UpdateCachedBoundsAndOffset() {
+  if (cached_bounds_and_offset_valid_)
+    return;
+  // First, set the valid flag true to calculate the current cursor bounds using
+  // the stale |display_offset_|. Applying |delta_offset| at the end of this
+  // function will set |cursor_bounds_| and |display_offset_| to correct values.
+  cached_bounds_and_offset_valid_ = true;
+  cursor_bounds_ = GetCursorBounds(selection_model_, insert_mode_);
+  cursor_bounds_.set_width(std::max(cursor_bounds_.width(), 1));
   // Update |display_offset_| to ensure the current cursor is visible.
   int display_width = display_rect_.width();
   int string_width = GetStringWidth();
+  int delta_offset = 0;
   if (string_width < display_width) {
     // Show all text whenever the text fits to the size.
-    display_offset_.set_x(0);
-  } else if ((display_offset_.x() + cursor_bounds_.right()) > display_width) {
+    delta_offset = -display_offset_.x();
+  } else if (cursor_bounds_.right() > display_rect_.right()) {
     // Pan to show the cursor when it overflows to the right,
-     display_offset_.set_x(display_width - cursor_bounds_.right());
-  } else if ((display_offset_.x() + cursor_bounds_.x()) < 0) {
+    delta_offset = display_rect_.right() - cursor_bounds_.right();
+  } else if (cursor_bounds_.x() < display_rect_.x()) {
     // Pan to show the cursor when it overflows to the left.
-    display_offset_.set_x(-cursor_bounds_.x());
+    delta_offset = display_rect_.x() - cursor_bounds_.x();
   }
+  display_offset_.Offset(delta_offset, 0);
+  cursor_bounds_.Offset(delta_offset, 0);
 }
 
 }  // namespace gfx
diff --git a/ui/gfx/render_text.h b/ui/gfx/render_text.h
index d90be42..c3f9104 100644
--- a/ui/gfx/render_text.h
+++ b/ui/gfx/render_text.h
@@ -171,9 +171,7 @@ class UI_EXPORT RenderText {
   void set_default_style(StyleRange style) { default_style_ = style; }
 
   const Rect& display_rect() const { return display_rect_; }
-  virtual void set_display_rect(const Rect& r);
-
-  const Point& display_offset() const { return display_offset_; }
+  virtual void SetDisplayRect(const Rect& r);
 
   // This cursor position corresponds to SelectionModel::selection_end. In
   // addition to representing the selection end, it's also where logical text
@@ -241,25 +239,33 @@ class UI_EXPORT RenderText {
   virtual SelectionModel FindCursorPosition(const Point& point);
 
   // Get the visual bounds containing the logical substring within |from| to
-  // |to|. These bounds could be visually discontinuous if the logical
-  // selection range is split by an odd number of LTR/RTL level change.
-  virtual std::vector<Rect> GetSubstringBounds(
-      size_t from, size_t to) const;
-
-  // Get the visual bounds describing the cursor at |selection|. These bounds
-  // typically represent a vertical line, but if |insert_mode| is true they
-  // contain the bounds of the associated glyph.
+  // |to|. These bounds could be visually discontinuous if the substring is
+  // split by a LTR/RTL level change. These bounds are in local coordinates, but
+  // may be outside the visible region if the text is longer than the textfield.
+  // Subsequent text, cursor, or bounds changes may invalidate returned values.
+  virtual std::vector<Rect> GetSubstringBounds(size_t from, size_t to);
+
+  // Get the visual bounds of a cursor at |selection|. These bounds typically
+  // represent a vertical line, but if |insert_mode| is true they contain the
+  // bounds of the associated glyph. These bounds are in local coordinates, but
+  // may be outside the visible region if the text is longer than the textfield.
+  // Subsequent text, cursor, or bounds changes may invalidate returned values.
   virtual Rect GetCursorBounds(const SelectionModel& selection,
                                bool insert_mode);
 
-  // Compute cursor_bounds_ and update display_offset_ when necessary. Cache
-  // the values for later use and return cursor_bounds_.
-  const Rect& CursorBounds();
+  // Compute the current cursor bounds, panning the text to show the cursor in
+  // the display rect if necessary. These bounds are in local coordinates.
+  // Subsequent text, cursor, or bounds changes may invalidate returned values.
+  const Rect& GetUpdatedCursorBounds();
 
  protected:
   RenderText();
 
-  void set_cursor_bounds_valid(bool valid) { cursor_bounds_valid_ = valid; }
+  const Point& GetUpdatedDisplayOffset();
+
+  void set_cached_bounds_and_offset_valid(bool valid) {
+      cached_bounds_and_offset_valid_ = valid;
+  }
 
   const StyleRanges& style_ranges() const { return style_ranges_; }
 
@@ -290,7 +296,9 @@ class UI_EXPORT RenderText {
 
   bool IsPositionAtWordSelectionBoundary(size_t pos);
 
-  void UpdateCursorBoundsAndDisplayOffset();
+  // Update the cached bounds and display offset to ensure that the current
+  // cursor is within the visible display area.
+  void UpdateCachedBoundsAndOffset();
 
   // Logical UTF-16 string data to be drawn.
   string16 text_;
@@ -298,12 +306,8 @@ class UI_EXPORT RenderText {
   // Logical selection range and visual cursor position.
   SelectionModel selection_model_;
 
-  // The cached cursor bounds.
+  // The cached cursor bounds; get these bounds with GetUpdatedCursorBounds.
   Rect cursor_bounds_;
-  // cursor_bounds_ is computed when needed and cached afterwards. And it is
-  // invalidated in operations such as SetCursorPosition, SetSelection, Font
-  // related style change, and other operations that trigger re-layout.
-  bool cursor_bounds_valid_;
 
   // The cursor visibility and insert mode.
   bool cursor_visible_;
@@ -323,8 +327,14 @@ class UI_EXPORT RenderText {
   // The local display area for rendering the text.
   Rect display_rect_;
   // The offset for the text to be drawn, relative to the display area.
+  // Get this point with GetUpdatedDisplayOffset (or risk using a stale value).
   Point display_offset_;
 
+  // The cached bounds and offset are invalidated by operations such as
+  // SetCursorPosition, SetSelectionModel, Font related style change, and other
+  // operations that adjust the visible text bounds.
+  bool cached_bounds_and_offset_valid_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderText);
 };
 
diff --git a/views/controls/textfield/native_textfield_views.cc b/views/controls/textfield/native_textfield_views.cc
index c41136d..810bad1 100644
--- a/views/controls/textfield/native_textfield_views.cc
+++ b/views/controls/textfield/native_textfield_views.cc
@@ -607,7 +607,7 @@ void NativeTextfieldViews::OnBoundsChanged(const gfx::Rect& previous_bounds) {
                          insets.top(),
                          width() - insets.width(),
                          height() - insets.height());
-  GetRenderText()->set_display_rect(display_rect);
+  GetRenderText()->SetDisplayRect(display_rect);
   OnCaretBoundsChanged();
 }
 
@@ -693,7 +693,7 @@ ui::TextInputType NativeTextfieldViews::GetTextInputType() {
 }
 
 gfx::Rect NativeTextfieldViews::GetCaretBounds() {
-  return GetRenderText()->CursorBounds();
+  return GetRenderText()->GetUpdatedCursorBounds();
 }
 
 bool NativeTextfieldViews::HasCompositionText() {
@@ -990,14 +990,8 @@ void NativeTextfieldViews::OnCaretBoundsChanged() {
       sel.selection_start(), gfx::SelectionModel::LEADING);
   gfx::Rect start_cursor = render_text->GetCursorBounds(start_sel, false);
   gfx::Rect end_cursor = render_text->GetCursorBounds(sel, false);
-  gfx::Rect display_rect = render_text->display_rect();
-  int total_offset_x = display_rect.x() + render_text->display_offset().x();
-  int total_offset_y = display_rect.y() + render_text->display_offset().y() +
-                       (display_rect.height() - start_cursor.height()) / 2;
-  gfx::Point start(start_cursor.x() + total_offset_x,
-                   start_cursor.bottom() + total_offset_y);
-  gfx::Point end(end_cursor.x() + total_offset_x,
-                 end_cursor.bottom() + total_offset_y);
+  gfx::Point start(start_cursor.x(), start_cursor.bottom());
+  gfx::Point end(end_cursor.x(), end_cursor.bottom());
   touch_selection_controller_->SelectionChanged(start, end);
 }
 
diff --git a/views/touchui/touch_selection_controller_impl_unittest.cc b/views/touchui/touch_selection_controller_impl_unittest.cc
index a722af1..75353fe 100644
--- a/views/touchui/touch_selection_controller_impl_unittest.cc
+++ b/views/touchui/touch_selection_controller_impl_unittest.cc
@@ -38,7 +38,7 @@ class TouchSelectionControllerImplTest : public ViewsTestBase {
     textfield_ = new Textfield();
     widget_ = new Widget;
     Widget::InitParams params(Widget::InitParams::TYPE_POPUP);
-    params.bounds = gfx::Rect(0, 0, 100, 100);
+    params.bounds = gfx::Rect(0, 0, 200, 200);
     widget_->Init(params);
     View* container = new View();
     widget_->SetContentsView(container);
@@ -58,12 +58,7 @@ class TouchSelectionControllerImplTest : public ViewsTestBase {
     gfx::RenderText* render_text = textfield_view_->GetRenderText();
     gfx::Rect cursor_bounds = render_text->GetCursorBounds(
         gfx::SelectionModel(cursor_pos), false);
-    gfx::Rect display_rect = render_text->display_rect();
-    int total_offset_x = display_rect.x() + render_text->display_offset().x();
-    int total_offset_y = display_rect.y() + render_text->display_offset().y() +
-                         (display_rect.height() - cursor_bounds.height()) / 2;
-    return gfx::Point(cursor_bounds.x() + total_offset_x,
-                      cursor_bounds.bottom() + total_offset_y);
+    return gfx::Point(cursor_bounds.x(), cursor_bounds.bottom());
   }
 
   TouchSelectionControllerImpl* GetSelectionController() {