RESOLVED FIXED 32353
Add a manual test for crash in DOMWindow::clearTimeout when DOMWindow is not connected to frame
https://bugs.webkit.org/show_bug.cgi?id=32353
Summary Add a manual test for crash in DOMWindow::clearTimeout when DOMWindow is not ...
Dmitry Titov
Reported 2009-12-09 16:25:45 PST
Patch for bug 29832 fixed a crash in Safari 4 caused by not resetting a JSDOMWindow wrapper on a detached frame in some cases. The test reproducing the crash can not be added as layout test because it takes ~10 seconds to run. Need to add it as a manual test.
Attachments
Proposed patch. (1.79 KB, patch)
2009-12-09 17:12 PST, Dmitry Titov
oliver: review+
dimich: commit-queue-
Dmitry Titov
Comment 1 2009-12-09 17:12:21 PST
Created attachment 44583 [details] Proposed patch.
WebKit Review Bot
Comment 2 2009-12-09 17:13:03 PST
style-queue ran check-webkit-style on attachment 44583 [details] without any errors.
Oliver Hunt
Comment 3 2009-12-09 19:06:38 PST
Comment on attachment 44583 [details] Proposed patch. r=me
Alexey Proskuryakov
Comment 4 2009-12-09 19:15:22 PST
I don't quite understand why this needs to take ~10 seconds (what's back/forward cache expiration?), but maybe a new DRT method can make this instant?
Dmitry Titov
Comment 5 2009-12-10 14:33:21 PST
(In reply to comment #4) > I don't quite understand why this needs to take ~10 seconds (what's > back/forward cache expiration?), but maybe a new DRT method can make this > instant? Indeed, there is no way today to cause immediate b/f cache expiration so the destruction of the cached pages can not be realistically tested in DRT today. I thought about something like layoutTestController.setBackForwardCacheExpiration(seconds) but wasn't sure the single test justifies it... Perhaps we should do it next time we have a need?
Alexey Proskuryakov
Comment 6 2009-12-10 15:12:11 PST
This case seems significant enough to warrant adding a DRT method. Perhaps it would be easier to just force immediate expiration, rather than set a timeout?
Dmitry Titov
Comment 7 2009-12-10 15:39:43 PST
(In reply to comment #6) > This case seems significant enough to warrant adding a DRT method. > > Perhaps it would be easier to just force immediate expiration, rather than set > a timeout? Ok. I'll take a look at adding a method. Will delay landing the patch until implementation is ready, then we'll decide.
Eric Seidel (no email)
Comment 8 2009-12-28 23:36:46 PST
Ping? Curious what the status is here. This patch has been approved for landing for a little over 2 weeks.
Eric Seidel (no email)
Comment 9 2010-03-05 17:14:26 PST
Ping?
Dmitry Titov
Comment 10 2010-03-05 19:12:01 PST
I agree with Alexey that having manual test is somewhat useless since nobody ever runs them. I have this task (make it possibel to test the bf cache expiration from DRT in my list and will get to it at some point, in a few weeks. So for now I guess I'll commit this test so it is there, but will address the DRT testing at some point.
Dmitry Titov
Comment 11 2010-03-05 19:18:17 PST
Note You need to log in before you can comment on or make changes to this bug.