Bug 49084 – Crash in documentWillBecomeInactive() when closing window with Reader showing content with SVG

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

[Matthew Delaney]

Crash from these steps:
1. Load http://webkit.org/blog/1273/the-html5-parsing-algorithm/
2. Enter Reader
3. Close window.

Currently only seeing this in a debug build of webkit.
<rdar://problem/8422888>

[Matthew Delaney]

Created attachment 73095 [details]
Simplified testcase

Here's a simplified testcase using iframes.

[Matthew Delaney]

I lied. That testcase doesn't work. I have the code fix, but still working on creating a reduced testcase for the bots to use.

[Alexey Proskuryakov]

Could you please attach a crash log?

[Alexey Proskuryakov]

Created attachment 73403 [details]
crash log

Attaching crash log. Unlike Matt's, it doesn't have documentWillBecomeInactive() in top frame - perhaps just a tools difference.

[Alexey Proskuryakov]

This is a regression on this page - shipping WebKit (of course) doesn't parse svg in html, and doesn't crash.

[Simon Fraser (smfr)]

Matt has a patch, but is still working on the testcase I think.

[Matthew Delaney]

Yea, I'll just the post the patch in a second w/o "r?". I quit on Friday evening trying to make a test case and will try again today.

[Matthew Delaney]

Created attachment 73517 [details]
Patch

[Alexey Proskuryakov]

Manual tests are almost useless, they are rarely if ever run.

[Nikolas Zimmermann]

(In reply to comment #9)
> Manual tests are almost useless, they are rarely if ever run.

Agreed. Matthew, can you try reproducing the crash using location.reload, maybe? Not sure if we have existing tests covering this...

[Matthew Delaney]

Committed r71772: <http://trac.webkit.org/changeset/71772>

[Matthew Delaney]

Forgot to mention, but I created this bug (and had it blocking on this one) for getting an eventual automated version of the manual test. https://bugs.webkit.org/show_bug.cgi?id=49346

So far, all the ideas from #ksvg and others haven't panned out, but I'm sure there's some clever way. Perhaps having a way in the layout tests to create a new window, load the manual test, and then close the window and see if it crashes would be another idea. I'm not aware of any current tests doing anything like this.