Global FontPlatformData cache should use OwnPtr.
Created attachment 192376 [details] Patch
Attachment 192376 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/FontCache.cpp', u'Source/WebCore/platform/graphics/FontCache.h', u'Source/WebCore/platform/graphics/blackberry/FontCacheBlackBerry.cpp', u'Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp', u'Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp', u'Source/WebCore/platform/graphics/mac/FontCacheMac.mm', u'Source/WebCore/platform/graphics/qt/FontCacheQt.cpp', u'Source/WebCore/platform/graphics/skia/FontCacheSkia.cpp', u'Source/WebCore/platform/graphics/win/FontCacheWin.cpp', u'Source/WebCore/platform/graphics/wince/FontCacheWinCE.cpp', u'Source/WebCore/platform/graphics/wx/FontCacheWx.cpp']" exit_code: 1 Source/WebCore/platform/graphics/FontCache.cpp:209: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/platform/graphics/FontCache.cpp:210: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 192376 [details] Patch Attachment 192376 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17118021
Comment on attachment 192376 [details] Patch Attachment 192376 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17024086
Comment on attachment 192376 [details] Patch Attachment 192376 [details] did not pass cr-linux-debug-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17164006
Comment on attachment 192376 [details] Patch Attachment 192376 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17148013
Comment on attachment 192376 [details] Patch Attachment 192376 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17142050
Created attachment 192377 [details] Patch Less 0, more nullptr.
Attachment 192377 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/FontCache.cpp', u'Source/WebCore/platform/graphics/FontCache.h', u'Source/WebCore/platform/graphics/blackberry/FontCacheBlackBerry.cpp', u'Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp', u'Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp', u'Source/WebCore/platform/graphics/mac/FontCacheMac.mm', u'Source/WebCore/platform/graphics/qt/FontCacheQt.cpp', u'Source/WebCore/platform/graphics/skia/FontCacheSkia.cpp', u'Source/WebCore/platform/graphics/win/FontCacheWin.cpp', u'Source/WebCore/platform/graphics/wince/FontCacheWinCE.cpp', u'Source/WebCore/platform/graphics/wx/FontCacheWx.cpp']" exit_code: 1 Source/WebCore/platform/graphics/FontCache.cpp:209: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/platform/graphics/FontCache.cpp:210: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 192377 [details] Patch Attachment 192377 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17023323
Comment on attachment 192377 [details] Patch Attachment 192377 [details] did not pass efl-ews (efl): Output: http://webkit-commit-queue.appspot.com/results/17131076
Comment on attachment 192377 [details] Patch Attachment 192377 [details] did not pass qt-wk2-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17164010
Comment on attachment 192377 [details] Patch Attachment 192377 [details] did not pass qt-ews (qt): Output: http://webkit-commit-queue.appspot.com/results/17017843
Comment on attachment 192377 [details] Patch Attachment 192377 [details] did not pass cr-linux-debug-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/17131078
Created attachment 192378 [details] Patch Even less 0, even more nullptr.
Attachment 192378 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/platform/graphics/FontCache.cpp', u'Source/WebCore/platform/graphics/FontCache.h', u'Source/WebCore/platform/graphics/blackberry/FontCacheBlackBerry.cpp', u'Source/WebCore/platform/graphics/chromium/FontCacheAndroid.cpp', u'Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp', u'Source/WebCore/platform/graphics/mac/FontCacheMac.mm', u'Source/WebCore/platform/graphics/qt/FontCacheQt.cpp', u'Source/WebCore/platform/graphics/skia/FontCacheSkia.cpp', u'Source/WebCore/platform/graphics/win/FontCacheWin.cpp', u'Source/WebCore/platform/graphics/wince/FontCacheWinCE.cpp', u'Source/WebCore/platform/graphics/wx/FontCacheWx.cpp']" exit_code: 1 Source/WebCore/platform/graphics/FontCache.cpp:209: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Source/WebCore/platform/graphics/FontCache.cpp:210: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 192378 [details] Patch Rejecting attachment 192378 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 192378, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: file Source/WebCore/platform/graphics/qt/FontCacheQt.cpp patching file Source/WebCore/platform/graphics/skia/FontCacheSkia.cpp patching file Source/WebCore/platform/graphics/win/FontCacheWin.cpp patching file Source/WebCore/platform/graphics/wince/FontCacheWinCE.cpp patching file Source/WebCore/platform/graphics/wx/FontCacheWx.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Anders Carlsson']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://webkit-commit-queue.appspot.com/results/17122572
Committed r147639: <http://trac.webkit.org/changeset/147639>
This patch seems to cause assertions in EFL API tests: ASSERTION FAILED: m_table /home/buildslave-1/webkit-buildslave/efl-linux-64-debug-wk2/build/Source/WTF/wtf/HashTable.h(210) : void WTF::HashTableConstIterator<Key, Value, Extractor, HashFunctions, Traits, KeyTraits>::checkValidity() const [with Key = WebCore::FontPlatformDataCacheKey, Value = WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> >, Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> > >, HashFunctions = WebCore::FontPlatformDataCacheKeyHash, Traits = WTF::HashMapValueTraits<WebCore::FontPlatformDataCacheKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::FontPlatformData> > >, KeyTraits = WebCore::FontPlatformDataCacheKeyTraits] 1 0x2b82fefeb70f WTF::HashTableConstIterator<WebCore::FontPlatformDataCacheKey, WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyHash, WTF::HashMapValueTraits<WebCore::FontPlatformDataCacheKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyTraits>::checkValidity() const 2 0x2b82fefe90d8 WTF::HashTableConstIterator<WebCore::FontPlatformDataCacheKey, WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyHash, WTF::HashMapValueTraits<WebCore::FontPlatformDataCacheKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyTraits>::get() const 3 0x2b82fefe644c WTF::HashTableIterator<WebCore::FontPlatformDataCacheKey, WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyHash, WTF::HashMapValueTraits<WebCore::FontPlatformDataCacheKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyTraits>::get() const 4 0x2b82fefe4928 WTF::HashTableIterator<WebCore::FontPlatformDataCacheKey, WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::FontPlatformDataCacheKey, WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyHash, WTF::HashMapValueTraits<WebCore::FontPlatformDataCacheKeyTraits, WTF::HashTraits<WTF::OwnPtr<WebCore::FontPlatformData> > >, WebCore::FontPlatformDataCacheKeyTraits>::operator->() const 5 0x2b82fefe2cec WebCore::FontCache::getCachedFontPlatformData(WebCore::FontDescription const&, WTF::AtomicString const&, bool) 6 0x2b82fefe2d68 WebCore::FontCache::getCachedFontData(WebCore::FontDescription const&, WTF::AtomicString const&, bool, WebCore::FontCache::ShouldRetain) 7 0x2b82fe72406c 8 0x2b82fe724494 WebCore::CSSFontSelector::getFontData(WebCore::FontDescription const&, WTF::AtomicString const&) 9 0x2b82fefe39d8 WebCore::FontCache::getFontData(WebCore::Font const&, int&, WebCore::FontSelector*) 10 0x2b82feff00cb WebCore::FontFallbackList::fontDataAt(WebCore::Font const*, unsigned int) const 11 0x2b82febc5384 WebCore::FontFallbackList::primaryFontData(WebCore::Font const*) const 12 0x2b82febc5335 WebCore::FontFallbackList::primarySimpleFontData(WebCore::Font const*) 13 0x2b82febc545a WebCore::Font::primaryFont() const 14 0x2b82febc53ac WebCore::Font::fontMetrics() const 15 0x2b82ff346c86 WebCore::RenderStyle::fontMetrics() const 16 0x2b82ff346f72 WebCore::RenderStyle::computedLineHeight(WebCore::RenderView*) const 17 0x2b82ff156de6 WebCore::RenderBlock::lineHeight(bool, WebCore::LineDirectionMode, WebCore::LinePositionMode) const 18 0x2b82ff18a4e3 19 0x2b82ff19bd86 WebCore::LineWidth::updateAvailableWidth(WebCore::LayoutUnit) 20 0x2b82ff19bba6 WebCore::LineWidth::LineWidth(WebCore::RenderBlock*, bool, WebCore::IndentTextOrNot) 21 0x2b82ff19542a WebCore::RenderBlock::LineBreaker::nextSegmentBreak(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo&, WebCore::RenderBlock::RenderTextInfo&, WebCore::RenderBlock::FloatingObject*, unsigned int, WTF::Vector<WebCore::WordMeasurement, 64ul>&) 22 0x2b82ff194f65 WebCore::RenderBlock::LineBreaker::nextLineBreak(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo&, WebCore::RenderBlock::RenderTextInfo&, WebCore::RenderBlock::FloatingObject*, unsigned int, WTF::Vector<WebCore::WordMeasurement, 64ul>&) 23 0x2b82ff18fb07 WebCore::RenderBlock::layoutRunsAndFloatsInRange(WebCore::LineLayoutState&, WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::InlineIterator const&, WebCore::BidiStatus const&, unsigned int) 24 0x2b82ff18f3fd WebCore::RenderBlock::layoutRunsAndFloats(WebCore::LineLayoutState&, bool) 25 0x2b82ff191dcc WebCore::RenderBlock::layoutInlineChildren(bool, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 26 0x2b82ff13bb95 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) 27 0x2b82ff13b143 WebCore::RenderBlock::layout() 28 0x2b82ff14092e WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, WebCore::LayoutUnit&, WebCore::LayoutUnit&) 29 0x2b82ff14052a WebCore::RenderBlock::layoutBlockChildren(bool, WebCore::LayoutUnit&) 30 0x2b82ff13bbb6 WebCore::RenderBlock::layoutBlock(bool, WebCore::LayoutUnit) 31 0x2b82ff13b143 WebCore::RenderBlock::layout()
Proper gdb backtrace: http://pastebin.com/dVhBAXhy
Comment on attachment 192378 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192378&action=review > Source/WebCore/platform/graphics/FontCache.cpp:213 > + FontPlatformData* fontPlatformDataForAlternateName = getCachedFontPlatformData(fontDescription, alternateName, true); This is a recursive call to getCachedFontPlatformData(). Are we sure the iterator stays valid after the call?
(In reply to comment #21) > (From update of attachment 192378 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192378&action=review > > > Source/WebCore/platform/graphics/FontCache.cpp:213 > > + FontPlatformData* fontPlatformDataForAlternateName = getCachedFontPlatformData(fontDescription, alternateName, true); > > This is a recursive call to getCachedFontPlatformData(). Are we sure the iterator stays valid after the call? I uploaded a follow-up fix at Bug 113954.