RFR: 8246745: ListCell/Skin: misbehavior on switching skin

Kevin Rushforth kcr at openjdk.java.net
Thu Sep 10 17:37:25 UTC 2020


On Wed, 10 Jun 2020 12:25:04 GMT, Jeanette Winzenburg <fastegal at openjdk.org> 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


More information about the openjfx-dev mailing list