Bug 263521 – Memory consumption/leak with img out of viewport and lazy loading

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Przemyslaw Gorszkowski]

Originally the problem was reported for WPE 2.38: https://github.com/WebPlatformForEmbedded/WPEWebKit/issues/1194

The same problem can be also reproduced on current WebKitGTK.

I create a simple test case on which the problem can be observed: https://people.igalia.com/pgorszkowski/img_lazy_loading.html?useLazy=1

The idea of the simple test case is:
1. create 1000 images with lazy loading(the same src) below the visible viewport
2. after 100ms remove existing images, and create another 1000 new images with lazy loading (the same src) below the visible viewport
3. do this recreation 1000 times

With lazy loading the memory grows quite fast from ~200MB to 2000MB after ~500 iterations

Without lazy loading (https://people.igalia.com/pgorszkowski/img_lazy_loading.html?useLazy=0) the memory stays on stable level ~200MB (never grows above 250MB) and after 1000 iterations it is still ~200MB

[Przemyslaw Gorszkowski]

I found that this fix solves the problem:

diff --git a/Source/WebCore/html/HTMLImageElement.cpp b/Source/WebCore/html/HTMLImageElement.cpp
index 3e4855fd20e2..5ddc923b1568 100644
--- a/Source/WebCore/html/HTMLImageElement.cpp
+++ b/Source/WebCore/html/HTMLImageElement.cpp
@@ -514,6 +518,8 @@ void HTMLImageElement::removedFromAncestor(RemovalType removalType, ContainerNod
         selectImageSource(RelevantMutation::Yes);
     }
 
+    m_imageLoader->clearImage();
+
     HTMLElement::removedFromAncestor(removalType, oldParentOfRemovedTree);
     FormAssociatedElement::elementRemovedFromAncestor(*this, removalType);
 }

but I am not sure about possible regressions or other impact which it can have (so I would be glad for any comments or advices).

I will elaborate more about my analysis in the next comment, which will be probably quite long because it is rather a complex process.

[Alexey Proskuryakov]

Is this the same as bug 262238?

[Przemyslaw Gorszkowski]

Yes, it is. 

I have checked also my proposal fix and it solves the problem from https://bugs.webkit.org/show_bug.cgi?id=262238

[Przemyslaw Gorszkowski]

But I checked it on WebKitGTK. It would be better to confirm it also on Safari.

[Przemyslaw Gorszkowski]

So the main problem is when the img element contains loading="lazy" and the element is outside the viewport which causes deferring the loading of the image until it is in visible area. The deferring load also increases the img element reference by m_protectedElement member in ImageLoader (which is kept by HTMLImageElement).
m_protectedElement is reset (and additional reference of the image element is decreased) when image is finally loaded.

The problem with dangling HTMLImageElement is when we remove it from the document before the image resource is finally loaded. When we remove the HTMLImageElement from document but image loading is deferred, the imageLoader still keeps additional reference for HTMLImageElement in m_protectedElement which causes the GC will never remove it (loading will never be done because HTMLImageElement is removed from document and will never be considered as in visible area).

To solve the problem we can add m_imageLoader->clearImage(); in HTMLImageElement::removedFromAncestor if the loading image was deferred.

[Przemyslaw Gorszkowski]

Pull request: https://github.com/WebKit/WebKit/pull/19559

[Przemyslaw Gorszkowski]

*** Bug 262238 has been marked as a duplicate of this bug. ***

[Radar WebKit Bug Importer]

<rdar://problem/117683012>

[EWS]

Committed 270745@main (78e4577732ca): <https://commits.webkit.org/270745@main>

Reviewed commits have been landed. Closing PR #19559 and removing active labels.