Bug 64839 - ASSERT (and crash) with dynamically moved <font-face>
Summary: ASSERT (and crash) with dynamically moved <font-face>
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-07-19 17:25 PDT by Tim Horton
Modified: 2012-05-27 08:45 PDT (History)
8 users (show)

See Also:


Attachments
repro (421 bytes, text/html)
2011-07-19 17:25 PDT, Tim Horton
no flags Details
Backtrace (10.90 KB, text/plain)
2011-07-20 13:34 PDT, Tim Horton
no flags Details
Patch (3.14 KB, patch)
2011-07-21 15:03 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (3.11 KB, patch)
2012-02-16 15:08 PST, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (3.12 KB, patch)
2012-02-17 07:55 PST, Rob Buis
koivisto: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2011-07-19 17:25:08 PDT
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
Comment 1 Tim Horton 2011-07-20 13:34:54 PDT
Created attachment 101503 [details]
Backtrace
Comment 2 Rob Buis 2011-07-21 15:03:04 PDT
Created attachment 101647 [details]
Patch
Comment 3 Nikolas Zimmermann 2011-07-21 23:46:28 PDT
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.
Comment 4 Nikolas Zimmermann 2011-07-21 23:46:51 PDT
Antti, could you have a look?
Comment 5 Rob Buis 2011-07-22 04:08:07 PDT
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.
Comment 6 Eric Seidel (no email) 2012-02-16 14:24:31 PST
Comment on attachment 101647 [details]
Patch

Seems OK.  I also don't see a cleaner way to clear it.
Comment 7 Rob Buis 2012-02-16 15:08:09 PST
Created attachment 127451 [details]
Patch
Comment 8 Rob Buis 2012-02-16 15:09:29 PST
Uploading to see if it regresses anything.
Comment 9 Philippe Normand 2012-02-16 16:13:08 PST
Comment on attachment 127451 [details]
Patch

Attachment 127451 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11542145
Comment 10 WebKit Review Bot 2012-02-16 18:28:05 PST
Comment on attachment 127451 [details]
Patch

Attachment 127451 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11542188
Comment 11 Gyuyoung Kim 2012-02-17 06:11:54 PST
Comment on attachment 127451 [details]
Patch

Attachment 127451 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11539473
Comment 12 Rob Buis 2012-02-17 07:55:06 PST
Created attachment 127589 [details]
Patch
Comment 13 Rob Buis 2012-05-27 08:45:56 PDT
Committed r108097: <http://trac.webkit.org/changeset/108097>