Bug 11729 – REGRESSION: Crash closing page with frames after selection

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Matt Lilek]

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.

[Matt Lilek]

Created attachment 11699 [details]
testcase

[Matt Lilek]

Created attachment 11700 [details]
crashlog

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]

I'll have a look.

[Geoffrey Garen]

Hmmm... I can't reproduce this with r17994.

[Matt Lilek]

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.

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]

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)]

*** Bug 11863 has been marked as a duplicate of this bug. ***

[David Kilzer (:ddkilzer)]

This bug affects GMail as well.  See Bug 11863.

[David Kilzer (:ddkilzer)]

(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).

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 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)]

Committed revision 18460.