[Rev 01] RFR: 8244110: NPE in MenuButtonSkinBase change listener

[Kevin Rushforth]

On Tue, 5 May 2020 11:07:43 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> Issue :
>> https://bugs.openjdk.java.net/browse/JDK-8244110
>> 
>> Root Cause :
>> Fix of [JDK-8175358](https://bugs.openjdk.java.net/browse/JDK-8175358) added code to remove accelerators from a scene
>> in Scene property listener of MenuButtonSkinBase. That fix uses getSkinnable() in listener. It turned out that it can
>> be null as found out by failing system test.  Fix :
>> As pointed out by @kleopatra on the JBS, getSkinnable() should never be null for a valid skin. Hence, the correct fix
>> is to remove listener cleanly in dispose method. See JBS comments for more info. This also addresses
>> [JDK-8244081](https://bugs.openjdk.java.net/browse/JDK-8244081).  Testing :
>> 1) Failing system test - passes after the fix
>> 2) 3 unit tests which used to log NPE message silently and pass have been made to fail without this fix - these tests
>> passes after the fix
>> Special Thanks to @kleopatra for most of the test digging and fix guidance.
>
> Ajit Ghaisas has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Make ChangeListener Final

Looks good. I confirm that the original system test and the new controls unit test both fail without the fix and now
pass with the fix.

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

Marked as reviewed by kcr (Lead).

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