Bug 11729

Summary: REGRESSION: Crash closing page with frames after selection
Product: WebKit Reporter: Matt Lilek <dev+webkit>
Component: New BugsAssignee: Geoffrey Garen <ggaren>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, ddkilzer, ggaren, miles_b, mitz
Priority: P1 Keywords: GoogleBug, HasReduction, Regression
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
testcase
none
crashlog
none
Minimal fix darin: review+

Matt Lilek
Reported 2006-11-30 16:41:59 PST
This is very similar to bug 11710. Open the testcase, select something in one of the frames, close the tab/window and WebKit will crash.
Attachments
testcase (295 bytes, text/html)
2006-11-30 16:43 PST, Matt Lilek
no flags
crashlog (21.43 KB, text/plain)
2006-11-30 16:45 PST, Matt Lilek
no flags
Minimal fix (2.23 KB, patch)
2006-12-27 01:22 PST, mitz
darin: review+
Matt Lilek
Comment 1 2006-11-30 16:43:43 PST
Created attachment 11699 [details] testcase
Matt Lilek
Comment 2 2006-11-30 16:45:17 PST
Created attachment 11700 [details] crashlog
mitz
Comment 3 2006-12-01 00:47:46 PST
The crash happens because the subframe points to a Page that's already been destroyed. The Page destructor traverses the frame tree and calls pageDestroyed() on every subframe, but that's too late since -[_webView close] tears down the tree before deleting the Page, so the Page destructor actually doesn't reach any subframes. It seems to me that Frame::pageDestroyed() could be Frame::detachedFromTree() and be called when the tree is torn down.
Geoffrey Garen
Comment 4 2006-12-03 17:29:07 PST
I'll have a look.
Geoffrey Garen
Comment 5 2006-12-03 18:10:17 PST
Hmmm... I can't reproduce this with r17994.
Matt Lilek
Comment 6 2006-12-03 22:41:08 PST
I can still get this to happen in the r17993 nightly. The most reliable way I've found is to launch the nightly, open a new tab, go directly to the attachment on this bug, select the text and close the tab which will cause a beachball and crash.
mitz
Comment 7 2006-12-03 22:48:06 PST
With a debug build you must have another window open in order to reproduce, because of this: if (allPages->isEmpty()) { Frame::endAllLifeSupport(); #ifndef NDEBUG // Force garbage collection, to release all Nodes before exiting. m_mainFrame = 0; #endif } Clearing m_mainFrame causes it (and the subframe it's holding onto via EventHandler) to be deleted while the Page is still valid.
Miles Bainbridge
Comment 8 2006-12-16 22:11:50 PST
I'm getting this exact crash in build 18260. I'm experiencing this in an Apple internal implementation of PeopleSoft 8. It seems to be 100% reproducible in this situation, though so far I've only tested it on a PPC machine. Due to the nature of the crash, it makes this build of WebKit unusable for PeopleSoft 8-based databases.
David Kilzer (:ddkilzer)
Comment 9 2006-12-17 18:04:48 PST
*** Bug 11863 has been marked as a duplicate of this bug. ***
David Kilzer (:ddkilzer)
Comment 10 2006-12-17 18:07:39 PST
This bug affects GMail as well. See Bug 11863.
David Kilzer (:ddkilzer)
Comment 11 2006-12-20 20:07:52 PST
(In reply to comment #9) > *** Bug 11863 has been marked as a duplicate of this bug. *** (In reply to comment #10) > This bug affects GMail as well. See Bug 11863. See Bug 11859 Comment #6 for a different stack trace when closing a GMail compose window using a locally-built debug build of WebKit r18370 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L127).
mitz
Comment 12 2006-12-27 01:22:19 PST
Created attachment 12055 [details] Minimal fix I realize that you are going to address this bug as part of some bigger change, but please consider this fix for now. I don't know how to make an automated test.
Darin Adler
Comment 13 2006-12-27 22:17:09 PST
Comment on attachment 12055 [details] Minimal fix The function name pageDestroyed is no longer a good one if it's used in these cases where the page is not being destroyed. And if it's an error to call setView(0) without also calling pageDestroyed(), perhaps a little refactoring of that is in order to make that mistake impossible. Since FrameLoader::detachFromParent does some of the same kind of work that removeChild does, I'd like to see some refactoring so that code does not exist in two different place. It's a bad thing that the page's frame count is now managed both by Frame and FrameTree -- that should not be a shared responsibility. Still, r=me, because we want this fixed in the tree; lets see if we can clean things up a bit after the fact. If maybe Geoff is doing some bigger fix maybe that will do the trick.
David Kilzer (:ddkilzer)
Comment 14 2006-12-28 15:48:39 PST
Committed revision 18460.
Note You need to log in before you can comment on or make changes to this bug.