Address onDismiss closure not being called in present(item:) function

[acosmicflamingo]

Addresses #257

While trying to create a test reproducer, I found that the issue actually doesn't deterministically fail in the tests, even though it is always reproduced and fails in the simulator. It gave me the impression that there is some race condition going on and after messing around with timeout values in the tests, I came across a potential solution that might actually be pretty clean to merge to main.

Once CI reports that the test fails, I'll push the following change in a new commit to demonstrate that it indeed fixes the problem:

diff --git a/Sources/UIKitNavigationShim/shim.m b/Sources/UIKitNavigationShim/shim.m
index a17a88c6..625b8f69 100644
--- a/Sources/UIKitNavigationShim/shim.m
+++ b/Sources/UIKitNavigationShim/shim.m
@@ -51,15 +51,10 @@ - (void)UIKitNavigation_viewDidDisappear:(BOOL)animated {
       [self UIKitNavigation_viewDidDisappear:animated];
 
       if ((self.isBeingDismissed || self.isMovingFromParentViewController) && self._UIKitNavigation_onDismiss != NULL) {
-        if ([self isKindOfClass:UIAlertController.class]) {
-          dispatch_async(dispatch_get_main_queue(), ^{
-            self._UIKitNavigation_onDismiss();
-            self._UIKitNavigation_onDismiss = nil;
-          });
-        } else {
-          self._UIKitNavigation_onDismiss();
+        self._UIKitNavigation_onDismiss();
+        dispatch_async(dispatch_get_main_queue(), ^{
           self._UIKitNavigation_onDismiss = nil;
-        }
+        });
       }
     }

[acosmicflamingo]

All tests pass! Yay! Now, if that change seems a bit too unconventional for shim.m, then the alternative could be a change like this:

diff --git a/Sources/UIKitNavigation/Navigation/Presentation.swift b/Sources/UIKitNavigation/Navigation/Presentation.swift
index 3e9267cc..cf485b5b 100644
--- a/Sources/UIKitNavigation/Navigation/Presentation.swift
+++ b/Sources/UIKitNavigation/Navigation/Presentation.swift
@@ -445,13 +445,15 @@
 
   @MainActor
   private class Presented {
-    weak var controller: UIViewController?
+    var controller: UIViewController?
     let presentationID: AnyHashable?
     deinit {
       // NB: This can only be assumed because it is held in a UIViewController and is guaranteed to
       //     deinit alongside it on the main thread. If we use this other places we should force it
       //     to be a UIViewController as well, to ensure this functionality.
       MainActor._assumeIsolated {
+        defer { controller = nil }
+
         guard let controller, controller.parent == nil else { return }
         controller.dismiss(animated: false)
       }

Has the same effect of making sure that the dust has settled before nil-ing out a property.

[acosmicflamingo]

The following test is absolutely flaky:

Test Case '-[CaseStudiesTests.PresentationTests testPushViewController_ManualPop]' started.
/Users/runner/work/swift-navigation/swift-navigation/Sources/UIKitNavigation/Navigation/Presentation.swift:215: error: -[CaseStudiesTests.PresentationTests testPushViewController_ManualPop] : failed - Can't dismiss navigation item: "navigationController" is "nil".
Test Case '-[CaseStudiesTests.PresentationTests testPushViewController_ManualPop]' failed (1.674 seconds).

You can see that between the successful commit I pushed and the latest failure that occurred, I changed absolutely nothing that would impact the test.

[acosmicflamingo]

I have confirmed the flakiness of PresentationTests.testPushViewController_ManualPop; I pushed 6 no-op commits and only saw 1 failure in #304.