Bug 15707 - Crash when manipulating document from within an iframe onload function
Summary: Crash when manipulating document from within an iframe onload function
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Frames (show other bugs)
Version: 523.x (Safari 3)
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
: 18803 (view as bug list)
Depends on:
Blocks: 15690
  Show dependency treegraph
 
Reported: 2007-10-26 02:14 PDT by Alexey Proskuryakov
Modified: 2009-02-23 01:11 PST (History)
1 user (show)

See Also:


Attachments
test case (2.92 KB, application/zip)
2007-10-26 02:14 PDT, Alexey Proskuryakov
no flags Details
proposed fix (4.82 KB, patch)
2009-02-18 09:53 PST, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-10-26 02:14:11 PDT
Several W3C DOM tests would crash for us if not masked by another bug. Reduction forthcoming.
Comment 1 Alexey Proskuryakov 2007-10-26 02:14:42 PDT
Created attachment 16875 [details]
test case
Comment 2 Alexey Proskuryakov 2007-10-26 04:27:18 PDT
Here's what is going on:
1) There are two subframes, each calls parent.open() from its onload handler.
2) As the first subframe loads and open()s its parent, the parent is destroyed, and calls willRemove() on the second frame.
3) The second frame stops loading, dispatches onload and thus calls parent.open() again!

Naturally, Document::open() causes havoc when entered recursively.
Comment 3 David Kilzer (:ddkilzer) 2007-11-15 09:15:30 PST
<rdar://problem/5602310>
Comment 4 Alexey Proskuryakov 2009-02-18 09:53:59 PST
Created attachment 27751 [details]
proposed fix
Comment 5 Alexey Proskuryakov 2009-02-18 10:06:31 PST
*** Bug 18803 has been marked as a duplicate of this bug. ***
Comment 6 Darin Adler 2009-02-22 23:34:50 PST
Comment on attachment 27751 [details]
proposed fix

r=me

Will this have any performance impact? I am not too happy about "protect". I always hate those.

Could we scope the "n" variable better?

Instead of this:

    for (n = m_firstChild; n; n = n->nextSibling())

we could do this:

    for (RefPtr<Node> n = m_firstChild; n; n = n->nextSibling())

Instead of this:

    while ((n = m_firstChild) != 0) {

we could do this:

    while (RefPtr<Node> n = m_firstChild) {

I think this would give slightly smaller/faster code because construction doesn't have to check the old value for 0.
Comment 7 Eric Seidel (no email) 2009-02-22 23:37:19 PST
I could have sworn we filed (and possibly fixed a) dupe of this for Chromium...?  At least this bug sounds very familiar to me.
Comment 8 Alexey Proskuryakov 2009-02-23 01:04:36 PST
Committed <http://trac.webkit.org/changeset/41133>.

> At least this bug sounds very familiar to me.

See comment 5 :-)
Comment 9 Alexey Proskuryakov 2009-02-23 01:11:47 PST
> Committed <http://trac.webkit.org/changeset/41133>.

And <http://trac.webkit.org/changeset/41134>.