Need AtomicStrings for the various font family names In: static int identifierForFamily(const AtomicString& family) { DEFINE_STATIC_LOCAL(AtomicString, cursiveFamily, ("-webkit-cursive")); DEFINE_STATIC_LOCAL(AtomicString, fantasyFamily, ("-webkit-fantasy")); DEFINE_STATIC_LOCAL(AtomicString, monospaceFamily, ("-webkit-monospace")); DEFINE_STATIC_LOCAL(AtomicString, sansSerifFamily, ("-webkit-sans-serif")); DEFINE_STATIC_LOCAL(AtomicString, serifFamily, ("-webkit-serif")); we use static locals. A bunch of other places we just use c-string constants: void CSSFontSelector::addFontFaceRule(const CSSFontFaceRule* fontFaceRule) static FontData* fontDataForGenericFamily(Document* document, const FontDescription& fontDescription, const AtomicString& familyName) void CSSStyleSelector::applyProperty(int id, CSSValue *value) This is error-prone and slower than necessary. Should be easy to fix with a little .in file wired up through make_names.pl in DerivedSources.make. Also need to make sure that Page() calls FontFamilyNames::init().
Turns out that the various Names::init() calls are made from Frame(...), I'm surprised by this. It's probably "wrong" in that we could have a Page which wanted to use the various Names* calls before the first Frame was created... but oh well.
Created attachment 97237 [details] proposed patch
Created attachment 97249 [details] updated patch (removed debug variables from perl script)
Comment on attachment 97249 [details] updated patch (removed debug variables from perl script) View in context: https://bugs.webkit.org/attachment.cgi?id=97249&action=review > Source/WebCore/dom/make_names.pl:90 > +if (length($fontNamesIn)) { > + my $names = new IO::File; > + my $familyNamesFileBase = "WebKitFontFamilyNames"; Why are we forking all this perl code here? Can't we share with the other names generation?
Created attachment 97811 [details] proposed patch v2
(In reply to comment #4) > (From update of attachment 97249 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97249&action=review > > > Source/WebCore/dom/make_names.pl:90 > > +if (length($fontNamesIn)) { > > + my $names = new IO::File; > > + my $familyNamesFileBase = "WebKitFontFamilyNames"; > > Why are we forking all this perl code here? Can't we share with the other names generation? The common parts appeared to be not that many to be worth obfuscating the understandability of the patch, but I tried now to restructure the common parts of the generation routines into separate functions. Eric, could you take another look?
Created attachment 104026 [details] updated patch
Comment on attachment 104026 [details] updated patch r=me
Comment on attachment 104026 [details] updated patch Clearing flags on attachment: 104026 Committed r93097: <http://trac.webkit.org/changeset/93097>
All reviewed patches have been landed. Closing bug.
Comment on attachment 104026 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=104026&action=review > Source/WebCore/CodeGenerators.pri:772 > +entities.depends = $$PWD/dom/make_names.pl $$FONT_NAMES This has entities.depends, not fontnames.depends. Is that intended?