RFR: 8290765: Remove parent disabled/treeVisible listeners [v2]
Michael Strauß
mstrauss at openjdk.org
Fri Aug 26 00:25:11 UTC 2022
On Thu, 25 Aug 2022 22:18:44 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:
>> `Node` adds InvalidationListeners to its parent's `disabled` and `treeVisible` properties and calls its own `updateDisabled()` and `updateTreeVisible(boolean)` methods when the property values change.
>>
>> These listeners are not required, since `Node` can easily call the `updateDisabled()` and `updateTreeVisible(boolean)` methods on its children, saving the memory cost of maintaining listeners and bindings.
>
> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains five commits:
>
> - Replace null check by explicit underConstruction flag
> - Merge branch 'master' into fixes/node-listener-cleanup
> - Merge
> - added tests
> - Remove Node.disabled and Node.treeVisible listeners
It's true that `Parent.getChildren()` must not return `null`, and that we shouldn't add null checks to protect against bugs.
However, in this specific instance, `Node.setTreeVisible` is called from the constructor of `Node`. At that point in time, the `Parent` constructor has not yet run, so its final fields are still unassigned (and therefore `null`).
The same problem exists for `SubScene.getRoot()` a few lines earlier, which is specified to never return `null`, but will indeed return `null` if called when the `SubScene` is under construction.
But there's a serious problem with the null checking approach: it's calling a protected method at an unexpected time, before the constructor of a derived class has run. Since that's not what derived classes would expect, I have instead opted for a different solution, which includes a `underConstruction` flag that is passed to `Node.setTreeVisible`. When the flag is set, the calls to `SubScene.getRoot()` and `Parent.getChildren()` are elided (they'd return `null` anyway, so there's no point in calling these methods).
-------------
PR: https://git.openjdk.org/jfx/pull/841
More information about the openjfx-dev
mailing list