Bug 40926 – (Blocked by SVG) high and unfreed memory usage on script tag insertion

• Component: SVG
• Status: RESOLVED DUPLICATE

[Alexander Romanovich]

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).

[Alexander Romanovich]

Created attachment 59262 [details]
test case for bug

[Alexey Proskuryakov]

These script elements aren't garbage collected, there were over 125000 left alive after the test finished for me.

[Alexey Proskuryakov]

<rdar://problem/8124718>

[Alexey Proskuryakov]

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

[Alexey Proskuryakov]

Clearly, haveFiredLoadEvent() is not the same as "is loading".

[Alexey Proskuryakov]

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...

[Alexey Proskuryakov]

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.

[WebKit Review Bot]

Attachment 76598 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7096004

[Early Warning System Bot]

Attachment 76598 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7160006

[Eric Seidel (no email)]

Attachment 76598 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7158026

[Alexey Proskuryakov]

Comment on attachment 76598 [details]
proposed fix

SVG tests are failing, need to fix.

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Created attachment 76670 [details]
SVG no subresources test

[Alexander Romanovich]

Is anyone working on the SVG block for this bug? Maybe a separate bug filed for it that I can CC myself on?

[Eric Seidel (no email)]

(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.

[Geoffrey Garen]

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 ***

[Alexey Proskuryakov]

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.