Bug 86715

Summary: [Chromium] REGRESSION: Assertion failure on svg/custom/acid3-test-77.html
Product: WebKit Reporter: Hajime Morrita <morrita>
Component: SVGAssignee: Stephen Chenney <schenney>
Severity: Normal CC: mitz, pdr, schenney, thorton, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Description Flags
Patch zimmermann: review+, webkit.review.bot: commit-queue-

Description Hajime Morrita 2012-05-17 01:43:39 PDT
Culprit is: http://trac.webkit.org/changeset/105057
Note that this doesn't crash on Apple Mac.

I didn't revert the original change since it also fixed another problem...

Here is a stacktrace:
00:33:18.801 14746   ASSERTION FAILED: m_purgePreventCount
00:33:18.801 14746   third_party/WebKit/Source/WebCore/platform/graphics/FontCache.cpp(280) : WebCore::SimpleFontData* WebCore::FontCache::getCachedFontData(const WebCore::FontPlatformData*, WebCore::FontCache::ShouldRetain)
00:33:18.804 14746   [15999:15999:4617000855690:ERROR:process_util_posix.cc(143)] Received signal 11
00:33:18.804 14746   	base::debug::StackTrace::StackTrace() [0x8625be]
00:33:18.804 14746   	base::(anonymous namespace)::StackDumpSignalHandler() [0x822729]
00:33:18.804 14746   	0x7f0333922af0
00:33:18.804 14746   	WebCore::FontCache::getCachedFontData() [0x1103fcc]
00:33:18.804 14746   	WebCore::FontCache::getFontDataForCharacters() [0x11d2f2c]
00:33:18.804 14746   	WebCore::Font::glyphDataAndPageForCharacter() [0x1112631]
00:33:18.804 14746   	WebCore::Font::glyphDataForCharacter() [0x1111c34]
00:33:18.805 14746   	WebCore::WidthIterator::glyphDataForCharacter() [0x11491d8]
00:33:18.805 14746   	WebCore::WidthIterator::advance() [0x114942c]
00:33:18.805 14746   	WebCore::SVGTextMetricsBuilder::advanceSimpleText() [0x1daf29a]
00:33:18.805 14746   	WebCore::SVGTextMetricsBuilder::advance() [0x1daf239]
00:33:18.805 14746   	WebCore::SVGTextMetricsBuilder::measureTextRenderer() [0x1dafb70]
00:33:18.805 14746   	WebCore::SVGTextMetricsBuilder::walkTree() [0x1dafc80]
00:33:18.805 14746   	WebCore::SVGTextMetricsBuilder::buildMetricsAndLayoutAttributes() [0x1dafe1b]
00:33:18.805 14746   	WebCore::SVGTextLayoutAttributesBuilder::buildLayoutAttributesForTextRenderer() [0x1da836b]
00:33:18.805 14746   	WebCore::RenderSVGText::subtreeChildWasAdded() [0x1d8a53d]
00:33:18.805 14746   	WebCore::RenderSVGText::addChild() [0x1d8c020]
00:33:18.805 14746   	WebCore::NodeRendererFactory::createRendererIfNeeded() [0x774be0]
00:33:18.805 14746   	WebCore::Node::createRendererIfNeeded() [0x7563c1]
00:33:18.806 14746   	WebCore::Text::attach() [0x7a709a]
00:33:18.806 14746   	WebCore::updateTreeAfterInsertion() [0x6c255d]
00:33:18.806 14746   	WebCore::ContainerNode::appendChild() [0x6c05cd]
00:33:18.806 14746   	WebCore::Node::setTextContent() [0x758919]
00:33:18.806 14746   	WebCore::NodeV8Internal::textContentAttrSetter() [0x1cd152c]
00:33:18.806 14746   	v8::internal::JSObject::SetPropertyWithCallback() [0xc82ba7]
00:33:18.806 14746   	v8::internal::JSObject::SetPropertyForResult() [0xc86589]
00:33:18.806 14746   	v8::internal::JSReceiver::SetProperty() [0xc84b29]
00:33:18.806 14746   	v8::internal::JSReceiver::SetProperty() [0xc828ac]
00:33:18.806 14746   	v8::internal::StoreIC::Store() [0xebe4dc]
00:33:18.806 14746   	v8::internal::StoreIC_Miss() [0xec113a]
00:33:18.806 14746   	0x214759a0618e
Comment 1 Nikolas Zimmermann 2012-05-17 06:23:13 PDT
Good to know, thanks Hajime! All we need is another FontCachePurgePreventer protection in subtreeChildWasAdded() -- I was unsure whether it's needed, and locally I saw no problems with that test, but anyhow - I'll create a follow-up patch to fix this.
Comment 2 Stephen Chenney 2012-05-17 09:16:01 PDT
The culprit, as Nikolas has acknowledged, is http://trac.webkit.org/changeset/117225

Funny thing is, I cannot reproduce this locally at all.
Comment 3 Philip Rogers 2012-05-17 09:17:09 PDT
Nor can I (Linux debug)
I believe schenney tried both Linux and Win debug.
Comment 4 Nikolas Zimmermann 2012-05-17 09:58:10 PDT
This assertion is known to be tricky to nail down. I'm busy with other work until tomorrow evening, so if anyone wants to take this over no problem - just a matter of adding a FontCachePurgePreventer.
Comment 5 Stephen Chenney 2012-05-17 13:47:57 PDT
I have a fix (well, Niko's fix) that seems to prevent the crash. I need to run it many times overnight, however, to be sure. So expect something tomorrow morning.

I also ascertained that you need to run the full test suite, or at least a big chunk, in order to hit the crash.
Comment 6 Hajime Morrita 2012-05-17 20:48:41 PDT
(In reply to comment #5)
> I have a fix (well, Niko's fix) that seems to prevent the crash. I need to run it many times overnight, however, to be sure. So expect something tomorrow morning.
> I also ascertained that you need to run the full test suite, or at least a big chunk, in order to hit the crash.
I think it's acceptable to land the speculative fix and see what happens
since it hurts little - the test already crashes.
Comment 7 Stephen Chenney 2012-05-18 07:30:18 PDT
Created attachment 142714 [details]
Comment 8 Stephen Chenney 2012-05-18 07:31:23 PDT
I have added font purge protections covering all cases where the SVG font metrics are cleared through the time they are rebuilt.
Comment 9 WebKit Review Bot 2012-05-18 23:08:53 PDT
Comment on attachment 142714 [details]

Rejecting attachment 142714 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
zz 3.
patching file Source/WebCore/rendering/svg/RenderSVGText.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/chromium/test_expectations.txt
Hunk #1 FAILED at 3821.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/chromium/test_expectations.txt.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Nikolas Zi..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/

Full output: http://queues.webkit.org/results/12730095
Comment 10 Stephen Chenney 2012-05-21 09:11:33 PDT
Committed r117790: <http://trac.webkit.org/changeset/117790>