mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
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 <klopez4212@gmail.com> Co-authored-by: Princess Donut <b238ea756dee4d98afa5883fc7f1de61eeabe65bf700e3a5a5a80db5e42e2c2b@buzz.block.builderlab.xyz> Signed-off-by: Kenny Lopez <klopez4212@gmail.com>
This commit is contained in:
co-authored by
Princess Donut
parent
50b0ab9a01
commit
826fd105c1
@@ -420,6 +420,7 @@ actor NativeEmojiRemoteImageLoader {
|
||||
|
||||
private let maximumConcurrentDownloads: Int
|
||||
private let downloader: Downloader
|
||||
private let admissionAttemptForTesting: (() -> Void)?
|
||||
private let cache = NSCache<NSURLRequest, UIImage>()
|
||||
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 {
|
||||
|
||||
@@ -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..<taskCount).map { index in
|
||||
Task {
|
||||
try await loader.image(
|
||||
for: URLRequest(
|
||||
@@ -476,8 +483,8 @@ class RunnerTests: XCTestCase {
|
||||
}
|
||||
}
|
||||
|
||||
await fulfillment(of: [tasksAttemptedAdmission], timeout: 2)
|
||||
await probe.waitUntilStarted(maximumConcurrentDownloads)
|
||||
try await Task.sleep(nanoseconds: 50_000_000)
|
||||
var snapshot = await probe.snapshot()
|
||||
XCTAssertEqual(snapshot.started, maximumConcurrentDownloads)
|
||||
XCTAssertEqual(snapshot.peakActive, maximumConcurrentDownloads)
|
||||
|
||||
@@ -18,9 +18,13 @@ Future<void> _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);
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user