This is hopefully the last step before our renderers can run cleanly in a chroot. WebKit needs to be able to ask for the correct font to use in the case that the current font doesn't include glyphs for certain code points. Currently we make a fontconfig call in our WebKit port to handle this. This patch changes this so that the call is sent our via ChromiumBridge. http://codereview.chromium.org/132007
Created attachment 31520 [details] patch
Comment on attachment 31520 [details] patch I'm leaving the "real" review for Darin Fisher, but I'll give you some style things to fix up to make it go faster. > diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog > +2009-06-18 Adam Langley <agl@google.com> > + > + Reviewed by NOBODY (OOPS!). > + Typically the link to the bug is near the top of the change log entry. (I did see you put it in bug it way at the bottom.) > diff --git a/WebCore/platform/chromium/ChromiumBridge.h b/WebCore/platform/chromium/ChromiumBridge.h > + static String getFontFamilyForCharacters(const UChar* characters, int numCharacters); "characters" doesn't seem to add any information here due to the api name and the const UChar* type. So it would be good to remove it. (WebKit style is to not put in param names when they don't add information.) > diff --git a/WebCore/platform/graphics/chromium/FontCacheLinux.cpp b/WebCore/platform/graphics/chromium/FontCacheLinux.cpp > + String family = ChromiumBridge::getFontFamilyForCharacters(characters, length); > + if (family.isEmpty()) > + return NULL; Use 0 instead of NULL.
Comment on attachment 31520 [details] patch My only comment, in addition to what Dave wrote, is that numCharacters should probably be size_t instead of int. Post a revised patch and LGTM.
Created attachment 31791 [details] patch Hopefully have addressed all comments
Did you attach the right patch?
Created attachment 31798 [details] patch > Did you attach the right patch? Opps, sorry.