Merge pull request #1233 from WebPlatformForEmbedded/pgorszkowski/2.38/fix_for_mem_leak_with_dynamic_lazy_loading_img

magomez · web-flow · commit 92f2cef5fd5f · 2023-11-16T11:30:03.000+01:00
Memory consumption/leak with img out of viewport and lazy loading
diff --git a/LayoutTests/fast/dom/dynamic-image-with-lazy-loading-leak-expected.txt b/LayoutTests/fast/dom/dynamic-image-with-lazy-loading-leak-expected.txt
@@ -0,0 +1,20 @@
+Tests that lazy image loading doesn't cause leaks when 'img' element is created and removed dynamically.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS internals.isElementAlive(imgElementId) is true
+PASS The img element didn't leak.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/dom/dynamic-image-with-lazy-loading-leak.html b/LayoutTests/fast/dom/dynamic-image-with-lazy-loading-leak.html
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test.js"></script>
+<div style="height: 2000px"></div>
+<div id="below-viewport-img-container" style="height: 1000px"></div>    
+<script>
+description("Tests that lazy image loading doesn't cause leaks when 'img' element is created and removed dynamically.");
+jsTestIsAsync = true;
+
+checkCount = 0;
+
+const imgCount = 10;
+imgs = [];
+const image_path = 'image.png';
+imgElementIds = [];
+
+function runTest()
+{
+    handle = setInterval(() => {
+        gc();
+        for (let imgElementId of imgElementIds) {
+            if (!internals.isElementAlive(imgElementId)) {
+                clearInterval(handle);
+                testPassed("The img element didn't leak.");
+                finishJSTest();
+                return;
+            }
+        }
+        checkCount++;
+        if (checkCount > 1000) {
+            clearInterval(handle);
+            testFailed("The img elements leaked.");
+            finishJSTest();
+        }
+    }, 10);
+}
+
+onload = () => {
+    for (let i = 0; i < imgCount; i++) {
+        img = document.getElementById("lazy-loaded-img-"+i);
+        imgElementId = internals.elementIdentifier(img);
+        shouldBeTrue("internals.isElementAlive(imgElementId)");
+        imgElementIds.push(imgElementId);
+        document.getElementById('below-viewport-img-container').removeChild(img);
+        img = null;
+    }
+    imgs = [];
+    setTimeout(runTest, 0);
+};
+
+for (let i = 0; i < imgCount; i++) {
+    img = new Image();
+    img.id = "lazy-loaded-img-"+i;
+    img.loading = 'lazy';
+    img.src = image_path;
+    document.getElementById('below-viewport-img-container').append(img);
+    imgs.push(img);
+    img = null;
+}
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/loader/ImageLoader.cpp b/Source/WebCore/loader/ImageLoader.cpp
@@ -519,6 +519,14 @@ void ImageLoader::timerFired()
     m_protectedElement = nullptr;
 }
 
+bool ImageLoader::hasPendingActivity() const
+{
+    // Because of lazy image loading, an image's load may be deferred indefinitely. To avoid leaking the element, we only
+    // protect it once the load has actually started.
+    bool imageWillBeLoadedLater = m_image && !m_image->isLoading() && m_image->stillNeedsLoad();
+    return (m_hasPendingLoadEvent && !imageWillBeLoadedLater) || m_hasPendingErrorEvent;
+}
+
 void ImageLoader::dispatchPendingEvent(ImageEventSender* eventSender)
 {
     ASSERT(eventSender == &beforeLoadEventSender() || eventSender == &loadEventSender() || eventSender == &errorEventSender());
diff --git a/Source/WebCore/loader/ImageLoader.h b/Source/WebCore/loader/ImageLoader.h
@@ -72,7 +72,7 @@ class ImageLoader : public CachedImageClient {
 
     // FIXME: Delete this code. beforeload event no longer exists.
     bool hasPendingBeforeLoadEvent() const { return m_hasPendingBeforeLoadEvent; }
-    bool hasPendingActivity() const { return m_hasPendingLoadEvent || m_hasPendingErrorEvent; }
+    bool hasPendingActivity() const;
 
     void dispatchPendingEvent(ImageEventSender*);
 

Original file line number	Diff line number	Diff line change
`@@ -519,6 +519,14 @@ void ImageLoader::timerFired()`
`519`	`519`	`m_protectedElement = nullptr;`
`520`	`520`	`}`
`521`	`521`
	`522`	`+bool ImageLoader::hasPendingActivity() const`
	`523`	`+{`
	`524`	`+ // Because of lazy image loading, an image's load may be deferred indefinitely. To avoid leaking the element, we only`
	`525`	`+ // protect it once the load has actually started.`
	`526`	`+ bool imageWillBeLoadedLater = m_image && !m_image->isLoading() && m_image->stillNeedsLoad();`
	`527`	`+ return (m_hasPendingLoadEvent && !imageWillBeLoadedLater) \|\| m_hasPendingErrorEvent;`
	`528`	`+}`
	`529`	`+`
`522`	`530`	`void ImageLoader::dispatchPendingEvent(ImageEventSender* eventSender)`
`523`	`531`	`{`
`524`	`532`	`ASSERT(eventSender == &beforeLoadEventSender() \|\| eventSender == &loadEventSender() \|\| eventSender == &errorEventSender());`