Bug 64839 – ASSERT (and crash) with dynamically moved <font-face>

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Tim Horton]

Created attachment 101411 [details]
repro

Steps to Reproduce:

Open attached document in a WebKit browser.

Expected result:

Not sure, but not a crash.

Actual result:

Crash in release build, assertion failure in debug build.

rdar://problem/9516492

[Tim Horton]

Created attachment 101503 [details]
Backtrace

[Rob Buis]

Created attachment 101647 [details]
Patch

[Nikolas Zimmermann]

Comment on attachment 101647 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=101647&action=review

> Source/WebCore/ChangeLog:8
> +        Reset the style declaration when rmeoving the font-face element from the document.

typo: removing.

> Source/WebCore/svg/SVGFontFaceElement.cpp:332
> +    m_styleDeclaration->parseDeclaration(emptyString());

Hm, I'm not sure whether this is the best way to fix it.
I'll CC Antti who may judge better.

[Nikolas Zimmermann]

Antti, could you have a look?

[Rob Buis]

Hi Niko,

(In reply to comment #3)
> (From update of attachment 101647 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=101647&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Reset the style declaration when rmeoving the font-face element from the document.
> 
> typo: removing.

Will fix.

> > Source/WebCore/svg/SVGFontFaceElement.cpp:332
> > +    m_styleDeclaration->parseDeclaration(emptyString());
> 
> Hm, I'm not sure whether this is the best way to fix it.
> I'll CC Antti who may judge better.

For background, the m_styleDeclaration in it contains bad font data
after the SVGFontFaceElement is removed from the doc. So since this
font data is useless anyway as soon as SVGFontFaceElement is removed,
this was the quickest way I found to clear it without actually
destroying the m_styleDeclaration.
Cheers,

Rob.

[Eric Seidel (no email)]

Comment on attachment 101647 [details]
Patch

Seems OK.  I also don't see a cleaner way to clear it.

[Rob Buis]

Created attachment 127451 [details]
Patch

[Rob Buis]

Uploading to see if it regresses anything.

[Philippe Normand]

Comment on attachment 127451 [details]
Patch

Attachment 127451 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11542145

[WebKit Review Bot]

Comment on attachment 127451 [details]
Patch

Attachment 127451 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11542188

[Gyuyoung Kim]

Comment on attachment 127451 [details]
Patch

Attachment 127451 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11539473

[Rob Buis]

Created attachment 127589 [details]
Patch

[Rob Buis]

Committed r108097: <http://trac.webkit.org/changeset/108097>