WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
22966
crash when destroying a webview that opened a page containing <script> tags
https://bugs.webkit.org/show_bug.cgi?id=22966
Summary
crash when destroying a webview that opened a page containing <script> tags
Gustavo Noronha (kov)
Reported
2008-12-22 11:27:07 PST
WebKit/GTK+ crashes when destroying a WebView that has opened a page containing a <script> tag. Here's how to reproduce it: open GtkLauncher, go to
http://google.com/
or the attached test html. If you try to close GtkLauncher now it will crash with segmentation fault. Removing the empty <script> tag from the test HTML is enough to make the crash go away. I have tried this with SVN revision 39434. WebKit was built using the following configure line: ./configure --prefix=/usr \ --host=i486-linux-gnu \ --build=i486-linux-gnu \ --with-http-backend=soup \ --with-font-backend=pango \ --disable-dashboard-support \ --enable-debug --enable-video It doesn't seem to crash when building with debug disabled. I'm attaching test cases. I'm not really sure this problem is specific to GTK+, I didn't try with other ports, though.
Attachments
simple test case
(666 bytes, text/x-csrc)
2008-12-22 11:29 PST
,
Gustavo Noronha (kov)
no flags
Details
simple test html
(285 bytes, text/html)
2008-12-22 11:29 PST
,
Gustavo Noronha (kov)
no flags
Details
back trace
(14.52 KB, text/plain)
2008-12-22 11:32 PST
,
Gustavo Noronha (kov)
no flags
Details
fixes the crash
(1.92 KB, patch)
2009-02-10 11:23 PST
,
Gustavo Noronha (kov)
darin
: review-
Details
Formatted Diff
Diff
proposed patch
(1.54 KB, patch)
2009-02-10 15:47 PST
,
Gustavo Noronha (kov)
darin
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Gustavo Noronha (kov)
Comment 1
2008-12-22 11:29:18 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`
Gustavo Noronha (kov)
Comment 2
2008-12-22 11:29:42 PST
Created
attachment 26209
[details]
simple test html
Gustavo Noronha (kov)
Comment 3
2008-12-22 11:32:26 PST
Created
attachment 26210
[details]
back trace
Gustavo Noronha (kov)
Comment 4
2009-02-10 11:23:42 PST
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.
Gustavo Noronha (kov)
Comment 5
2009-02-10 12:24:24 PST
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
Darin Adler
Comment 6
2009-02-10 12:41:37 PST
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.
Gustavo Noronha (kov)
Comment 7
2009-02-10 15:12:00 PST
(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.
Gustavo Noronha (kov)
Comment 8
2009-02-10 15:47:58 PST
Created
attachment 27545
[details]
proposed patch ok, this basically implements what Darin asks for, which also fixes the bug
Darin Adler
Comment 9
2009-02-10 16:05:50 PST
Comment on
attachment 27545
[details]
proposed patch r=me
Gustavo Noronha (kov)
Comment 10
2009-02-17 04:35:10 PST
Landed in
r41037
.
Jan Alonzo
Comment 11
2009-02-17 11:50:32 PST
***
Bug 22024
has been marked as a duplicate of this bug. ***
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug