Bug 49084

Summary: Crash in documentWillBecomeInactive() when closing window with Reader showing content with SVG
Product: WebKit Reporter: Matthew Delaney <mdelaney7>
Component: SVGAssignee: Matthew Delaney <mdelaney7>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, mdelaney7, simon.fraser, zimmermann
Priority: P1 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.6   
Bug Depends on:    
Bug Blocks: 49346    
Attachments:
Description Flags
Simplified testcase
none
crash log
none
Patch darin: review+

Description Matthew Delaney 2010-11-05 11:51:30 PDT
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>
Comment 1 Matthew Delaney 2010-11-05 11:56:12 PDT
Created attachment 73095 [details]
Simplified testcase

Here's a simplified testcase using iframes.
Comment 2 Matthew Delaney 2010-11-05 14:04:26 PDT
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.
Comment 3 Alexey Proskuryakov 2010-11-05 23:18:35 PDT
Could you please attach a crash log?
Comment 4 Alexey Proskuryakov 2010-11-09 13:06:12 PST
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.
Comment 5 Alexey Proskuryakov 2010-11-09 13:07:33 PST
This is a regression on this page - shipping WebKit (of course) doesn't parse svg in html, and doesn't crash.
Comment 6 Simon Fraser (smfr) 2010-11-09 13:08:03 PST
Matt has a patch, but is still working on the testcase I think.
Comment 7 Matthew Delaney 2010-11-09 13:09:58 PST
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.
Comment 8 Matthew Delaney 2010-11-10 11:20:22 PST
Created attachment 73517 [details]
Patch
Comment 9 Alexey Proskuryakov 2010-11-10 11:39:38 PST
Manual tests are almost useless, they are rarely if ever run.
Comment 10 Nikolas Zimmermann 2010-11-10 11:59:14 PST
(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...
Comment 11 Matthew Delaney 2010-11-10 14:45:04 PST
Committed r71772: <http://trac.webkit.org/changeset/71772>
Comment 12 Matthew Delaney 2010-11-18 15:16:51 PST
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.