On Wed, 6 Aug 2025 17:43:36 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> tests/system/src/test/java/test/javafx/stage/RestoreStagePositionTest.java 
>> line 130:
>> 
>>> 128: 
>>> 129:         Util.runAndWait(() -> stage.setMaximized(true));
>>> 130:         Util.waitForIdle(stage.getScene());
>> 
>> I know that @andy-goryachev-oracle suggested its use, but for tests that do 
>> various windowing operations (maximize, full screen, restore, etc), 
>> `waitForIdle` is too short (its on the order of 160 msec). To make the test 
>> more robust, I recommend reverting this back to the `sleep(800)` you had 
>> earlier.
>> 
>> Arguably, we should have a utility method for this sort of "wait for 
>> windowing changes to settle down", but we don't.
>
> There is `Util::waitForIdle(scene, pulseCount)`.
> 
> The real question is how to account for the platform-dependent transitions 
> (which may take some time) reliably.  Waiting for a set amount of time might 
> work, but we have to pick a good timeout value.  Is there any other 
> possibility?
> 
> I am ok with 800ms - it feels like it should be long enough for any 
> transition to take place.

Changes to the maximized state are synchronous on all platforms. By the time 
`runAndWait` returns the window should be maximized. On Mac and Windows the 
JavaFX window state properties should be up-to-date because the OS 
notifications are also synchronous. I added waitForIdle because on Linux the 
configure events might come in later but we only need to wait long enough for 
the event loop to receive the events. That should be far less than 800ms.

> The real question is how to account for the platform-dependent transitions 
> (which may take some time) reliably.

I can't think of any window attribute change that performs an asynchronous 
transition (but maybe I'm forgetting one). runAndWait should be enough on Mac 
and Windows. On Linux you need to give the event loop a chance to spin a few 
times.

The only real issue is (was?) fullscreen on macOS. The wait isn't really for 
the transition (it's done by the time setFullScreen returns) but to avoid 
scheduling deferred Runnables that might execute during the transition. That's 
less of an issue since #1797 was integrated.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1860#discussion_r2258030257

Reply via email to