On Wed, 10 Jun 2020 12:25:04 GMT, Jeanette Winzenburg <[email protected]> wrote:
> ListCellSkin installs listeners to the ListView/fixedCellSize that introduce > a memory leak, NPE on replacing the > listView and incorrect update of internal state (see bug report for details) > Fixed by removing the listeners (and the internal state had been copied from > listView on change) and access of listView > state when needed. > Added tests that failed before and pass after the fix, plus a sanity test to > guarantee same (correct) behavior > before/after. (catching up on some overdue reviews) The fix and tests look fine to me. I left one minor suggestion to add a comment in one of the tests. modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java line 68: > 66: installDefaultSkin(cell); > 67: cell.updateListView(null); > 68: listView.setFixedCellSize(100); I presume this test is just checking that no exception is thrown? If so, can you add a comment to that effect? ------------- Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/251
