On Tue, 12 Aug 2025 07:48:53 +0000 Axel Spoerl via Development <development@qt-project.org> wrote:
> > > Yes, any code wiling to raise the window in the application uses the > > mentioned method that does setWindowState, setVisible, raise and > > activateWindow in a row. It's convenient since you don't have to > > think which state the window is in to raise it. > > Thanks, @Ilya Fedin<mailto:fedin-ilja2...@ya.ru>. > One reason why it happens in autotests is: Events have to be > processed explicitly in Qt, while the XCB thread is doing its own > business. If it the race / activation drop occurs in an application, > that needs to be fixed. Do you have a bug report / reprocer to look > at? > > Confidential > ________________________________ > From: Ilya Fedin <fedin-ilja2...@ya.ru> > Sent: Tuesday, 12 August 2025 09:23 > To: Axel Spoerl via Development <development@qt-project.org> > Cc: Axel Spoerl <axel.spo...@qt.io> > Subject: Re: [Development] Calling QWindow::requestActivate after > QWindow::show and before QTest::qWaitForWindowActive > > On Tue, 12 Aug 2025 07:07:57 +0000 > Axel Spoerl via Development <development@qt-project.org> wrote: > > > 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. > > Yes, any code wiling to raise the window in the application uses the > mentioned method that does setWindowState, setVisible, raise and > activateWindow in a row. It's convenient since you don't have to think > which state the window is in to raise it. > > > > > 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-boun...@qt-project.org> on behalf of > > EXT Mitch Curtis via Development <development@qt-project.org> Sent: > > Tuesday, 12 August 2025 07:29 To: David C. Partridge > > <david.partri...@perdrix.co.uk>; development@qt-project.org > > <development@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.partri...@perdrix.co.uk> > > Date: Tuesday, 12 August 2025 at 12:15 > > To: EXT Mitch Curtis <mitch.cur...@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-boun...@qt-project.org> On Behalf Of > > EXT Mitch Curtis via Development > > Sent: 12 August 2025 01:53 > > To: Qt Development <development@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@qt-project.org > > https://lists.qt-project.org/listinfo/development > No, as I already said, I'm unable to make a reproducer (and thus report) due to the flaky nature of the bug. -- Development mailing list Development@qt-project.org https://lists.qt-project.org/listinfo/development
