RFR: 8360940: Layout stops updating when using (public api) Parent#setNeedsLayout(true) due to incorrect state management

[Kevin Rushforth]

On Sat, 23 Aug 2025 14:58:29 GMT, John Hendrikx <jhendrikx at openjdk.org> wrote:

> Ensures proper propagation of layout flags when using `forceParentLayout = true`.
> 
> This was the root cause of issue #1874
> 
> ### Note
> 
> Apparently it is still quite easy to mess up the layout flags. Basically, the layout flag tracked by Parent should always either be `CLEAN` for any scene graph branch, or `!CLEAN` + a layout pulse is scheduled on the corresponding `Scene`.
> 
> However, with careful use of the public API `requestLayout` one can get these flags in a bad state still:
> 
> Let's say I have a branch `A (root node under Scene) -> B -> C`, **and** a layout is in progress, **and** we're currently in the `layoutChildren` method of `C`. The flag `performingLayout` will be `true` for all nodes in the branch `A` ->  `B` -> `C`.  The `layout` method will set the layout flag to `CLEAN` as its first action, so when we're at `C::layoutChildren`, all flags have been reset to `CLEAN` already.  See the `Parent::layout` method for how all this works.
> 
> Now, to mess up the flags, all you need to do is call `requestLayout` on `B` or `C` from the `layoutChildren` of `C` (or indirectly by changing something and something is listening to this and schedules a layout on something somewhere in this branch); note that `requestLayout` is not documented to be illegal to call during layout, and some classes in FX will do so (ScrollPaneSkin, NumberAxis, etc..) risking the flags getting in a bad state... -- usually you get away with this, as there are many ways that layout is triggered, and eventually the flags will get overwritten and reset to a consistent state.
> 
> The bad state occurs because this code path is followed (all code from `Parent`):
> 
>     public void requestLayout() {
>         clearSizeCache();
>         markDirtyLayout(false, forceParentLayout);
>     }
>     
> Calls to `markDirtyLayout(false, false)`:
> 
>     private void markDirtyLayout(boolean local, boolean forceParentLayout) {
>         setLayoutFlag(LayoutFlags.NEEDS_LAYOUT);
>         if (local || layoutRoot) {
>             if (sceneRoot) {
>                 Toolkit.getToolkit().requestNextPulse();
>                 if (getSubScene() != null) {
>                     getSubScene().setDirtyLayout(this);
>                 }
>             } else {
>                 markDirtyLayoutBranch();
>             }
>         } else {
>             requestParentLayout(forceParentLayout);
>         }
>     }
>     
> Before going into the `else` (as none of the nodes is a layout root, and `local` was set to `false`) it will do **setLayoutFlag(LayoutFlags.NEEDS_LAYOUT)** ...

The fix looks correct. Can you provide an automated test?

I recommend removing `(public api)` from the title of the JBS bug and this PR -- we don't typically do that and it seems superfluous here.

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

PR Comment: https://git.openjdk.org/jfx/pull/1879#issuecomment-3258862425
PR Comment: https://git.openjdk.org/jfx/pull/1879#issuecomment-3258867305