Replace StyleRange with BreakList; update RenderText, etc.

ui/gfx/render_text_mac.cc

Index: ui/gfx/render_text_mac.cc
diff --git a/ui/gfx/render_text_mac.cc b/ui/gfx/render_text_mac.cc
index a6367141fe8fada68628d05355661d717e1fcfaf..0055418fd758506c8948c091646c9c56f143c27c 100644
--- a/ui/gfx/render_text_mac.cc
+++ b/ui/gfx/render_text_mac.cc
@@ -158,7 +158,7 @@ void RenderTextMac::DrawVisualText(Canvas* canvas) {
     renderer.DrawPosText(&run.glyph_positions[0], &run.glyphs[0],
                          run.glyphs.size());
     renderer.DrawDecorations(run.origin.x(), run.origin.y(), run.width,
-                             run.style);
+                             run.underline, run.strike, run.diagonal_strike);
   }
 }
 
@@ -168,7 +168,10 @@ RenderTextMac::TextRun::TextRun()
       width(0),
       font_style(Font::NORMAL),
       text_size(0),
-      foreground(SK_ColorBLACK) {
+      foreground(SK_ColorBLACK),
+      underline(false),
+      strike(false),
+      diagonal_strike(false) {
 }
 
 RenderTextMac::TextRun::~TextRun() {
@@ -176,44 +179,47 @@ RenderTextMac::TextRun::~TextRun() {
 
 void RenderTextMac::ApplyStyles(CFMutableAttributedStringRef attr_string,
                                 CTFontRef font) {
-  // Clear attributes and reserve space to hold the maximum number of entries,
-  // which is at most three per style range per the code below.
-  attributes_.reset(CFArrayCreateMutable(NULL, 3 * style_ranges().size(),
+  // Temporarily apply composition underlines and selection colors.
+  ApplyCompositionAndSelectionStyles();
+
+  // Track the current color and style with iterators.
+  BreakList<SkColor>::const_iterator color = colors().list().begin();
+  size_t max_style_count = colors().list().size();
+  BreakList<bool>::const_iterator style[NUM_TEXT_STYLES];
+  for (size_t i = 0; i < NUM_TEXT_STYLES; ++i) {
+    style[i] = styles(static_cast<TextStyle>(i)).list().begin();
+    max_style_count += styles(static_cast<TextStyle>(i)).list().size();
+  }
+
+  // Clear attributes and reserve space to hold the maximum number of entries.
+  // 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_|.
+  attributes_.reset(CFArrayCreateMutable(NULL, max_style_count,

[Alexei Svitkine (slow), 2013/01/28 20:52:30]

Please put the "Note:" comment above the current first line in the comment.

The previous parameter here was based on the fact that the loop below can add
at-most 3 entries to |attributes_| per each iteration of the loop.

The new breaks approach makes this hard to reserve accurately, so I suggest
passing 0 to it, which per the docs makes it so there's no limit on the
capacity.

https://developer.apple.com/library/mac/#documentation/CoreFoundation/Referen...

(And change the comment to just say "Clear the attributes storage." after the
Note: comment.)

[msw, 2013/01/29 17:17:25]

Done (but potentially reverting in the subsequent patch set...). I'm not sure
what's causing RenderTextTest.StringSizeSanity to crash on Mac, but I'll take a
look once I get in; I can't ping/remote into my Mac box :(.

                                          &kCFTypeArrayCallBacks));
 
   // https://developer.apple.com/library/mac/#documentation/Carbon/Reference/CoreText_StringAttributes_Ref/Reference/reference.html
-  for (size_t i = 0; i < style_ranges().size(); ++i) {
-    const StyleRange& style = style_ranges()[i];
-    const CFRange range = CFRangeMake(style.range.start(),
-                                      style.range.length());
-
-    // 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_|.
-
+  for (size_t i = 0, end = 0; i < text().length(); i = end) {
+    end = GetNextBreak(color, style);
+    const CFRange range = CFRangeMake(i, end - i);
     base::mac::ScopedCFTypeRef<CGColorRef> foreground(
-        gfx::CGColorCreateFromSkColor(style.foreground));
+        gfx::CGColorCreateFromSkColor(color->second));
     CFAttributedStringSetAttribute(attr_string, range,
-                                   kCTForegroundColorAttributeName,
-                                   foreground);
+        kCTForegroundColorAttributeName, foreground);
     CFArrayAppendValue(attributes_, foreground);
 
-    if (style.underline) {
+    if (style[UNDERLINE]->second) {
       CTUnderlineStyle value = kCTUnderlineStyleSingle;
-      base::mac::ScopedCFTypeRef<CFNumberRef> underline(
+      base::mac::ScopedCFTypeRef<CFNumberRef> underline_value(
           CFNumberCreate(NULL, kCFNumberSInt32Type, &value));
       CFAttributedStringSetAttribute(attr_string, range,
-                                     kCTUnderlineStyleAttributeName,
-                                     underline);
-      CFArrayAppendValue(attributes_, underline);
+          kCTUnderlineStyleAttributeName, underline_value);

[Alexei Svitkine (slow), 2013/01/28 20:52:30]

Nit:

This is actually against the style guide, see:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Calls

We used to have some text on the Chromium style guide page that allowed this,
but it's no longer there. Please change the formatting back here. (The other
allowed style is one param on each line).

[msw, 2013/01/29 17:17:25]

Done.

+      CFArrayAppendValue(attributes_, underline_value);
     }
 
-    if (style.font_style & (Font::BOLD | Font::ITALIC)) {
-      int traits = 0;
-      if (style.font_style & Font::BOLD)
-        traits |= kCTFontBoldTrait;
-      if (style.font_style & Font::ITALIC)
-        traits |= kCTFontItalicTrait;
+    const int traits = (style[BOLD]->second ? kCTFontBoldTrait : 0) |
+                       (style[ITALIC]->second ? kCTFontItalicTrait : 0);
+    if (traits != 0) {
       base::mac::ScopedCFTypeRef<CTFontRef> styled_font(
           CTFontCreateCopyWithSymbolicTraits(font, 0.0, NULL, traits, traits));
       // TODO(asvitkine): Handle |styled_font| == NULL case better.
@@ -223,7 +229,12 @@ void RenderTextMac::ApplyStyles(CFMutableAttributedStringRef attr_string,
         CFArrayAppendValue(attributes_, styled_font);
       }
     }
+
+    AdvanceIterators(end, &color, style);
   }
+
+  // Undo the temporarily applied composition underlines and selection colors.
+  UndoCompositionAndSelectionStyles();
 }
 
 void RenderTextMac::ComputeRuns() {
@@ -283,7 +294,7 @@ void RenderTextMac::ComputeRuns() {
     }
 
     // TODO(asvitkine): Style boundaries are not necessarily per-run. Handle
-    //                  this better.
+    //                  this better. Also, support strike and diagonal_strike.
     CFDictionaryRef attributes = CTRunGetAttributes(ct_run);
     CTFontRef ct_font =
         base::mac::GetValueFromDictionary<CTFontRef>(attributes,
@@ -310,7 +321,7 @@ void RenderTextMac::ComputeRuns() {
             attributes, kCTUnderlineStyleAttributeName);
     CTUnderlineStyle value = kCTUnderlineStyleNone;
     if (underline && CFNumberGetValue(underline, kCFNumberSInt32Type, &value))
-      run->style.underline = (value == kCTUnderlineStyleSingle);
+      run->underline = (value == kCTUnderlineStyleSingle);
 
     run_origin.offset(run_width, 0);
   }