RFR: 8088420: JavaFX WebView memory leak via EventListener [v2]

[Jay Bhaskar]

On Thu, 19 May 2022 21:34:54 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Jay Bhaskar has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Platform.exit() , removing code block, as it is causing other test fail
>
> modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp line 57:
> 
>> 55:          if (it->second && it->second->use_count() == 1) {
>> 56:              delete it->second;
>> 57:              it->second = nullptr;
> 
> Do you need to remove the item from the list in this case?

applied new change , for removal of unused pairs

> modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp line 91:
> 
>> 89:     for (win_it = windowHasEvent.begin(); win_it != windowHasEvent.end(); win_it++) {
>> 90:         // de register associated event listeners with window
>> 91:         if(window == win_it->second) {
> 
> Minor: add space after the `if`.

done

> modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.cpp line 92:
> 
>> 90:         // de register associated event listeners with window
>> 91:         if(window == win_it->second) {
>> 92:             unregisterListener(win_it->first);
> 
> Same question as earlier: do you need to also remove the entries matching the `window` from the list here?

applied

> modules/javafx.web/src/main/native/Source/WebCore/bindings/java/EventListenerManager.h line 48:
> 
>> 46: 
>> 47: class JavaObjectWrapperHandler {
>> 48:     jobject handler_;
> 
> Have you considered using `JGObject` here instead of raw `jobject`?

No

-------------

PR: https://git.openjdk.java.net/jfx/pull/799