Bug 15707 – Crash when manipulating document from within an iframe onload function

• Status: RESOLVED
• Resolution: FIXED
• Component: Frames

[Alexey Proskuryakov]

Several W3C DOM tests would crash for us if not masked by another bug. Reduction forthcoming.

[Alexey Proskuryakov]

Created attachment 16875 [details]
test case

[Alexey Proskuryakov]

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.

[David Kilzer (:ddkilzer)]

<rdar://problem/5602310>

[Alexey Proskuryakov]

Created attachment 27751 [details]
proposed fix

[Alexey Proskuryakov]

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

[Darin Adler]

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.

[Eric Seidel (no email)]

I could have sworn we filed (and possibly fixed a) dupe of this for Chromium...?  At least this bug sounds very familiar to me.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/41133>.

> At least this bug sounds very familiar to me.

See comment 5 :-)

[Alexey Proskuryakov]

> Committed <http://trac.webkit.org/changeset/41133>.

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