[Webkit-unassigned] [Bug 262219] Previously focused form input elements are not getting garbage collected

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=262219

--- Comment #13 from Cristian Linte <ioancris at adobe.com> ---
The leak is from Safari own JS context (world) where it loads FormMetadataJS and keeps references to HTML elements from the page. JS wrappers referenced from that context will cause all other JS wrappers for same element, e.g. from page JS context, to be kept alive and that is why we detect that leak via JS WeakRef.deref in the test files.
There doesn't seem to be anything that a web page can do to workaround this leaks. At best it can try to minimize the leaks which can be infeasible.
As such I hope that this gets a high priority since this is a major leak for large apps that keep removing and adding new input elements or form elements and the user focuses one of them.

The references from other JS context are visible in Web Inspector memory(JS heap) snapshot. I've attached screenshot to showcase the leak seen in the memory snapshot. The snapshot was taken after focus on input element from first Form Sample and and after clicking Unparent Children button for both form samples. The form sample that had the input with focus has all form elements leaked. The screenshots only showcase the input elements.


FormMetadataJS on the global object in that Safari JS context has _controlUniqueIDToControlMap that is never cleared and will keep form control elements alive. This is a big issue with web sites that keep removing and add new elements and also because the JS wrapper itself might end up leaking other JS objects or JS wrappers.
Another JS context is one for ReaderArticleFinderJS which also keeps references to HTML elements and this one not only form and form controls but also divs and other elements.

There is another leak on the native side where Safari::BrowserBundlePageFormClient keeps a reference to Webkit API wrapper for the html input element that was last focused. Once a new input is focused and unfocused it will clear the reference to the old one. This only leaks the native DOM element and those have only weak references from what I understood how Webkit works so this leak is not that bad.
I'll add more details in the ticket.



Details:
1. Native leaks:
Safari will leak the last focused input element. It keeps a reference to the Webkit API wrapper which in turn keeps the native DOM node. This is less of an issue since it would leak only that DOM node, my understanding is that Webkit DOM nodes have weak references to other DOM nodes and strong references are usually kept only via the JS wrappers.
Steps to showcase the leak
- focus input element (invokes InjectedBundlePageFormClient::textFieldDidBeginEditing). Safari`Safari::BrowserBundlePageFormClient::textFieldDidBeginEditing exits after keeping 1 reference (via Safari::WK::Type stored on this + 0x38). RefCount:  1
- unfocus input element or browser window (invokes InjectedBundlePageFormClient::textFieldDidEndEditing). Safari::BrowserBundlePageFormClient::textFieldDidEndEditing exits after keeping 1 reference (via TextFieldDidEndEditingState stored on this + 0x50). RefCount:  2
- async invocation of block defined in BrowserBundlePageFormClient::textFieldDidEndEditing executes which calls resetStateAfterTextFieldDidEndEditing which clears reference from textFieldDidBeginEditing. RefCount: 1
- focus another input element. RefCount: 1 for old focused element and new focused element
- unfocus input element. TextFieldDidEndEditingState is overwritten removing the reference to the old element. RefCount: 0 for older focused element and 2 for last focused element.
- async invocation of block defined in textFieldDidEndEditing. RefCount: 1 for last focused element
I haven't included temporary refs that the code takes as it executes. The ref count is taken after end of execution for Webkit API callbacks.

2. JS leak
- focus input element (invokes InjectedBundlePageFormClient::textFieldDidBeginEditing). Safari`Safari::BrowserBundlePageFormClient::textFieldDidBeginEditing creates FrameMetadata which calls WKBundleScriptWorldCreateWorld and loads JS script for FormMetadataJS (embedded in SafariShared binary) and also AutomaticPasswordJS. It then calls "globalThis.FormMetadataJS.textFieldOrSelectElementMetadata(textField, 0, false)" where textField is the focused input element. That JS execution will end up storing in FormMetadataJS references to form and form controls, input/texarea/select/button from the page(or frame).
- unfocus input element doesn't call FormMetadataJS and references from that JS context (world) is leaked. The reference is kept from `_forms` and `_controlUniqueIDToControlMap` arrays. The `_forms` are is updated sometimes and references might get removed so is less of an issue while `_controlUniqueIDToControlMap` is never cleared and it is a permanent leak. 
- as new input elements are created and then focused the leak increases.

The JS wrappers for the DOM nodes from the FormMetadataJS context will keep alive the JS wrappers for the same DOM nodes from the main page context. This wrappers in return can keep alive other wrappers or normal JS objects.
A JS wrapper for DOM node uses the top DOM node ancestor as an opaque root and all JS wrappers for nodes that use the same root node are kept alive. This means that form and form elements referenced by FormMetadataJS will also keep alive their ancestors and their subtrees.
The Webkit documentation has details about JS wrappers and how they are kept alive via opaque roots to implement correct JS lifetime semantics for the web.
https://github.com/WebKit/WebKit/blob/main/Introduction.md#understanding-document-object-model

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20231120/cecefb14/attachment-0001.htm>