[webkit-reviews] review granted: [Bug 6218] CSS1: WebTextRenderer caches and re-uses fallback renderers that are based on family lists : [Attachment 12531] Implement trees of fallback glyph pages

[bugzilla-request-daemon at macosforge.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 6218: CSS1: WebTextRenderer caches and re-uses fallback renderers that are
based on family lists
http://bugs.webkit.org/show_bug.cgi?id=6218

Attachment 12531: Implement trees of fallback glyph pages
http://bugs.webkit.org/attachment.cgi?id=12531&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
This is great!

The patch definitely removes an optimization: needCharTransform combined the
smallCaps check and the RTL/mirror check into a single check outside the loop.
The change means we now have an additional branch in the loop. But I'm not sure
what the impact will be. Since this code is so hot someone will need to profile
it again and possibly rearrange things a bit.

+		 return data;
+	     } else {
+		 // Even system fallback can fail.

No need for an else after a return.

+	 return floatWidthForSimpleText(run, style, 0, 0);
     else

Same here.

+	 node = new GlyphPageTreeNode();

I usually leave out the () in cases like this one.

I agree with Hyatt, we should land this!