On Fri, 27 May 2022 12:13:43 GMT, Jay Bhaskar <jbhas...@openjdk.org> wrote:
>> This PR is new implementation of JavaEvent listener memory management. >> Issue >> [JDK-8088420](https://bugs.openjdk.java.net/browse/JDK-8088420?filter=-1) >> >> 1. Calling remove event listener does not free jni global references. >> 2. When WebView goes out of scope (disposed from app) , its Event Listeners >> are not being garbage collected. >> >> Solution: >> 1. Detached the jni global reference from JavaEventListener. >> 2. Create scoped ref counted wrapper class JavaObjectWrapperHandler for jni >> global reference. >> 3. Create unique JavaObjectWrapperHandler object for each JavaEventListener. >> 4. EventListenerManager is a singleton class , which stores the >> JavaObjectWrapperHandler mapped with JavaEventListener. >> 5. EventListenerManager also stores the JavaEventListener mapped with >> DOMWindow. >> 6. When Event listener explicitly removed , JavaEventListener is being >> forwarded to EventListenerManager to clear the listener. >> 7. When WebView goes out of scope, EventListenerManager will de-registered >> all the event listeners based on the ref counts attached with WebView >> DOMWindow. > > Jay Bhaskar has updated the pull request incrementally with one additional > commit since the last revision: > > Change for unregisterDomWindow function and code cleanup The changes look good. While doing my final review I noticed one more thing that I think should be changed. modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp line 57: > 55: } > 56: > 57: if (it->second && it->second->use_count() > 1) Shouldn't this be `else if`? The previous block calls `erase(it)`, so it isn't valid to dereference `it` after that call. ------------- PR: https://git.openjdk.java.net/jfx/pull/799
