RFR: 8354797: Parent.needsLayoutProperty() should return read-only getter

John Hendrikx jhendrikx at openjdk.org
Wed Apr 16 10:54:52 UTC 2025


On Wed, 16 Apr 2025 10:10:36 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> `Parent.needsLayout` is implemented with a `ReadOnlyBooleanWrapper`. The property getter returns the wrapper itself, but what it should be doing is return the read-only getter instead.
>> 
>> A single reviewer should be sufficient.
>
> modules/javafx.graphics/src/main/java/javafx/scene/Parent.java line 972:
> 
>> 970:             needsLayout = new ReadOnlyBooleanWrapper(this, "needsLayout", layoutFlag == LayoutFlags.NEEDS_LAYOUT);
>> 971:         }
>> 972:         return needsLayout.getReadOnlyProperty();
> 
> `ReadOnlyBooleanWrapper` is eventually inherited from `ReadOnlyBooleanProperty`, which is why it did not show any error earlier.
> 
> `needsLayout.getReadOnlyProperty()` returns a private member variable of `ReadOnlyBooleanWrapper` class, which seems to be dummy and fetches the actual value from Wrapper class that were passed when creating an instance of `ReadOnlyBooleanWrapper` i.e. `new ReadOnlyBooleanWrapper(this, "needsLayout", layoutFlag == LayoutFlags.NEEDS_LAYOUT);`
> 
> It seems returning `needsLayout` holds good too...

The reason you shouldn't return `needsLayout` directly is because you can circumvent this easily by casting it to `BooleanProperty` and then call setters to mutate it.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1780#discussion_r2046668509


More information about the openjfx-dev mailing list