[Development] Calling QWindow::requestActivate after QWindow::show and before QTest::qWaitForWindowActive

[Axel Spoerl]

+1 to removing it, when the removal fixes flakiness.

Confidential
________________________________
From: EXT Mitch Curtis <mitch.curtis at qt.io>
Sent: Wednesday, 13 August 2025 07:50
To: Axel Spoerl <axel.spoerl at qt.io>; development at qt-project.org <development at qt-project.org>
Subject: Re: [Development] Calling QWindow::requestActivate after QWindow::show and before QTest::qWaitForWindowActive


Thanks for the explanation, Axel.



Does anyone disagree with removing these calls for all platforms in specific tests that have been shown to be flaky only on XCB?




Confidential

From: Development <development-bounces at qt-project.org> on behalf of Axel Spoerl via Development <development at qt-project.org>
Date: Tuesday, 12 August 2025 at 15:11
To: development at qt-project.org <development at qt-project.org>
Subject: Re: [Development] Calling QWindow::requestActivate after QWindow::show and before QTest::qWaitForWindowActive

Good morning and thanks to Mitch for initiating this discussion!



QWindow::requestActivate() returns early with a warning, if the window in question doesn't accept focus.

In all other cases, it dispatches to virtual QPlatformWindow::requestActivateWindow(), which can be overridden in each platform implementation.

  *   The default implementation dispatches to QWindowSystemInterface::handleFocusWindowChanged(). It causes Qt to treat the given window as the focus window, without acting on platform level.

  *   XCB: Sends an NET_ACTIVE_WINDOW if the window is toplevel, otherwise XCB_INPUT_FOCUS_PARENT, to its parent. QXcb implementation deferres the activation if the window is unmapped. That's where data races might occur. They are typically harmless in an application, where the mouse is busy moving and a "forgotten event" is hardly noticed.

  *   Windows: The win APIs BringWindowToTop() and SetActiveWindow() are called on the native handle after some sanity checks.

  *   MacOS: The native window is made key window and first responder.

  *   Wayland: The wayland XDG shell sends an activation request to the shell, the normal wayland shell does a no-op.

  *   To my knowledge, the eglfs amd off screen implementations also implement platform specific activation.



=> Whenever a window is already active, requestActivate() neither hurts nor does it anything useful. It may trigger a race on XCB just after show(), which causes the activation to be dropped. I have only seen that in autotests. It would be interesting to examine the bug that Ilya has seen. If the described scenario causes a show() and requestActivate() just after each other, we indeed might have an issue.



Our existing autotests implement different use cases with and without QWindow::requestActivate(). A few examples:

  *   tst_QWindow::positioning() expects activation after QWindow::showNormal(), without calling requestActivate(). That's what Mitch and I are referring to. The call isn't needed if the one and only window around is shown.

  *   tst_QWindow::activateAndClose() calls showNormal() followed by requestActivate(). It has failed on 35 integrations this year on Ubuntu. I can reproduce that. The test always passes on XCB, when requestActivate() is removed. That's the race condition Mitch and I were discussing.



That said, IMHO

  *   The usage of requestActivate() following a show() in an autotest should be explained / commented. Reviewers should ask for such explanation.

  *   The calls can't be removed en masse and light hearted from autotests. Removals have to be evaluated case by case.

  *   Let's remove if it fixes flakiness and doesn't hurt the test otherwise.



Cheers

Axel



Confidential

________________________________

From: Development <development-bounces at qt-project.org> on behalf of EXT Mitch Curtis via Development <development at qt-project.org>
Sent: Tuesday, 12 August 2025 07:29
To: David C. Partridge <david.partridge at perdrix.co.uk>; development at qt-project.org <development at qt-project.org>
Subject: Re: [Development] Calling QWindow::requestActivate after QWindow::show and before QTest::qWaitForWindowActive



First we need to agree on whether or not this is the right thing to do. If we agree that it is, then we should absolutely do that. That’s what I was advocating for in my comments on the change, too.





Confidential

From: David C. Partridge <david.partridge at perdrix.co.uk>
Date: Tuesday, 12 August 2025 at 12:15
To: EXT Mitch Curtis <mitch.curtis at qt.io>
Subject: RE: [Development] Calling QWindow::requestActivate after QWindow::show and before QTest::qWaitForWindowActive

Shouldn't some words about this be added to the user documentation to tell
us not to do that...

D.

-----Original Message-----
From: Development <development-bounces at qt-project.org> On Behalf Of EXT
Mitch Curtis via Development
Sent: 12 August 2025 01:53
To: Qt Development <development at qt-project.org>
Subject: [Development] Calling QWindow::requestActivate after QWindow::show
and before QTest::qWaitForWindowActive

Hi,

There's a discussion on
https://codereview.qt-project.org/c/qt/qtdeclarative/+/666818 about whether
we can standardise (by documenting in some form) that we shouldn't call
requestActivate before qWaitForWindowActive. Essentially, existing code in
tests looks like this:

    window->show();
    window->requestActivate();
    QVERIFY(QTest::qWaitForWindowActive(window.data()));

And would become this:

    window->show();
    QVERIFY(QTest::qWaitForWindowActive(window.data()));

As Axel has explained, this causes race conditions on XCB, and apparently is
redundant on all platforms:

> To begin with, an autotest should be as close as possible to a real world
scenario.
> When the only window around is being shown, it is automatically activated.
The additional window->requestActivate() redundant and not a real world
scenario. This alone justifies its removal.

I'd like to get agreement that this is the right thing to do on all
platforms before proceeding (and then removing it en masse).

Cheers.

Confidential
--
Development mailing list
Development at qt-project.org
https://lists.qt-project.org/listinfo/development
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20250813/81945da8/attachment.htm>