Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[Windows] Lifecycle: Add AppInstance activated event #34883
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
[Windows] Lifecycle: Add AppInstance activated event #34883
Changes from 5 commits
14d323369050ebec30eb1e4b684fb27df668ab84a6c1554db60a221f144023aFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Async and Threading Safety — Same blocking pattern as the library code:
keyInstance.RedirectActivationToAsync(args).AsTask().GetAwaiter().GetResult()runs insideHandleWindowsAppInstanceActivated, invoked as aWindowsLifecycle.OnAppInstanceActivatedhandler on the UI thread (either directly fromOnLaunchedor dispatched fromMauiWinUIApplication.HandleAppInstanceActivated). This is presented to app authors as recommended single-instancing boilerplate, but it teaches the same deadlock/hang-prone anti-pattern flagged in the product code instead ofawaiting the redirect.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description and metadata refer to a new
OnAppActivation(...)hook, but the implementation introducesOnAppInstanceActivated(...)instead. Please update the PR description (or rename the API) so the documented public surface matches the code that will ship.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Public API baseline — This PR adds Core Windows public API surface (WindowsLifecycle.OnAppInstanceActivated, its generated Invoke, the WindowsLifecycleBuilderExtensions.OnAppInstanceActivated(...) extension method, and MauiWinUIApplication.OnAppInstanceActivated(...)), but src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt was not updated. The sibling Windows lifecycle delegates/extensions are tracked in PublicAPI.Shipped.txt, so the Core public API analyzer will reject these additions until the corresponding Unshipped entries are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[moderate] Regression Prevention and Test Coverage — When
OnLaunchedfires again for an already-running instance (_application != null && _services != null), this callsOnAppInstanceActivated(activatedEventArgs)directly usingAppInstance.GetCurrent().GetActivatedEventArgs(), whileAppInstance.GetCurrent().Activated(line 91, wired up on the prior launch) remains subscribed viaHandleAppInstanceActivated. If WinAppSDK delivers the same reactivation through both paths,WindowsLifecycle.OnAppInstanceActivatedhandlers (includingWebAuthenticator.OnAppInstanceActivatedCallback) can run twice for one logical activation with no de-duplication guard.LifecycleEventOrderTests.Windows.cs's newOnAppInstanceActivatedFiresExactlyOncetest only covers the initial single-launch case, not this already-running-instance reactivation path, so a double-invocation regression here would go undetected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Async and Threading Safety —
AppInstance.Activatedcan be delivered on a non-UI thread for redirected/file/protocol activations, but this callsOnAppInstanceActivated(args)synchronously and therefore runs public MAUI lifecycle handlers off the WinUI UI thread. Handlers are likely to touchApplication.Current, windows, or controls (the sample added in this PR does), which can throw cross-thread exceptions or race with startup. Dispatch this callback through the application/windowDispatcherQueue(with a null/disposed guard) before invoking lifecycle delegates.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Windows lifecycle threading — AppInstance.GetCurrent().Activated is raised by the Windows App SDK on a non-UI thread, but this handler calls OnAppInstanceActivated(args) directly. That runs every public WindowsLifecycle.OnAppInstanceActivated callback off the WinUI dispatcher, so app code that touches Window, DispatcherQueue, or other UI-affine state from this lifecycle hook can fail intermittently on redirected/protocol activations. Capture/use the WinUI DispatcherQueue (or the MAUI application dispatcher once available) before invoking lifecycle handlers, or make the off-thread contract explicit and keep MAUI-owned handlers from touching UI state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configureBuilderis declared as a non-nullableAction<MauiAppBuilder>but defaults tonull. With nullable annotations enabled in the repo this will produce a nullability warning (and can become a build break when warnings are treated as errors). Make the parameter nullable (e.g.,Action<MauiAppBuilder>? configureBuilder = null).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[major] Device test reliability - These Android tests read
LifecycleEventLogimmediately, butOnStart/OnResumecan be delivered after the test runner starts. That makes the assertions race startup lifecycle delivery and can fail with missingOnStart/OnResume. Please wait for Android startup to reachOnResume(or expose a startup-complete signal) before asserting the log/order.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[moderate] Device test correctness — The order assertions compare raw IndexOf results without first asserting that each event was found. A missing early event produces -1, which can still satisfy the < comparisons (for example, missing OnAppInstanceActivated makes -1 < launchingIndex pass), so this test can report the startup ordering as correct while the ordered event is absent. Add explicit >= 0 assertions (as the iOS test does for FinishedLaunching) before comparing indices; the same pattern exists in LifecycleEventOrderTests.Android.cs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.