When an image is preloaded in javascript, the size of the image is never reported to the javascript garbage collector. If a large image is repeatedly preloaded but never used, it will not free any of the images until the garbage collector has run out of space to allocate any new free javascript variables.
Created attachment 56651 [details] Suggested patch Adds a check to see if an image is loaded but is not part of the document.
Attachment 56651 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/manual-tests/image-prefetch-stress.html:6: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:7: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:8: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:9: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:10: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:14: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:15: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:16: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:17: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:18: Line contains tab character. [whitespace/tab] [5] WebCore/manual-tests/image-prefetch-stress.html:19: Line contains tab character. [whitespace/tab] [5] Total errors found: 11 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 56651 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/2301384
Created attachment 56654 [details] Updated 56651 Updates style changes and adds ifdefs around JSC specific code.
Comment on attachment 56654 [details] Updated 56651 Your manual test references a file URL that exists only on your machine. You should either update the test to use an image that exists in the SVN repository or add a suitable image alongside the test.
> JSC::JSGlobalData *globalData WebKit style puts the "*" next to the type name, since "pointer" is logically a party of the type. > +i = 0; > +setInterval(function(){ > + x = new Image(); Need 'var' in front of these two variable declarations for "i" and "x". With this patch, how many images load / how much memory is used before GC triggers?
Created attachment 56749 [details] more style changes. re ggaren: It should garbage collect after maxExtraCost additional memory has been reported to the garbage collector (what appears to be a megabyte). On my system, with only the manual test running in the browser, the usage doesn't seem to get above 130MB.
Comment on attachment 56749 [details] more style changes. I asked Nathan to take another look at this because the patch doesn't seem to fully solve the problem. In his test, about 130MB of image data is still reported as allocated, and the number of live JSHTMLImageElements still increases linearly.
Created attachment 57280 [details] Fixed test case. It appears that the number of live HTMLImageElements is still increasing because the image elements are being created faster than they can load creating a backlog of images to be loaded. The new test case doesn't load images as quickly, showing that the patch does in fact work.
Comment on attachment 57280 [details] Fixed test case. r=me
Comment on attachment 57280 [details] Fixed test case. Clearing flags on attachment: 57280 Committed r60337: <http://trac.webkit.org/changeset/60337>
All reviewed patches have been landed. Closing bug.
Is a similar fix needed for Audio and Video elements?
(In reply to comment #13) > Is a similar fix needed for Audio and Video elements? Do those elements preload while outside the document? If so, yes.
Dan pointed out on IRC that checking just for the <img> tag is probably sufficient, since that's what new Image uses.
(In reply to comment #14) > Do those elements preload while outside the document? If so, yes. Indeed they do. I filed bug 39858.
Because not just <img> and <image> elements can preload images, we don't want to restrict based on the element associated with the loader.
Created attachment 57287 [details] removed <img> and <image> tag checks.
Comment on attachment 57287 [details] removed <img> and <image> tag checks. r=me
Comment on attachment 57287 [details] removed <img> and <image> tag checks. Clearing flags on attachment: 57287 Committed r60345: <http://trac.webkit.org/changeset/60345>