Summary: | crash when destroying a webview that opened a page containing <script> tags | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> | ||||||||||||
Component: | WebKitGTK | Assignee: | Gustavo Noronha (kov) <gustavo> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Critical | CC: | jmalonzo | ||||||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
Attachments: |
|
Description
Gustavo Noronha (kov)
2008-12-22 11:27:07 PST
Created attachment 26208 [details]
simple test case
build with:
gcc -g -o test-sigsegv test-sigsegv.c `pkg-config --cflags --libs webkit-1.0`
Created attachment 26209 [details]
simple test html
Created attachment 26210 [details]
back trace
Created attachment 27531 [details]
fixes the crash
It seems like there is this one condition in which m_group can be made NULL while m_singlePageGroup is left as it is, which causes the ASSERT on initGroup to fail, causing the crash.
Small backtrace of when setGroupName is called with an empty string: #0 WebCore::Page::setGroupName (this=0x96a4488, name=@0xbf8c3928) at ../../WebCore/page/Page.cpp:230 __PRETTY_FUNCTION__ = "void WebCore::Page::setGroupName(const WebCore::String&)" #1 0xb717d419 in ~Page (this=0x96a4488) at ../../WebCore/page/Page.cpp:158 No locals. #2 0xb6dc702b in webkit_web_view_dispose (object=0x96a4828) at ../../WebKit/gtk/webkit/webkitwebview.cpp:823 webView = (WebKitWebView *) 0x96a4828 priv = (WebKitWebViewPrivate *) 0x96a4870 Comment on attachment 27531 [details]
fixes the crash
Why would we want to clear m_singlePageGroup in this case? If the group name is empty then this item should still be in the single page group, so you wouldn't want to clear it. Also, m_singlePageGroup is an OwnPtr so it will be initialized to 0, so the change to Page is unneeded and hence wrong; we left that out on purpose.
I think the right fix is to set m_group to m_singlePageGroup.get() rather than to 0.
(In reply to comment #6) > I think the right fix is to set m_group to m_singlePageGroup.get() rather than > to 0. OK, that makes sense to me. I'll upload a new patch soon. Do you think a LayoutTest is required for this? I meant to write one, but didn't see it fitting anywhere, and couldn't find a good description, nor a good way to write the test. Created attachment 27545 [details]
proposed patch
ok, this basically implements what Darin asks for, which also fixes the bug
Comment on attachment 27545 [details]
proposed patch
r=me
Landed in r41037. *** Bug 22024 has been marked as a duplicate of this bug. *** |