From 826fd105c10a3261ef3afd665df8c3bd2a52a336 Mon Sep 17 00:00:00 2001 From: Kenny Lopez Date: Mon, 17 Aug 2026 18:49:57 +0100 Subject: [PATCH] Complete rejected emoji picker lifecycles Complete a reentrant iOS picker caller immediately instead of leaving its open-state callback stranded, while preserving ownership of the live native sheet. Make the native download concurrency regression wait until every task has attempted admission before checking the active bound, removing the timing-based sleep. Co-authored-by: Kenny Lopez Co-authored-by: Princess Donut Signed-off-by: Kenny Lopez --- mobile/ios/Runner/NativeEmojiPickerView.swift | 8 +++++++ mobile/ios/RunnerTests/RunnerTests.swift | 21 ++++++++++++------- .../emoji_picker/ios_native_picker.dart | 10 ++++++--- .../features/channels/emoji_picker_test.dart | 8 ++++--- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/mobile/ios/Runner/NativeEmojiPickerView.swift b/mobile/ios/Runner/NativeEmojiPickerView.swift index 610eb1a86..ebed6e73f 100644 --- a/mobile/ios/Runner/NativeEmojiPickerView.swift +++ b/mobile/ios/Runner/NativeEmojiPickerView.swift @@ -420,6 +420,7 @@ actor NativeEmojiRemoteImageLoader { private let maximumConcurrentDownloads: Int private let downloader: Downloader + private let admissionAttemptForTesting: (() -> Void)? private let cache = NSCache() private var activeDownloadCount = 0 private var waiters: [Waiter] = [] @@ -427,11 +428,13 @@ actor NativeEmojiRemoteImageLoader { init( maximumConcurrentDownloads: Int = defaultMaximumConcurrentDownloads, cacheByteLimit: Int = defaultCacheByteLimit, + admissionAttemptForTesting: (() -> Void)? = nil, downloader: @escaping Downloader = NativeEmojiRemoteImageLoader.download ) { precondition(maximumConcurrentDownloads > 0) precondition(cacheByteLimit >= 0) self.maximumConcurrentDownloads = maximumConcurrentDownloads + self.admissionAttemptForTesting = admissionAttemptForTesting self.downloader = downloader cache.totalCostLimit = cacheByteLimit } @@ -442,6 +445,7 @@ actor NativeEmojiRemoteImageLoader { return cached } + recordAdmissionAttemptForTesting() try await acquireDownloadSlot() defer { releaseDownloadSlot() } @@ -455,6 +459,10 @@ actor NativeEmojiRemoteImageLoader { return image } + private func recordAdmissionAttemptForTesting() { + admissionAttemptForTesting?() + } + private func acquireDownloadSlot() async throws { try Task.checkCancellation() guard activeDownloadCount >= maximumConcurrentDownloads else { diff --git a/mobile/ios/RunnerTests/RunnerTests.swift b/mobile/ios/RunnerTests/RunnerTests.swift index f856ae4ff..a6ccdde99 100644 --- a/mobile/ios/RunnerTests/RunnerTests.swift +++ b/mobile/ios/RunnerTests/RunnerTests.swift @@ -458,15 +458,22 @@ class RunnerTests: XCTestCase { func testRemoteEmojiLoaderLimitsConcurrentDownloads() async throws { let maximumConcurrentDownloads = 3 + let taskCount = 8 let probe = NativeEmojiDownloadProbe() + let tasksAttemptedAdmission = XCTestExpectation( + description: "all download tasks attempted admission" + ) + tasksAttemptedAdmission.expectedFulfillmentCount = taskCount let loader = NativeEmojiRemoteImageLoader( maximumConcurrentDownloads: maximumConcurrentDownloads, - cacheByteLimit: 0 - ) { _ in - await probe.holdDownload() - return UIImage() - } - let tasks = (0..<8).map { index in + cacheByteLimit: 0, + admissionAttemptForTesting: { tasksAttemptedAdmission.fulfill() }, + downloader: { _ in + await probe.holdDownload() + return UIImage() + } + ) + let tasks = (0.. _presentIosEmojiPicker({ required void Function(String emoji) onSelect, VoidCallback? onDismiss, }) async { - // Only one native sheet owns the handler at a time; coalesce a reentrant open - // so it cannot steal the live sheet's callbacks from its original owner. - if (_iosEmojiPickerPresenting) return; + // Only one native sheet owns the handler at a time. Reject a reentrant open + // without replacing the live sheet's callbacks, and complete the rejected + // caller so its local picker-open lifecycle is not stranded. + if (_iosEmojiPickerPresenting) { + onDismiss?.call(); + return; + } _iosEmojiPickerPresenting = true; final container = ProviderScope.containerOf(context, listen: false); diff --git a/mobile/test/features/channels/emoji_picker_test.dart b/mobile/test/features/channels/emoji_picker_test.dart index 8c7c05ef5..f64932f0a 100644 --- a/mobile/test/features/channels/emoji_picker_test.dart +++ b/mobile/test/features/channels/emoji_picker_test.dart @@ -941,11 +941,13 @@ void main() { await tester.pumpAndSettle(); expect(presents, 1); - // A second open while the first sheet is live is coalesced: it neither - // presents again nor replaces the live sheet's method-call handler. + // A second open while the first sheet is live is rejected: it neither + // presents again nor replaces the live sheet's method-call handler, + // and its independent lifecycle is completed immediately. await tester.tap(find.text('Open second')); await tester.pumpAndSettle(); expect(presents, 1); + expect(secondDismissals, 1); // Native events still reach the original owner, and only it. await _sendNativeEmojiPickerCall(tester, 'selected', '\u{1F525}'); @@ -953,7 +955,7 @@ void main() { expect(firstSelected, ['\u{1F525}']); expect(secondSelected, isEmpty); expect(firstDismissals, 1); - expect(secondDismissals, 0); + expect(secondDismissals, 1); } finally { _setMockNativeEmojiPickerHandler(null); debugDefaultTargetPlatformOverride = previousPlatform;