On Tue, 4 Oct 2022 15:14:43 GMT, Jeanette Winzenburg <faste...@openjdk.org>
wrote:
>> Andy Goryachev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8294589: unnecessary null checks
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java
> line 328:
>
>> 326: mouseEventHandler = t -> {
>> 327: if (getSkinnable() != null) {
>> 328: Bounds containerScreenBounds =
>> container.localToScreen(container.getLayoutBounds());
>
> this null check is wrong: skinnable is guaranteed to never be null - except
> after dispose, at which time it's illegal to access any of the skin's
> methods/fields/state. So checking here is smearing over a precondition
> violation of the caller (most probably a dangling handler/listener) which has
> to be fixed.
>
> Note we deliberately removed all null checks in all (cleaned) skins :) If we
> see any exceptions, the procedure is to
>
> - write a test exposing the exception (it's failing), this test belongs into
> SkinCleanTest
> - find and fix the violator
> - see the test passing
>
> There might be other exceptions as well (like IOOB or similar) which require
> a similar procedure, see the available tests. Actually, I think it's a good
> idea to actively look out for possible macroscopic misbehavior if
> handlers/listeners are not removed properly. SkinCleanupTest has examples.
agreed.
-------------
PR: https://git.openjdk.org/jfx/pull/906
