Summary: | REGRESSION: destroying a frame from its own script causes various crashes | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||||||||
Component: | WebCore JavaScript | Assignee: | Darin Adler <darin> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Critical | CC: | bdakin, mrowe | ||||||||||||
Priority: | P1 | Keywords: | Regression | ||||||||||||
Version: | 420+ | ||||||||||||||
Hardware: | Mac | ||||||||||||||
OS: | OS X 10.4 | ||||||||||||||
URL: | http://www.rhapsody.com/home.html | ||||||||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2007-01-20 01:13:07 PST
Created attachment 12566 [details]
a reduction (almost)
This reduction produces a crash in a different place (actually, two different places!), but the root problem seems to be very similar.
Alexey, your reduction doesn't crash for me with ToT. For that matter, the original page doesn't crash either. We can't hope reproduce the bug on the original site until the fix for bug 10934 is checked in. I also don't see a crash with the reduction. I also cannot reproduce this anymore. Going to keep open for a little while to investigate what happened. Created attachment 12605 [details]
a reduction (almost)
Turns out the reduction didn't cause a crash because I added a comment that broke parsing just before uploading it.
I could also reproduce the crash at rhapsody.com, but it is intermittent; not sure what triggers it.
Created attachment 12607 [details]
patch, fixes bug; need to turn test case into layout test
Comment on attachment 12607 [details]
patch, fixes bug; need to turn test case into layout test
+ // FIXME: Why not use updateDocuemntsRendering to update rendering of all documents?
Typo: "Docuemnts"
r=me
I believe this doesn't fix the rhapsody.com issue, because it happens due to loading subresources, rather that using a timer: 0 com.apple.WebCore 0x0162e3e4 WTF::OwnPtr<WebCore::Chrome>::get() const + 20 (OwnPtr.h:36) 1 com.apple.WebCore 0x0162e420 WebCore::Page::chrome() const + 40 (Page.h:83) 2 com.apple.WebCore 0x013255c4 WebCore::KJSProxy::evaluate(WebCore::String const&, int, WebCore::String const&, WebCore::Node*) + 1212 (kjs_proxy.cpp:75) 3 com.apple.WebCore 0x014eb218 WebCore::FrameLoader::executeScript(WebCore::String const&, int, WebCore::Node*, WebCore::String const&) + 136 (FrameLoader.cpp:684) 4 com.apple.WebCore 0x0102583c WebCore::HTMLTokenizer::scriptExecution(WebCore::DeprecatedString const&, WebCore::HTMLTokenizer::State, WebCore::DeprecatedString, int) + 392 (HTMLTokenizer.cpp:501) 5 com.apple.WebCore 0x0102720c WebCore::HTMLTokenizer::notifyFinished(WebCore::CachedResource*) + 624 (HTMLTokenizer.cpp:1660) 6 com.apple.WebCore 0x01157f60 WebCore::CachedScript::checkNotify() + 108 (CachedScript.cpp:91) 7 com.apple.WebCore 0x011580a0 WebCore::CachedScript::data(WTF::Vector<char, (unsigned long)0>&, bool) + 180 (CachedScript.cpp:83) 8 com.apple.WebCore 0x0115a544 WebCore::Loader::didFinishLoading(WebCore::SubresourceLoader*) + 340 (loader.cpp:108) 9 com.apple.WebCore 0x014f3510 WebCore::SubresourceLoader::didFinishLoading() + 204 (SubresourceLoader.cpp:166) 10 com.apple.WebCore 0x014f1854 WebCore::ResourceLoader::didFinishLoading(WebCore::ResourceHandle*) + 60 11 com.apple.WebCore 0x014c87f0 -[WebCoreResourceHandleAsDelegate connectionDidFinishLoading:] + 144 (ResourceHandleMac.mm:368) 12 com.apple.Foundation 0x9298f84c -[NSURLConnection(NSURLConnectionInternal) _sendDidFinishLoadingCallback] + 188 13 com.apple.Foundation 0x9298dab8 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 556 14 com.apple.Foundation 0x9298d810 _sendCallbacks + 156 15 com.apple.CoreFoundation 0x907dd4cc __CFRunLoopDoSources0 + 384 16 com.apple.CoreFoundation 0x907dc9fc __CFRunLoopRun + 452 17 com.apple.CoreFoundation 0x907dc47c CFRunLoopRunSpecific + 268 18 com.apple.HIToolbox 0x93203740 RunCurrentEventLoopInMode + 264 19 com.apple.HIToolbox 0x93202dd4 ReceiveNextEventCommon + 380 20 com.apple.HIToolbox 0x93202c40 BlockUntilNextEventMatchingListInMode + 96 21 com.apple.AppKit 0x93706ae4 _DPSNextEvent + 384 22 com.apple.AppKit 0x937067a8 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 116 23 com.apple.Safari 0x00006740 0x1000 + 22336 24 com.apple.AppKit 0x93702cec -[NSApplication run] + 472 25 com.apple.AppKit 0x937f387c NSApplicationMain + 452 Yes, that's another problem not seen in the reduction. Also easy to fix (just need to 0-check the page), but needs another test case I guess. Still trying to make a test case for the actual rhapsody.com bug. (In reply to comment #10) > Also easy to fix (just need to 0-check the page), but needs another test case I guess. Actually, I'm not sure if it's good for addMessageToConsole() to depend on frame or page existence at all - it would probably be better for messages to be logged even if the page is gone. (In reply to comment #12) > Actually, I'm not sure if it's good for addMessageToConsole() to depend on > frame or page existence at all - it would probably be better for messages to be > logged even if the page is gone. I understand what you're saying, but on the other hand, WebKit is not supposed to do things like logging directly. It needs to communicate log messages to the client so the client can determine where to put them. No page, no client. We don't have a global WebKit client. Feel free to file a separate bug about your concern that we will miss log messages and maybe we can solve that problem. Comment on attachment 12607 [details]
patch, fixes bug; need to turn test case into layout test
Clearing the review flag because I don't have a test.
Created attachment 12662 [details]
improved patch, but still no layout test
(In reply to comment #15) > Created an attachment (id=12662) [edit] > improved patch, but still no layout test I know how to test this, if you need help. CC Beth: I guess we don't need to file this bug, afterall! Created attachment 12667 [details]
a real reduction
So, I was finally able to make a test that demonstrates the original problem! Many thanks to ggaren for inspiration :)
(In reply to comment #13) > Feel free to file a separate bug about your > concern that we will miss log messages and maybe we can solve that problem. Done, bug 12406. Committed revision 19130. |