Fix init order of SetupWithClassicDesktopLifetime - #21954
Conversation
|
You can test this PR using the following package version. |
|
|
|
Having thought about it more, the fact that This makes the whole thing sane, without hacks. The following numbers helped this decision:
Users of |
|
You can test this PR using the following package version. |
* Fix init order of SetupWithClassicDesktopLifetime * Don't raise Startup in Setup * Fix warning
* Fix init order of SetupWithClassicDesktopLifetime * Don't raise Startup in Setup * Fix warning
* Fix init order of SetupWithClassicDesktopLifetime * Don't raise Startup in Setup * Fix warning
What does the pull request do?
This PR fixes an initialization order bug when
AppBuilder.SetupWithClassicDesktopLifetime()is called instead ofStartWithClassicDesktopLifetime().What is the current behavior?
When
SetupWithClassicDesktopLifetimeis used, internal events are registered before theAppBuilderinitializers run: at that point, many subsystems aren't available yet, causing issues such asShutdownRequestednot being hooked up. Additionally,Startupis called way earlier, or might be called twice ifStart()is called manually on the created lifetime.A related problem arises when calling
AppBuilder.SetupWithLifetime()manually withoutStart: the internal event handlers aren't even initialized.On macOS, this causes the
ShutdownRequestedevent not to be called when quitting the app from the dock, then triggering a double close of the main window (and hitting a debug assertion).What is the updated/expected behavior with this PR?
StartWithClassicDesktopLifetime,SetupWithClassicDesktopLifetimeandSetupWithLifetimeinitialize the subsystems and event handlers in the same exact order.Quitting a macOS application raises the
ShutdownRequestedevent correctly and respects its cancellation.Unit tests have been added.
How was the solution implemented (if it's not obvious)?
An internal
ISetupApplicationLifetimeis added and implemented byClassicDesktopStyleApplicationLifetime. TheAppBuilderis responsible for calling it to initialize the lifetime in the correct order properly.Fixed issues
SetupWithClassicDesktopLifetime()is used #20683