RESOLVED FIXED Bug 19588
CRASH doing open() on destroyed window 
https://bugs.webkit.org/show_bug.cgi?id=19588
Summary CRASH doing open() on destroyed window
Eric Roman
Reported 2008-06-16 10:18:59 PDT
Remove iframe from parent, and then do a open() on its contentWindow -- reproducible crash on Safari 3.1 and nightly.
Attachments
Reproducible test case for crash -- click the button (461 bytes, text/html)
2008-06-16 10:20 PDT, Eric Roman 		no flags
Details
Crash dump from running the above test on nightly webkit r34581 (27.77 KB, text/plain)
2008-06-16 10:23 PDT, Eric Roman 		no flags
Details
Proposed fix: add null check (3.31 KB, patch)
2008-07-21 16:57 PDT, Julien Chaffraix 		eric: review-
DetailsFormatted DiffDiff
Updated version: use top() and cover the other crash pointed by Eric (6.75 KB, patch)
2008-07-22 14:21 PDT, Julien Chaffraix 		eric: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Roman
Comment 1 2008-06-16 10:20:26 PDT
Created attachment 21730 [details] Reproducible test case for crash -- click the button
Eric Roman
Comment 2 2008-06-16 10:23:01 PDT
Created attachment 21731 [details] Crash dump from running the above test on nightly webkit r34581
Mark Rowe (bdash)
Comment 3 2008-06-16 13:46:02 PDT
<rdar://problem/6012706>
Sam Weinig
Comment 4 2008-07-16 15:19:29 PDT
This is not a new crash. Using bisect-builds, I was able to reproduce this all the way back to r29753, the beginning of builds available to the tool.
Julien Chaffraix
Comment 5 2008-07-21 16:57:30 PDT
Created attachment 22415 [details] Proposed fix: add null check
Alexey Proskuryakov
Comment 6 2008-07-21 22:46:02 PDT
See also: bug 15707.
Eric Seidel (no email)
Comment 7 2008-07-22 08:32:59 PDT
Comment on attachment 22415 [details] Proposed fix: add null check Why shouldn't you just use "top()" here? It seems "top()" wouldn't crash. Seems like the end of the function doesn't check against null pages either... so that will need to be fixed and a test case added too.
Julien Chaffraix
Comment 8 2008-07-22 14:21:05 PDT
Created attachment 22432 [details] Updated version: use top() and cover the other crash pointed by Eric > Why shouldn't you just use "top()" here? It seems "top()" wouldn't crash. top() is indeed fine. > Seems like the end of the function doesn't check against null pages either... > so that will need to be fixed and a test case added too. True, I have fixed that too including a layout test.
Eric Seidel (no email)
Comment 9 2008-07-23 13:17:34 PDT
Comment on attachment 22432 [details] Updated version: use top() and cover the other crash pointed by Eric Looks fine, but: if (page) { 184 for (Frame* frame = page should change to be an early return: if (!page) return 0;
Julien Chaffraix
Comment 10 2008-07-24 00:47:06 PDT
Updated with Eric's comment and landed in r35321.
Note You need to log in before you can comment on or make changes to this bug.