Bug 262238

Summary: img elements with loading=“lazy” that are not visible are not getting garbage collected
Product: WebKit Reporter: Kin Blas <jblas>
Component: ImagesAssignee: Przemyslaw Gorszkowski <pgorszkowski>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: jblas, pgorszkowski, sabouhallawa, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac (Apple Silicon)   
OS: macOS 13   
See Also: https://bugs.webkit.org/show_bug.cgi?id=263521
Attachments:
Description Flags
All-in-One test case that illustrates the img loading="lazy" retainer issue
none
Video demonstrating the img loading="lazy" retainer issue none

Description Kin Blas 2023-09-27 20:07:31 PDT 
In our web application we’ve noticed a significant number of detached DOM subtrees not getting garbage collected when running in Safari Desktop and on iOS. We’ve managed to narrow the issue down to img elements with loading=“lazy” set on them, that have not been loaded yet.


The retained (leaking) subtrees in question correspond to panel UI within our application, so you can imagine how the process size of the web application tab grows steadily as the user triggers the showing and hiding of various panels. Our panels are quite large and complex so things sometimes get the point where Safari iOS will reload our web application.


Attached to this bug is an HTML file containing a minimal test case that illustrates the issue. Note that there must be at least one image with loading=“lazy” that has never entered the viewport to trigger the retainer issue.


There is also a video attached to this bug that demonstrates what we see.


I can reproduce this issue in Safari 16.5 as well as WebKit 268505@main.


Instructions:

Test #1:

* Load the test case in a Safari or Webkit build.
* Click all of the Remove buttons on the page.
* Trigger a garbage collection via the terminal with ‘notifyutil -p org.WebKit.lowMemory’
* Observe that all of the test cases turn green except for the sample that uses images with loading=“lazy”
* Click on the “Unparent Children” button for the loading=“lazy” test case
* Trigger a garbage collection
* Observe that all elements in the subtree turn red, indicating they have been garbage-collected, except for the img elements which remain in memory.


Test #2

* Load the test case in a Safari or Webkit build.
* Scroll down to the lazy=“loading” test case and scroll through ALL of the images.
* Click all of the Remove buttons on the page.
* Trigger a garbage collection via the terminal with ‘notifyutil -p org.WebKit.lowMemory’
* Observe that all of the test cases turn green


The expectation is that for both tests, all test cases turn green regardless of whether or not the loading=“lazy” img elements have ever entered the viewport.
Comment 1 Kin Blas 2023-09-27 20:09:51 PDT 
Created attachment 467913 [details]
All-in-One test case that illustrates the img loading="lazy" retainer issue

Added an All-in-One test case that illustrates the img loading="lazy" retainer issue
Comment 2 Kin Blas 2023-09-27 20:11:37 PDT 
Created attachment 467914 [details]
Video demonstrating the img loading="lazy" retainer issue

Attaching a sample video demonstrating the img loading="lazy" retainer issue
Comment 3 Radar WebKit Bug Importer 2023-09-27 20:28:44 PDT 
<rdar://problem/116157415>
Comment 4 Kin Blas 2023-09-28 09:58:37 PDT 
One more observation I probably should've mentioned. This retainer issue doesn't happen if the img is a direct descendant of the scrolling element, there has to be an intermediate ancestor between the scrolling element and the img.
Comment 5 Przemyslaw Gorszkowski 2023-10-23 12:32:53 PDT 
It seems to be the same problem reported in https://bugs.webkit.org/show_bug.cgi?id=263521

Are you able to check the proposed solution in that report on Safari?


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);
 }

Thanks
Comment 6 Przemyslaw Gorszkowski 2023-10-25 13:02:25 PDT 

*** This bug has been marked as a duplicate of bug 263521 ***