Summary: | Crash when manipulating document from within an iframe onload function | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||
Component: | Frames | Assignee: | Alexey Proskuryakov <ap> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | eric | ||||||
Priority: | P1 | Keywords: | InRadar | ||||||
Version: | 523.x (Safari 3) | ||||||||
Hardware: | Mac | ||||||||
OS: | OS X 10.4 | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 15690 | ||||||||
Attachments: |
|
Description
Alexey Proskuryakov
2007-10-26 02:14:11 PDT
Created attachment 16875 [details]
test case
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. Created attachment 27751 [details]
proposed fix
*** Bug 18803 has been marked as a duplicate of this bug. *** 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.
I could have sworn we filed (and possibly fixed a) dupe of this for Chromium...? At least this bug sounds very familiar to me. Committed <http://trac.webkit.org/changeset/41133>. > At least this bug sounds very familiar to me. See comment 5 :-) > Committed <http://trac.webkit.org/changeset/41133>. And <http://trac.webkit.org/changeset/41134>. |