diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue35826.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue35826.cs new file mode 100644 index 000000000000..3755b6554bcd --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue35826.cs @@ -0,0 +1,140 @@ +using Microsoft.Maui.Media; + +namespace Maui.Controls.Sample.Issues; + +[Issue(IssueTracker.Github, 35826, "PickPhotosAsync hangs when called from a child activity", PlatformAffected.Android)] +public class Issue35826 : ContentPage +{ + public Issue35826() + { + var instructions = new Label + { + AutomationId = "InstructionsLabel", + Text = "1. Tap 'Open Child Activity'\n" + + "2. In the child activity, tap 'Pick Photos'\n" + + "3. Press Back to cancel the picker\n" + + "Expected: Status shows 'Cancelled'\n" + + "Bug: Picker hangs indefinitely", + FontSize = 14 + }; + + var openButton = new Button + { + AutomationId = "OpenChildActivityButton", + Text = "Open Child Activity", + HorizontalOptions = LayoutOptions.Fill + }; + openButton.Clicked += OnOpenChildActivityClicked; + + var statusLabel = new Label + { + AutomationId = "StatusLabel", + Text = "Status: Ready", + FontSize = 16, + FontAttributes = FontAttributes.Bold + }; + + Content = new VerticalStackLayout + { + Padding = 30, + Spacing = 25, + Children = { instructions, openButton, statusLabel } + }; + } + + void OnOpenChildActivityClicked(object sender, EventArgs e) + { +#if ANDROID + var activity = Microsoft.Maui.ApplicationModel.Platform.CurrentActivity; + if (activity != null) + { + var intent = new Android.Content.Intent(activity, typeof(Issue35826ChildActivity)); + activity.StartActivity(intent); + } +#endif + } +} + +#if ANDROID +// A plain AppCompatActivity that calls MediaPicker.PickPhotosAsync(). +// Before the fix, Platform.Init() on this activity was silently ignored by the guard in +// ActivityForResultRequest.Register(), so no launcher was registered for it and the +// picker task never completed. After the fix each activity gets its own launcher entry +// in the ConditionalWeakTable, so the result is delivered correctly. +[Android.App.Activity(Label = "Issue35826 Child Activity", Theme = "@style/Maui.SplashTheme")] +public class Issue35826ChildActivity : AndroidX.AppCompat.App.AppCompatActivity +{ + Android.Widget.TextView _resultLabel; + + protected override void OnCreate(Android.OS.Bundle savedInstanceState) + { + base.OnCreate(savedInstanceState); + + Microsoft.Maui.ApplicationModel.Platform.Init(this, savedInstanceState); + + var layout = new Android.Widget.LinearLayout(this) + { + Orientation = Android.Widget.Orientation.Vertical + }; + layout.SetPadding(50, 50, 50, 50); + + _resultLabel = new Android.Widget.TextView(this) + { + Text = "Result: Ready" + }; + _resultLabel.SetPadding(0, 0, 0, 50); + SetViewIdResourceName(_resultLabel, "ChildActivityResultLabel"); + + var pickButton = new Android.Widget.Button(this) + { + Text = "Pick Photos" + }; + SetViewIdResourceName(pickButton, "ChildActivityPickButton"); + + pickButton.Click += async (_, _) => + { + _resultLabel.Text = "Result: Picking..."; + try + { + var result = await MediaPicker.PickPhotosAsync(); + _resultLabel.Text = result?.Count > 0 + ? $"Result: Got {result.Count} photo(s)" + : "Result: Cancelled"; + } + catch (OperationCanceledException) + { + _resultLabel.Text = "Result: Cancelled"; + } + catch (Exception ex) + { + _resultLabel.Text = $"Result: Error - {ex.Message}"; + } + }; + + layout.AddView(_resultLabel); + layout.AddView(pickButton); + SetContentView(layout); + } + + // Sets ViewIdResourceName on a native Android view so Appium can locate it by + // resource-id (the same mechanism MAUI uses for AutomationId on Android). + void SetViewIdResourceName(Android.Views.View view, string automationId) + { + var resourceName = $"{PackageName}:id/{automationId}"; + AndroidX.Core.View.ViewCompat.SetAccessibilityDelegate(view, new AutomationIdDelegate(resourceName)); + } + + class AutomationIdDelegate : AndroidX.Core.View.AccessibilityDelegateCompat + { + readonly string _resourceName; + + public AutomationIdDelegate(string resourceName) => _resourceName = resourceName; + + public override void OnInitializeAccessibilityNodeInfo(Android.Views.View host, AndroidX.Core.View.Accessibility.AccessibilityNodeInfoCompat info) + { + base.OnInitializeAccessibilityNodeInfo(host, info); + info.ViewIdResourceName = _resourceName; + } + } +} +#endif diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue35826.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue35826.cs new file mode 100644 index 000000000000..ba64118787f3 --- /dev/null +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue35826.cs @@ -0,0 +1,79 @@ +#if ANDROID +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.TestCases.Tests.Issues; + +public class Issue35826 : _IssuesUITest +{ + public Issue35826(TestDevice device) : base(device) { } + + public override string Issue => "PickPhotosAsync hangs when called from a child activity"; + + const string OpenChildActivityButton = "OpenChildActivityButton"; + const string ChildActivityPickButton = "ChildActivityPickButton"; + const string ChildActivityResultLabel = "ChildActivityResultLabel"; + + [Test] + [Category(UITestCategories.Essentials)] + public void PickPhotosAsyncShouldReturnFromChildActivity() + { + // This regression only manifests on Android API 36, where the ActivityResultLauncher + // ownership rules are enforced strictly enough that using the wrong activity's launcher + // causes the result to never be delivered, hanging the task indefinitely. + if (App is AppiumApp appiumApp) + { + var apiLevel = (long?)appiumApp.Driver.Capabilities.GetCapability("deviceApiLevel") ?? 0; + if (apiLevel < 36) + { + Assert.Ignore($"Issue #35826 only manifests on Android API 36+. Current device API: {apiLevel}."); + } + } + + // Verify the host page loaded + App.WaitForElement(OpenChildActivityButton); + + // Open the child (non-MAUI AppCompatActivity) + App.Tap(OpenChildActivityButton); + + // Verify the child activity's UI is visible + App.WaitForElement(ChildActivityPickButton); + App.WaitForElement(ChildActivityResultLabel); + + // Tap Pick Photos — calls MediaPicker.PickPhotosAsync() from the child activity. + // Before the fix the ActivityResultLauncher was never registered for child activities + // (the guard in ActivityForResultRequest.Register() blocked it), so the task hung + // indefinitely and the result label stayed on "Picking...". + App.Tap(ChildActivityPickButton); + + // Cancel the system photo picker by pressing Back. + // After the fix each activity has its own launcher entry so the result is delivered. + App.Back(); + + // If the bug is present WaitForTextToBePresentInElement times out because the + // TaskCompletionSource is never resolved. With the fix it updates promptly to + // the expected cancellation state. Error indicates a launcher/ownership failure + // or another exception path and must fail this regression. + var returned = App.WaitForTextToBePresentInElement(ChildActivityResultLabel, "Cancelled", + timeout: TimeSpan.FromSeconds(120)); + + var resultText = App.FindElement(ChildActivityResultLabel).GetText(); + + Assert.That(returned, Is.True, + $"PickPhotosAsync must return from a child activity as a cancellation result after backing out of the picker. " + + $"Actual result label: '{resultText}'. " + + $"If this fails the result label is still showing 'Picking...' after 120 seconds or an exception path was hit."); + + Assert.That(resultText, Does.Not.Contain("Picking"), + "PickPhotosAsync must not hang in a child activity."); + + Assert.That(resultText, Does.Not.Contain("Error"), + $"PickPhotosAsync should cancel cleanly when backing out of the picker, not surface an exception. Actual result label: '{resultText}'."); + + // Return to the host page + App.Back(); + App.WaitForElement(OpenChildActivityButton); + } +} +#endif \ No newline at end of file diff --git a/src/Essentials/src/MediaPicker/MediaPicker.android.cs b/src/Essentials/src/MediaPicker/MediaPicker.android.cs index 08a3de2da51b..e8cda06b4f0c 100644 --- a/src/Essentials/src/MediaPicker/MediaPicker.android.cs +++ b/src/Essentials/src/MediaPicker/MediaPicker.android.cs @@ -8,6 +8,7 @@ using Android.Content.PM; using Android.Graphics; using Android.Provider; +using AndroidX.Activity; using AndroidX.Activity.Result; using AndroidX.Activity.Result.Contract; using Microsoft.Maui.ApplicationModel; @@ -177,11 +178,14 @@ void OnResult(Intent intent) async Task PickUsingPhotoPicker(MediaPickerOptions options, bool photo) { + var launchingActivity = ActivityStateManager.Default.GetCurrentActivity(true) as ComponentActivity + ?? throw new InvalidOperationException("The current activity must inherit from AndroidX.Activity.ComponentActivity."); + var pickVisualMediaRequest = new PickVisualMediaRequest.Builder() .SetMediaType(photo ? ActivityResultContracts.PickVisualMedia.ImageOnly.Instance : ActivityResultContracts.PickVisualMedia.VideoOnly.Instance) .Build(); - var androidUri = await PickVisualMediaForResult.Instance.Launch(pickVisualMediaRequest); + var androidUri = await PickVisualMediaForResult.Instance.Launch(launchingActivity, pickVisualMediaRequest); if (androidUri?.Equals(AndroidUri.Empty) ?? true) { @@ -208,6 +212,9 @@ async Task PickUsingPhotoPicker(MediaPickerOptions options, bool pho async Task> PickMultipleUsingPhotoPicker(MediaPickerOptions options, bool photo) { + var launchingActivity = ActivityStateManager.Default.GetCurrentActivity(true) as ComponentActivity + ?? throw new InvalidOperationException("The current activity must inherit from AndroidX.Activity.ComponentActivity."); + // Android has a limitation that you need to use a different request for single and multiple picks. // If the selection limit is 1, we can use the single pick method, // otherwise we need to use the multiple pick method. @@ -230,7 +237,7 @@ async Task> PickMultipleUsingPhotoPicker(MediaPickerOptions opt var pickVisualMediaRequest = pickVisualMediaRequestBuilder.Build(); - var androidUris = await PickMultipleVisualMediaForResult.Instance.Launch(pickVisualMediaRequest); + var androidUris = await PickMultipleVisualMediaForResult.Instance.Launch(launchingActivity, pickVisualMediaRequest); if (androidUris?.IsEmpty ?? true) { diff --git a/src/Essentials/src/Platform/ActivityForResultRequest.android.cs b/src/Essentials/src/Platform/ActivityForResultRequest.android.cs index 89828d6ccc7b..26ad904c1ad6 100644 --- a/src/Essentials/src/Platform/ActivityForResultRequest.android.cs +++ b/src/Essentials/src/Platform/ActivityForResultRequest.android.cs @@ -1,5 +1,6 @@ using System; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Threading.Tasks; using AndroidX.Activity; using AndroidX.Activity.Result; @@ -19,40 +20,78 @@ namespace Microsoft.Maui.ApplicationModel; /// /// Google docs /// -/// This must be unconditionally registered every time our activity is created. +/// +/// Each instance gets its own registered launcher and its +/// own pending entry so that child activities +/// can use MediaPicker independently of the main activity, and so that two activities with +/// concurrent in-flight requests cannot clobber each other. +/// +/// +/// The result callback closes over the specific instance +/// that was passed to and resolves the pending TCS using THAT +/// instance as the lookup key. Result delivery therefore does NOT depend on whichever +/// activity is "current" at delivery time — which is critical because the launching +/// activity may have been destroyed and recreated (rotation/config change) before the +/// picker returns. +/// /// internal abstract class ActivityForResultRequest where TContract : ActivityResultContract, new() where TResult : JavaObject { - protected ActivityResultLauncher launcher; - protected TaskCompletionSource tcs = null; - protected WeakReference registeredActivity = null; + // Tracks one ActivityResultLauncher per ComponentActivity instance. + // ConditionalWeakTable holds weak references to keys, so entries are automatically + // eligible for collection when the activity is no longer referenced. + readonly ConditionalWeakTable _activityLaunchers = new(); + + // Tracks pending TaskCompletionSource per ComponentActivity to prevent race conditions. + // This prevents Activity B from overwriting Activity A's pending request. + readonly ConditionalWeakTable> _pendingRequests = new(); /// - /// Gets a value indicating whether the request is registered. + /// Gets a value indicating whether the request has a launcher registered for the + /// currently focused activity. /// - protected bool IsRegistered => launcher is not null; + protected bool HasLauncherForCurrentActivity => GetLauncherForCurrentActivity() is not null; /// /// Registers this request to start an activity for a result. + /// Each instance receives its own launcher so child + /// activities can use MediaPicker independently of the main activity. /// /// The component activity to register the request with. public void Register(ComponentActivity componentActivity) { - // Only register if we don't have a valid registration already - // This prevents temporary activities from invalidating the launcher registered with the main activity - if (registeredActivity?.TryGetTarget(out var existingActivity) == true && - !existingActivity.IsDestroyed && !existingActivity.IsFinishing) - { + if (componentActivity is null) + throw new ArgumentNullException(nameof(componentActivity)); + + // Skip if already registered for this specific activity instance (e.g. called again + // after a no-op restart). Calling RegisterForActivityResult twice on the same + // activity is not legal — must happen once during onCreate. + if (_activityLaunchers.TryGetValue(componentActivity, out _)) return; - } var contract = new TContract(); - var callback = new ActivityResultCallback(result => tcs?.SetResult(result)); - launcher = componentActivity.RegisterForActivityResult(contract, callback); - registeredActivity = new WeakReference(componentActivity); + // CRITICAL: capture the same `componentActivity` instance the launcher is being + // registered for. The callback resolves the pending TCS for THIS specific activity, + // NOT for whatever ActivityStateManager.Default.GetCurrentActivity() happens to be + // at delivery time. That makes delivery invariant under rotation / config changes — + // even if the activity is destroyed/recreated, the captured reference remains valid + // for the duration of the in-flight callback (Android keeps the registered activity + // alive long enough to deliver its own result). + var registeredActivity = componentActivity; + var callback = new ActivityResultCallback(result => + { + if (_pendingRequests.TryGetValue(registeredActivity, out var tcs)) + { + _pendingRequests.Remove(registeredActivity); + tcs?.TrySetResult(result); + } + }); + + var launcher = componentActivity.RegisterForActivityResult(contract, callback); + _activityLaunchers.Add(componentActivity, launcher); } /// @@ -66,14 +105,57 @@ public void Register(ComponentActivity componentActivity) public Task Launch(T input) where T : JavaObject { - tcs = new TaskCompletionSource(); + var launchingActivity = ActivityStateManager.Default.GetCurrentActivity() as ComponentActivity; + if (launchingActivity is null) + { + Trace.WriteLine(""" + ActivityForResultRequest.Launch() called but current activity is null or not a ComponentActivity. + Ensure your Activity inherits from ComponentActivity and call Microsoft.Maui.ApplicationModel.Platform.Init(Activity, Bundle) in OnCreate. + """); + var canceledTcs = new TaskCompletionSource(); + canceledTcs.SetCanceled(); + return canceledTcs.Task; + } + + return Launch(launchingActivity, input); + } + + /// + /// Launches the activity result request for a specific activity instance. + /// + /// The type of the input parameter. + /// The activity that owns the request lifecycle and launcher. + /// The input parameter to launch the request with. + /// + /// A task that represents the asynchronous operation, containing the result of the activity. + /// + public Task Launch(ComponentActivity launchingActivity, T input) + where T : JavaObject + { + if (launchingActivity is null) + throw new ArgumentNullException(nameof(launchingActivity)); - if (!IsRegistered) + if (_pendingRequests.TryGetValue(launchingActivity, out var existingTcs)) + { + // Instead of rejecting the new launch, cancel the orphaned previous request and replace it. + // This prevents permanent deadlock if a picker result never arrives due to process death or OEM edge cases. + // Rejection semantics would block all future launches from this activity forever. + Trace.WriteLine("ActivityForResultRequest: canceling overlapping pending request and launching new request."); + _pendingRequests.Remove(launchingActivity); + existingTcs?.TrySetCanceled(); + } + + var tcs = new TaskCompletionSource(); + _pendingRequests.Add(launchingActivity, tcs); + + // Get the launcher for this specific activity + if (!_activityLaunchers.TryGetValue(launchingActivity, out var launcher)) { Trace.WriteLine(""" - ActivityForResultRequest is not registered; cancelling the request. + ActivityForResultRequest is not registered for the launching activity; cancelling the request. Ensure your Activity inherits from ComponentActivity and call Microsoft.Maui.ApplicationModel.Platform.Init(Activity, Bundle) in OnCreate. """); + _pendingRequests.Remove(launchingActivity); tcs.SetCanceled(); return tcs.Task; } @@ -84,9 +166,36 @@ Ensure your Activity inherits from ComponentActivity and call Microsoft.Maui.App } catch (Exception ex) { - tcs.SetException(ex); + _pendingRequests.Remove(launchingActivity); + tcs.TrySetException(ex); } return tcs.Task; } -} \ No newline at end of file + + /// + /// Cancels any pending request for the specified activity. + /// This should be called from the activity's OnDestroy() or when the activity is being destroyed + /// to ensure the pending task is completed rather than hanging indefinitely. + /// + /// The activity whose pending request should be cancelled. + internal void CancelPendingRequest(ComponentActivity componentActivity) + { + if (_pendingRequests.TryGetValue(componentActivity, out var tcs)) + { + _pendingRequests.Remove(componentActivity); + tcs?.TrySetCanceled(); + } + } + + ActivityResultLauncher GetLauncherForCurrentActivity() + { + if (ActivityStateManager.Default.GetCurrentActivity() is ComponentActivity currentActivity && + _activityLaunchers.TryGetValue(currentActivity, out var launcher)) + { + return launcher; + } + + return null; + } +} diff --git a/src/Essentials/src/Platform/ActivityStateManager.android.cs b/src/Essentials/src/Platform/ActivityStateManager.android.cs index a66a1f2d7007..f7b63cb05002 100644 --- a/src/Essentials/src/Platform/ActivityStateManager.android.cs +++ b/src/Essentials/src/Platform/ActivityStateManager.android.cs @@ -206,8 +206,23 @@ void Application.IActivityLifecycleCallbacks.OnActivityCreated(Activity activity _onActivityStateChanged(activity, ActivityState.Created); } - void Application.IActivityLifecycleCallbacks.OnActivityDestroyed(Activity activity) => + void Application.IActivityLifecycleCallbacks.OnActivityDestroyed(Activity activity) + { + // Only cancel pending picker requests when the activity is truly finishing + // (user pressed Back, or the activity was explicitly finished). On a + // configuration-change destroy (e.g. rotation) the system immediately + // recreates the activity and the picker result still needs to be delivered + // to the original (captured) activity instance — cancelling here would + // turn rotation-during-picker into a silent task cancellation. + if (activity is ComponentActivity componentActivity && componentActivity.IsFinishing + && MediaPickerImplementation.IsPhotoPickerAvailable) + { + PickVisualMediaForResult.Instance.CancelPendingRequest(componentActivity); + PickMultipleVisualMediaForResult.Instance.CancelPendingRequest(componentActivity); + } + _onActivityStateChanged(activity, ActivityState.Destroyed); + } void Application.IActivityLifecycleCallbacks.OnActivityPaused(Activity activity) {