On Thu, 19 May 2022 21:34:54 GMT, Kevin Rushforth <k...@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
