RenderText: handle center-aligned text in UpdateCachedBoundsAndOffset()

ui/gfx/render_text.cc

Index: ui/gfx/render_text.cc
diff --git a/ui/gfx/render_text.cc b/ui/gfx/render_text.cc
index fb7364b66f2b538f5733867b11afc11330755c6d..6431a3ac18a55ca3f6b29b0843e2b959d0d19b98 100644
--- a/ui/gfx/render_text.cc
+++ b/ui/gfx/render_text.cc
@@ -892,7 +892,7 @@ void RenderText::SetDisplayOffset(int horizontal_offset) {
         max_offset = extra_content;
         break;
       case ALIGN_CENTER:
-        min_offset = -extra_content / 2;
+        min_offset = -(extra_content + 1) / 2;
         max_offset = extra_content / 2;
         break;
       default:
@@ -1309,48 +1309,34 @@ void RenderText::UpdateCachedBoundsAndOffset() {
 
   // TODO(ckocagil): Add support for scrolling multiline text.
 
-  // 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;
-  if (cursor_enabled())
-    cursor_bounds_ = GetCursorBounds(selection_model_, insert_mode_);
+  int delta_x = 0;
 
-  // Update |display_offset_| to ensure the current cursor is visible.
-  const int display_width = display_rect_.width();
-  const int content_width = GetContentWidth();
+  if (cursor_enabled()) {
+    // When cursor is enabled, ensure it is visible. For this, set the valid
+    // flag true and calculate the current cursor bounds using the stale
+    // |display_offset_|. Then calculate the change in offset needed to to move

[msw, 2014/07/16 01:41:43]

nit: fix "to to"

[mohsen, 2014/07/16 20:48:10]

Done.

+    // the cursor into the visible area.
+    cached_bounds_and_offset_valid_ = true;
+    cursor_bounds_ = GetCursorBounds(selection_model_, insert_mode_);
 
-  int delta_x = 0;
-  if (content_width <= display_width || !cursor_enabled()) {
-    // Don't pan if the text fits in the display width or when the cursor is
-    // disabled.
-    delta_x = -display_offset_.x();
-  } else if (cursor_bounds_.right() > display_rect_.right()) {
-    // TODO(xji): when the character overflow is a RTL character, currently, if
-    // we pan cursor at the rightmost position, the entered RTL character is not
-    // displayed. Should pan cursor to show the last logical characters.
-    //
-    // Pan to show the cursor when it overflows to the right.
-    delta_x = display_rect_.right() - cursor_bounds_.right();
-  } else if (cursor_bounds_.x() < display_rect_.x()) {
-    // TODO(xji): have similar problem as above when overflow character is a
-    // LTR character.
-    //
-    // Pan to show the cursor when it overflows to the left.
-    delta_x = display_rect_.x() - cursor_bounds_.x();
-  } else if (display_offset_.x() != 0) {
-    // Reduce the pan offset to show additional overflow text when the display

[msw, 2014/07/16 01:41:43]

Does the new code still do this via min/max clamping in SetDisplayOffset?

[mohsen, 2014/07/16 20:48:10]

Yes.

-    // width increases.
-    const int negate_rtl = horizontal_alignment_ == ALIGN_RIGHT ? -1 : 1;
-    const int offset = negate_rtl * display_offset_.x();
-    if (display_width > (content_width + offset)) {
-      delta_x = negate_rtl * (display_width - (content_width + offset));
+    if (cursor_bounds_.right() > display_rect_.right()) {
+      // TODO(xji): when the character overflow is a RTL character, currently,

[msw, 2014/07/16 01:41:43]

nit: consolidate these two as TODO(bidi) above the if/else

[mohsen, 2014/07/16 20:48:10]

Done. Please check if the result is satisfactory.

+      // if we pan cursor at the rightmost position, the entered RTL character
+      // is not displayed. Should pan cursor to show the last logical
+      // characters.
+      //
+      // Pan to show the cursor when it overflows to the right.

[msw, 2014/07/16 01:41:43]

nit: remove the "pan to show" comments, the top comment is sufficient.

[mohsen, 2014/07/16 20:48:10]

Done.

+      delta_x = display_rect_.right() - cursor_bounds_.right();
+    } else if (cursor_bounds_.x() < display_rect_.x()) {
+      // TODO(xji): have similar problem as above when overflow character is a
+      // LTR character.
+      //
+      // Pan to show the cursor when it overflows to the left.
+      delta_x = display_rect_.x() - cursor_bounds_.x();
     }
   }
 
-  Vector2d delta_offset(delta_x, 0);
-  display_offset_ += delta_offset;
-  cursor_bounds_ += delta_offset;
+  SetDisplayOffset(display_offset_.x() + delta_x);
 }
 
 void RenderText::DrawSelection(Canvas* canvas) {