RFR: 8290844: Add Skin.install() method [v3]

[Kevin Rushforth]

On Fri, 12 Aug 2022 17:30:20 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line 245:
>> 
>>> 243:             if (skin != null) {
>>> 244:                 if (skin.getSkinnable() != Control.this) {
>>> 245:                     throw new IllegalArgumentException("There must be 1:1 relationship between Skin and Skinnable");
>> 
>> I wonder an error message like "Skin was not created for this Skinnable" or "Skin does not correspond to this Skinnable" would be clearer?
>> 
>> For the implementation, you should also unbind, if needed, before throwing the exception (we really should have made this a read-only property so it couldn't be bound, since a Skin is a "use once" object).
>> 
>> 
>>                 if (isBound()) {
>>                     unbind();
>>                 }
>
> Good point!
> Actually, unbind() is sufficient, as it does the required check.  From javadoc: _If the Property is not bound, calling this method has no effect._
> Is there another reason to call isBound(), perhaps for clarity?

Good question. I do see the check in other places where an exception is thrown from invalidated, such as TextField, TextArea, TableColumnHeader. More importantly, those properties also restore their previous value (irrespective of binding), which we might want to consider here.

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

PR: https://git.openjdk.org/jfx/pull/845