Use GetDisplayText() instead of text() for rendering text.

ui/gfx/render_text_mac.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gfx/render_text_mac.h"
 
 #include <ApplicationServices/ApplicationServices.h>
 
 #include <algorithm>
 #include <cmath>
@@ skipping 15 matching lines @@
 
 scoped_ptr<RenderText> RenderTextMac::CreateInstanceOfSameType() const {
   return make_scoped_ptr(new RenderTextMac);
 }
 
 const base::string16& RenderTextMac::GetDisplayText() {
   return text_elided() ? display_text() : layout_text();
 }
 
 Size RenderTextMac::GetStringSize() {
-  EnsureLayout();
-  return Size(std::ceil(string_size_.width()), string_size_.height());
+  gfx::SizeF size_f = GetStringSizeF();
+  return Size(std::ceil(size_f.width()), size_f.height());

[oshima, 2015/03/05 21:27:58]

nit: gfx::ToCeiledSize ?

[Jun Mukai, 2015/03/06 01:32:38]

Done.

 }
 
 SizeF RenderTextMac::GetStringSizeF() {
   EnsureLayout();
   return string_size_;
 }
 
 SelectionModel RenderTextMac::FindCursorPosition(const Point& point) {
   // TODO(asvitkine): Implement this. http://crbug.com/131618
   return SelectionModel();
@@ skipping 58 matching lines @@
   // TODO(asvitkine): Implement this. http://crbug.com/131618
   return IsValidLogicalIndex(index);
 }
 
 void RenderTextMac::OnLayoutTextAttributeChanged(bool text_changed) {
   DCHECK(!multiline()) << "RenderTextMac does not support multi line";
   if (text_changed) {
     if (elide_behavior() != NO_ELIDE &&
         elide_behavior() != FADE_TAIL &&
         !layout_text().empty()) {
-      UpdateDisplayText(GetContentWidth());
+      UpdateDisplayText(std::ceil(GetLayoutTextSize().width()));

[oshima, 2015/03/05 21:27:58]

It's probably better to rename it to GetLayoutTextWidthF() and do ceiling in the
function because this is only the caller of the function.

[msw, 2015/03/05 21:55:29]

The postfix 'F' implies that it returns a floating point value, so the width
shouldn't be ceil'ed with that function name, right?

[oshima, 2015/03/05 23:09:11]

Oops, yes. Just GetLayoutTextWidth.

[Jun Mukai, 2015/03/06 01:32:38]

Done.

     } else {
       UpdateDisplayText(0);
     }
   }
   line_.reset();
   attributes_.reset();
   runs_.clear();
   runs_valid_ = false;
 }
 
 void RenderTextMac::OnDisplayTextAttributeChanged() {
   OnLayoutTextAttributeChanged(true);
 }
 
 void RenderTextMac::EnsureLayout() {
   if (line_.get())
     return;
   runs_.clear();
   runs_valid_ = false;
 
-  CTFontRef ct_font = base::mac::NSToCFCast(
-      font_list().GetPrimaryFont().GetNativeFont());
-
-  const void* keys[] = { kCTFontAttributeName };
-  const void* values[] = { ct_font };
-  base::ScopedCFTypeRef<CFDictionaryRef> attributes(
-      CFDictionaryCreate(NULL,
-                         keys,
-                         values,
-                         arraysize(keys),
-                         NULL,
-                         &kCFTypeDictionaryValueCallBacks));
-
-  base::ScopedCFTypeRef<CFStringRef> cf_text(
-      base::SysUTF16ToCFStringRef(text()));
-  base::ScopedCFTypeRef<CFAttributedStringRef> attr_text(
-      CFAttributedStringCreate(NULL, cf_text, attributes));
-  base::ScopedCFTypeRef<CFMutableAttributedStringRef> attr_text_mutable(
-      CFAttributedStringCreateMutableCopy(NULL, 0, attr_text));
-
-  // TODO(asvitkine|msw): Respect GetTextDirection(), which may not match the
-  // natural text direction. See kCTTypesetterOptionForcedEmbeddingLevel, etc.
-
-  ApplyStyles(attr_text_mutable, ct_font);
-  line_.reset(CTLineCreateWithAttributedString(attr_text_mutable));
-
-  CGFloat ascent = 0;
-  CGFloat descent = 0;
-  CGFloat leading = 0;
-  // TODO(asvitkine): Consider using CTLineGetBoundsWithOptions() on 10.8+.
-  double width = CTLineGetTypographicBounds(line_, &ascent, &descent, &leading);
-  // Ensure ascent and descent are not smaller than ones of the font list.
-  // Keep them tall enough to draw often-used characters.
-  // For example, if a text field contains a Japanese character, which is
-  // smaller than Latin ones, and then later a Latin one is inserted, this
-  // ensures that the text baseline does not shift.
-  CGFloat font_list_height = font_list().GetHeight();
-  CGFloat font_list_baseline = font_list().GetBaseline();
-  ascent = std::max(ascent, font_list_baseline);
-  descent = std::max(descent, font_list_height - font_list_baseline);
-  string_size_ =
-      SizeF(width, std::max(ascent + descent + leading,
-                            static_cast<CGFloat>(min_line_height())));
-  common_baseline_ = ascent;
+  line_ = EnsureLayoutInternal(GetDisplayText(), &attributes_);
+  string_size_ = GetCTLineSize(line_.get(), &common_baseline_);
 }
 
 void RenderTextMac::DrawVisualText(Canvas* canvas) {
   DCHECK(line_);
   if (!runs_valid_)
     ComputeRuns();
 
   internal::SkiaTextRenderer renderer(canvas);
   ApplyFadeEffects(&renderer);
   ApplyTextShadows(&renderer);
@@ skipping 20 matching lines @@
       text_size(0),
       foreground(SK_ColorBLACK),
       underline(false),
       strike(false),
       diagonal_strike(false) {
 }
 
 RenderTextMac::TextRun::~TextRun() {
 }
 
-void RenderTextMac::ApplyStyles(CFMutableAttributedStringRef attr_string,
-                                CTFontRef font) {
+gfx::SizeF RenderTextMac::GetLayoutTextSize() {
+  base::ScopedCFTypeRef<CFMutableArrayRef> attributes_owner;
+  base::ScopedCFTypeRef<CTLineRef> line(
+      EnsureLayoutInternal(layout_text(), &attributes_owner));
+  SkScalar baseline;
+  return GetCTLineSize(line.get(), &baseline);
+}
+
+gfx::SizeF RenderTextMac::GetCTLineSize(CTLineRef line, SkScalar* baseline) {
+  CGFloat ascent = 0;
+  CGFloat descent = 0;
+  CGFloat leading = 0;
+  // TODO(asvitkine): Consider using CTLineGetBoundsWithOptions() on 10.8+.
+  double width = CTLineGetTypographicBounds(line, &ascent, &descent, &leading);
+  // Ensure ascent and descent are not smaller than ones of the font list.
+  // Keep them tall enough to draw often-used characters.
+  // For example, if a text field contains a Japanese character, which is
+  // smaller than Latin ones, and then later a Latin one is inserted, this
+  // ensures that the text baseline does not shift.
+  CGFloat font_list_height = font_list().GetHeight();
+  CGFloat font_list_baseline = font_list().GetBaseline();
+  ascent = std::max(ascent, font_list_baseline);
+  descent = std::max(descent, font_list_height - font_list_baseline);
+  *baseline = ascent;
+  return SizeF(
+      width, std::max(ascent + descent + leading,
+                      static_cast<CGFloat>(min_line_height())));
+}
+
+base::ScopedCFTypeRef<CTLineRef> RenderTextMac::EnsureLayoutInternal(
+    const base::string16& text,
+    base::ScopedCFTypeRef<CFMutableArrayRef>* attributes_owner) {
+  CTFontRef ct_font = base::mac::NSToCFCast(
+      font_list().GetPrimaryFont().GetNativeFont());
+
+  const void* keys[] = { kCTFontAttributeName };
+  const void* values[] = { ct_font };
+  base::ScopedCFTypeRef<CFDictionaryRef> attributes(
+      CFDictionaryCreate(NULL,
+                         keys,
+                         values,
+                         arraysize(keys),
+                         NULL,
+                         &kCFTypeDictionaryValueCallBacks));
+
+  base::ScopedCFTypeRef<CFStringRef> cf_text(
+      base::SysUTF16ToCFStringRef(text));
+  base::ScopedCFTypeRef<CFAttributedStringRef> attr_text(
+      CFAttributedStringCreate(NULL, cf_text, attributes));
+  base::ScopedCFTypeRef<CFMutableAttributedStringRef> attr_text_mutable(
+      CFAttributedStringCreateMutableCopy(NULL, 0, attr_text));
+
+  // TODO(asvitkine|msw): Respect GetTextDirection(), which may not match the
+  // natural text direction. See kCTTypesetterOptionForcedEmbeddingLevel, etc.
+
+  *attributes_owner = ApplyStyles(attr_text_mutable, ct_font);
+  return base::ScopedCFTypeRef<CTLineRef>(
+      CTLineCreateWithAttributedString(attr_text_mutable));
+}
+
+base::ScopedCFTypeRef<CFMutableArrayRef> RenderTextMac::ApplyStyles(
+    CFMutableAttributedStringRef attr_string,
+    CTFontRef font) {
   // Temporarily apply composition underlines and selection colors.
   ApplyCompositionAndSelectionStyles();
 
   // Note: CFAttributedStringSetAttribute() does not appear to retain the values
   // passed in, as can be verified via CFGetRetainCount(). To ensure the
   // attribute objects do not leak, they are saved to |attributes_|.
   // Clear the attributes storage.
-  attributes_.reset(CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks));
+  base::ScopedCFTypeRef<CFMutableArrayRef> attributes(
+      CFArrayCreateMutable(NULL, 0, &kCFTypeArrayCallBacks));
 
   // https://developer.apple.com/library/mac/#documentation/Carbon/Reference/CoreText_StringAttributes_Ref/Reference/reference.html
   internal::StyleIterator style(colors(), styles());
-  const size_t layout_text_length = GetDisplayText().length();
+  const size_t layout_text_length = CFAttributedStringGetLength(attr_string);
   for (size_t i = 0, end = 0; i < layout_text_length; i = end) {
-    end = TextIndexToDisplayIndex(style.GetRange().end());
+    end = std::min(style.GetRange().end(), layout_text_length);

[msw, 2015/03/05 21:55:29]

nit q: should this use something like TextIndexToGivenTextIndex (promoted from
RTHB to RT)? (no worries if that's a significant change for little benefit and
this is sufficient)

[Jun Mukai, 2015/03/06 01:32:38]

good idea. done.

     const CFRange range = CFRangeMake(i, end - i);
     base::ScopedCFTypeRef<CGColorRef> foreground(
         CGColorCreateFromSkColor(style.color()));
     CFAttributedStringSetAttribute(attr_string, range,
         kCTForegroundColorAttributeName, foreground);
-    CFArrayAppendValue(attributes_, foreground);
+    CFArrayAppendValue(attributes, foreground);
 
     if (style.style(UNDERLINE)) {
       CTUnderlineStyle value = kCTUnderlineStyleSingle;
       base::ScopedCFTypeRef<CFNumberRef> underline_value(
           CFNumberCreate(NULL, kCFNumberSInt32Type, &value));
       CFAttributedStringSetAttribute(attr_string, range,
                                      kCTUnderlineStyleAttributeName,
                                      underline_value);
-      CFArrayAppendValue(attributes_, underline_value);
+      CFArrayAppendValue(attributes, underline_value);
     }
 
     const int traits = (style.style(BOLD) ? kCTFontBoldTrait : 0) |
                        (style.style(ITALIC) ? kCTFontItalicTrait : 0);
     if (traits != 0) {
       base::ScopedCFTypeRef<CTFontRef> styled_font(
           CTFontCreateCopyWithSymbolicTraits(font, 0.0, NULL, traits, traits));
       // TODO(asvitkine): Handle |styled_font| == NULL case better.
       if (styled_font) {
         CFAttributedStringSetAttribute(attr_string, range, kCTFontAttributeName,
                                        styled_font);
-        CFArrayAppendValue(attributes_, styled_font);
+        CFArrayAppendValue(attributes, styled_font);
       }
     }
 
     style.UpdatePosition(DisplayIndexToTextIndex(end));
   }
 
   // Undo the temporarily applied composition underlines and selection colors.
   UndoCompositionAndSelectionStyles();
+
+  return attributes;
 }
 
 void RenderTextMac::ComputeRuns() {
   DCHECK(line_);
 
   CFArrayRef ct_runs = CTLineGetGlyphRuns(line_);
   const CFIndex ct_runs_count = CFArrayGetCount(ct_runs);
 
   // TODO(asvitkine): Don't use GetLineOffset() until draw time, since it may be
   // updated based on alignment changes without resetting the layout.
@@ skipping 76 matching lines @@
     CTUnderlineStyle value = kCTUnderlineStyleNone;
     if (underline && CFNumberGetValue(underline, kCFNumberSInt32Type, &value))
       run->underline = (value == kCTUnderlineStyleSingle);
 
     run_origin.offset(run_width, 0);
   }
   runs_valid_ = true;
 }
 
 }  // namespace gfx