I've attached a script that will append and remove a script tag to a div over the course of 10 seconds. When you run it in Safari 5.0 with the Activity Monitor open, you will observe the memory usage of Safari climb steadily for the 10 seconds the script runs for. Repeat clicks will very quickly drive memory usage up over 1gig. If you take the same code and run it in Firefox 3.6.3, you will also see the memory usage go up but: 1) by a much much smaller amount 2) over repeat clicks Firefox's memory usage will suddenly drop back down, and will not continue to climb endlessly. This problem exists also in WebKit nightly (61502).
Created attachment 59262 [details] test case for bug
These script elements aren't garbage collected, there were over 125000 left alive after the test finished for me.
<rdar://problem/8124718>
This is caused by this code in JSDOMBinding.h: // If a wrapper is the last reference to an image or script element // that is loading but not in the document, the wrapper is observable // because it is the only thing keeping the image element alive, and if // the image element is destroyed, its load event will not fire. // FIXME: The DOM should manage this issue without the help of JavaScript wrappers. if (node->hasTagName(imgTag) && !static_cast<HTMLImageElement*>(node)->haveFiredLoadEvent()) return true; if (node->hasTagName(scriptTag) && !static_cast<HTMLScriptElement*>(node)->haveFiredLoadEvent()) return true; #if ENABLE(VIDEO) if (node->hasTagName(audioTag) && !static_cast<HTMLAudioElement*>(node)->paused()) return true; #endif
Clearly, haveFiredLoadEvent() is not the same as "is loading".
Besides this synthetic test case, this affects inline scripts, because they also don't fire the load event. Script elements with src are probably affected if load fails...
Created attachment 76598 [details] proposed fix It seems the we may get this flag wrong if src attribute is removed or changed during loading. Fixing that is something for another day. See also: bug 39628, bug 31253, bug 23372.
Attachment 76598 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7096004
Attachment 76598 [details] did not build on qt: Build output: http://queues.webkit.org/results/7160006
Attachment 76598 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7158026
Comment on attachment 76598 [details] proposed fix SVG tests are failing, need to fix.
Created attachment 76669 [details] work in progress, breaks SVG SVG seems quite complicated - when I remove the failing assertion, svg/dom/SVGScriptElement/script-load-and-error-events.svg breaks badly. I don't know the SVG spec and can't tell for sure if its expected results are correct. I suspect that SVG is not quite right: 1) The test expects onload for <script xlink:href="resources/certainlydoesnotexist.js"></script> 2) WebKit fires onload for <script> without any external resources.
Created attachment 76670 [details] SVG no subresources test
Is anyone working on the SVG block for this bug? Maybe a separate bug filed for it that I can CC myself on?
(In reply to comment #12) > Created an attachment (id=76669) [details] > work in progress, breaks SVG > > SVG seems quite complicated - when I remove the failing assertion, svg/dom/SVGScriptElement/script-load-and-error-events.svg breaks badly. I don't know the SVG spec and can't tell for sure if its expected results are correct. > > I suspect that SVG is not quite right: > 1) The test expects onload for <script xlink:href="resources/certainlydoesnotexist.js"></script> > 2) WebKit fires onload for <script> without any external resources. I haven't looked at the test in quesiton. However SVG has a separate load event "SVGLoad": http://www.w3.org/TR/2002/PR-SVG11-20021115/interact.html It's fired basically every time a tag is closed if there is a listener.
It looks like the test case here is fixed by the fix for bug 59604. *** This bug has been marked as a duplicate of bug 59604 ***
It's certainly correct to close this as a dupe, but we'll need to sort out the other bugs and issues touched by this patch eventually.