Save memory and improve performance.
Created attachment 203045 [details] WIP
Attachment 203045 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/css/CSSFontSelector.cpp', u'Source/WebCore/page/Settings.cpp', u'Source/WebCore/platform/graphics/Font.cpp', u'Source/WebCore/platform/graphics/Font.h', u'Source/WebCore/platform/graphics/FontCache.cpp', u'Source/WebCore/platform/graphics/FontCache.h', u'Source/WebCore/platform/graphics/FontFallbackList.cpp', u'Source/WebCore/platform/graphics/FontFallbackList.h']" exit_code: 1 Source/WebCore/platform/graphics/Font.cpp:222: Missing spaces around / [whitespace/operators] [3] Source/WebCore/platform/graphics/Font.cpp:320: Missing spaces around / [whitespace/operators] [3] Source/WebCore/platform/graphics/FontFallbackList.h:93: The parameter name "fontSelector" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 3 in 8 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 203045 [details] WIP Attachment 203045 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/724146
Comment on attachment 203045 [details] WIP Attachment 203045 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/742075
Comment on attachment 203045 [details] WIP Attachment 203045 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/742074
Comment on attachment 203045 [details] WIP Attachment 203045 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/670169
Comment on attachment 203045 [details] WIP Attachment 203045 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/715067
Looks like these are failing in bots Regressions: Unexpected text-only failures (2) fast/ruby/nested-ruby.html [ Failure ] svg/W3C-SVG-1.1/fonts-desc-02-t.svg [ Failure ] Regressions: Unexpected image-only failures (2) fast/text/international/lang-sensitive-fonts.html [ ImageOnlyFailure ] platform/mac/fonts/han-disunification.html [ ImageOnlyFailure ]
Comment on attachment 203045 [details] WIP Attachment 203045 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/656461 New failing tests: platform/mac/fonts/han-disunification.html svg/W3C-SVG-1.1/fonts-desc-02-t.svg fast/ruby/nested-ruby.html fast/text/international/lang-sensitive-fonts.html
Created attachment 203128 [details] Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-14 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.3
Comment on attachment 203045 [details] WIP Attachment 203045 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/678085 New failing tests: platform/mac/fonts/han-disunification.html svg/W3C-SVG-1.1/fonts-desc-02-t.svg fast/ruby/nested-ruby.html fast/text/international/lang-sensitive-fonts.html
Created attachment 203144 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.3
Created attachment 203148 [details] patch
Comment on attachment 203148 [details] patch Attachment 203148 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/743257
Comment on attachment 203148 [details] patch Attachment 203148 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/672477
Comment on attachment 203148 [details] patch Attachment 203148 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/745034
Created attachment 203153 [details] patch 2
Comment on attachment 203153 [details] patch 2 Attachment 203153 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/689255
Comment on attachment 203153 [details] patch 2 Attachment 203153 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/660166
Comment on attachment 203153 [details] patch 2 Attachment 203153 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/659110
Created attachment 203185 [details] patch 3
Comment on attachment 203185 [details] patch 3 View in context: https://bugs.webkit.org/attachment.cgi?id=203185&action=review Pretty neat. r=me with some whining: > Source/WebCore/ChangeLog:8 > + Style system generates many Font objects that are indentical or similar enough to have identical FontGlyphs. Typo, identical. > Source/WebCore/ChangeLog:10 > + It also improves perfomrance as the glyph cache and the width cache hang from FontGlyphs and the hit rate of Typo, performance. > Source/WebCore/css/CSSFontSelector.cpp:69 > + , m_uniqueId(++fontSelectorId) Can't this overflow? You're not even using a 64-bit type! > Source/WebCore/css/CSSFontSelector.cpp:631 > + DEFINE_STATIC_LOCAL(String, webkitPrefix, ("-webkit-")); > + if (familyName.startsWith(webkitPrefix)) > + return true; String::startsWith() has a nice overload for this, it's better to just check startsWith("-webkit-") > Source/WebCore/css/CSSFontSelector.h:54 > + virtual ~CSSFontSelector() FINAL OVERRIDE; ... na-na nah nah naaaaaa! Sorry to ruin the '80s song, but we should just mark the class FINAL instead of doing it to every virtual method. > Source/WebCore/platform/graphics/Font.cpp:169 > + if (a.fontDescriptionCacheKey != b.fontDescriptionCacheKey) > + return false; > + if (a.families != b.families) > + return false; > + if (a.fontSelectorId != b.fontSelectorId || a.fontSelectorVersion != b.fontSelectorVersion || a.fontSelectorFlags != b.fontSelectorFlags) > + return false; I would order these comparisons by cost from lowest to highest, so we can fail faster. > Source/WebCore/platform/graphics/Font.cpp:202 > + if (!key.fontDescriptionCacheKey.size || key.families.isEmpty()) > + return 0; The families Vector should never be empty, so I don't think this test is correct. > Source/WebCore/platform/graphics/Font.h:317 > +void pruneUnreferencedFromFontGlyphsCache(); Mildly awkward name. I'd call it pruneUnreferencedEntriesFromFontGlyphsCache(); (because reasons.) > Source/WebCore/platform/graphics/FontCache.cpp:80 > + FontDescriptionFontDataCacheKey m_fontDescriptioKey; Typo, m_fontDescriptioKey. > Source/WebCore/platform/graphics/FontCache.h:67 > + FontDescriptionFontDataCacheKey(unsigned size = 0) This should be marked explicit.
> > Source/WebCore/css/CSSFontSelector.cpp:69 > > + , m_uniqueId(++fontSelectorId) > > Can't this overflow? You're not even using a 64-bit type! Yeah, in theory though that would take a while. Also it is unlikely that any fonts from a colliding font selector are alive at that point. If that happens you may get a wrong font. > > Source/WebCore/css/CSSFontSelector.cpp:631 > > + DEFINE_STATIC_LOCAL(String, webkitPrefix, ("-webkit-")); > > + if (familyName.startsWith(webkitPrefix)) > > + return true; > > String::startsWith() has a nice overload for this, it's better to just check startsWith("-webkit-") Ok. Didn't know that. > > > Source/WebCore/css/CSSFontSelector.h:54 > > + virtual ~CSSFontSelector() FINAL OVERRIDE; > > ... na-na nah nah naaaaaa! > > Sorry to ruin the '80s song, but we should just mark the class FINAL instead of doing it to every virtual method. :(
http://trac.webkit.org/changeset/150897