Stamp routed location on view controllers for navigation identity

Source/Turbo/Navigator/Extensions/UIViewController+RoutedLocation.swift

@@ -0,0 +1,13 @@
+import UIKit
+
+private let routedLocationKey = malloc(1)!
+
+extension UIViewController {
+    /// The location the navigator routed this view controller to, stamped at route time by
+    /// `NavigationHierarchyController`. Navigation identity checks read this so two instances
+    /// of the same custom view-controller class at different URLs are distinguishable.
+    var routedLocation: URL? {
+        get { objc_getAssociatedObject(self, routedLocationKey) as? URL }
+        set { objc_setAssociatedObject(self, routedLocationKey, newValue, .OBJC_ASSOCIATION_RETAIN_NONATOMIC) }
+    }
+}

Source/Turbo/Navigator/NavigationHierarchyController.swift

@@ -37,6 +37,8 @@ class NavigationHierarchyController {
         if let alert = controller as? UIAlertController {
             presentAlert(alert, via: proposal)
         } else {
+            controller.routedLocation = proposal.url
+
             if let visitable = controller as? Visitable {
                 visitable.visitableView.allowsPullToRefresh = proposal.pullToRefreshEnabled
             }
@@ -134,9 +136,9 @@ class NavigationHierarchyController {
                                with controller: UIViewController,
                                via proposal: VisitProposal,
                                didReplaceModalContext: Bool = false) {
-        if visitingSamePage(on: navigationController, with: controller, via: proposal) {
+        if visitingSamePage(on: navigationController, via: proposal) {
             navigationController.replaceLastViewController(with: controller)
-        } else if visitingPreviousPage(on: navigationController, with: controller, via: proposal) {
+        } else if visitingPreviousPage(on: navigationController, via: proposal) {
             navigationController.popViewController(animated: proposal.animated)
         } else if proposal.options.action == .advance || didReplaceModalContext {
             navigationController.pushViewController(controller, animated: proposal.animated)
@@ -145,27 +147,41 @@ class NavigationHierarchyController {
         }
     }
 
-    private func visitingSamePage(on navigationController: UINavigationController,
-                                  with controller: UIViewController,
-                                  via proposal: VisitProposal) -> Bool {
-        if let visitable = navigationController.topViewController as? Visitable {
-            return visitable.initialVisitableURL.isSameLocation(as: proposal.url, pathProperties: proposal.properties)
-        } else if let topViewController = navigationController.topViewController {
-            return topViewController.isMember(of: type(of: controller))
+    private func visitingSamePage(
+        on navigationController: UINavigationController,
+        via proposal: VisitProposal
+    ) -> Bool {
+        guard let topViewController = navigationController.topViewController else { return false }
+
+        guard let location = topViewController.routedLocation else {
+            logger.warning("Top view controller \(topViewController) has no routed location; " +
+                           "treating the visit as a new page. Expected for controllers pushed onto the stack outside the navigator.")
+            return false
         }
-        return false
+
+        return location.isSameLocation(
+            as: proposal.url,
+            pathProperties: proposal.properties
+        )
     }
 
-    private func visitingPreviousPage(on navigationController: UINavigationController,
-                                      with controller: UIViewController,
-                                      via proposal: VisitProposal) -> Bool {
+    private func visitingPreviousPage(
+        on navigationController: UINavigationController,
+        via proposal: VisitProposal
+    ) -> Bool {
         guard navigationController.viewControllers.count >= 2 else { return false }
 
         let previousController = navigationController.viewControllers[navigationController.viewControllers.count - 2]
-        if let previousVisitable = previousController as? VisitableViewController {
-            return previousVisitable.initialVisitableURL.isSameLocation(as: proposal.url, pathProperties: proposal.properties)
+        guard let location = previousController.routedLocation else {
+            logger.warning("Previous view controller \(previousController) has no routed location; " +
+                           "treating the visit as a new page. Expected for controllers pushed onto the stack outside the navigator.")
+            return false
         }
-        return type(of: previousController) == type(of: controller)
+
+        return location.isSameLocation(
+            as: proposal.url,
+            pathProperties: proposal.properties
+        )
     }
 
     private func replace(with controller: UIViewController, via proposal: VisitProposal) {

Tests/Turbo/Navigator/NavigationHierarchyControllerTests.swift

@@ -83,6 +83,55 @@ final class NavigationHierarchyControllerTests: XCTestCase {
         assertVisited(url: oneURL, on: .main)
     }
 
+    func test_customViewController_visitingDifferentURL_pushesOnMainStack() {
+        let navigator = makeCustomViewControllerNavigator()
+
+        navigator.route(oneURL)
+        XCTAssertEqual(navigationController.viewControllers.count, 1)
+        XCTAssert(navigationController.topViewController is CustomViewController)
+
+        navigator.route(twoURL)
+        XCTAssertEqual(navigationController.viewControllers.count, 2)
+        XCTAssert(navigationController.topViewController is CustomViewController)
+    }
+
+    func test_customViewController_visitingSameURL_replacesOnMainStack() {
+        let navigator = makeCustomViewControllerNavigator()
+
+        navigator.route(oneURL)
+        XCTAssertEqual(navigationController.viewControllers.count, 1)
+
+        navigator.route(oneURL)
+        XCTAssertEqual(navigationController.viewControllers.count, 1)
+        XCTAssert(navigationController.topViewController is CustomViewController)
+    }
+
+    func test_customViewController_visitingPreviousURL_popsOnMainStack() {
+        let navigator = makeCustomViewControllerNavigator()
+
+        navigator.route(oneURL)
+        navigator.route(twoURL)
+        XCTAssertEqual(navigationController.viewControllers.count, 2)
+
+        navigator.route(oneURL)
+        XCTAssertEqual(navigationController.viewControllers.count, 1)
+        XCTAssert(navigationController.topViewController is CustomViewController)
+    }
+
+    func test_unroutedViewController_onStack_visitPushesOnMainStack() {
+        // A view controller pushed onto the stack outside the navigator (e.g. mixed native
+        // navigation) has no routed location. A subsequent visit must treat it as a new page
+        // and push — never crash or incorrectly replace/pop.
+        navigationController.pushViewController(UIViewController(), animated: false)
+        XCTAssertEqual(navigationController.viewControllers.count, 1)
+
+        navigator.route(oneURL)
+
+        XCTAssertEqual(navigationController.viewControllers.count, 2)
+        XCTAssert(navigationController.topViewController is VisitableViewController)
+        assertVisited(url: oneURL, on: .main)
+    }
+
     func test_default_default_default_replaceAction_replacesOnMainStack() {
         let proposal = VisitProposal(action: .replace)
         navigator.route(proposal)
@@ -482,12 +531,31 @@ final class NavigationHierarchyControllerTests: XCTestCase {
 
     private var navigator: Navigator!
     private let alertControllerDelegate = AlertControllerDelegate()
+    // `Navigator.delegate` is weak, so the test must hold a strong reference.
+    private let customViewControllerDelegate = CustomViewControllerDelegate()
     private var hierarchyController: NavigationHierarchyController!
     private var navigationController: TestableNavigationController!
     private var modalNavigationController: TestableNavigationController!
 
     private let window = UIWindow()
 
+    // A navigator whose delegate routes custom, non-`Visitable` view controllers, wired to the
+    // same navigation controllers the test asserts against.
+    private func makeCustomViewControllerNavigator() -> Navigator {
+        let navigator = Navigator(
+            session: session,
+            modalSession: modalSession,
+            delegate: customViewControllerDelegate,
+            configuration: .init(name: "Test", startLocation: oneURL)
+        )
+        navigator.hierarchyController = NavigationHierarchyController(
+            delegate: navigator,
+            navigationController: navigationController,
+            modalNavigationController: modalNavigationController
+        )
+        return navigator
+    }
+
     // Simulate a "real" app so presenting view controllers works under test.
     private func loadNavigationControllerInWindow() {
         window.rootViewController = navigationController
@@ -548,3 +616,15 @@ private class AlertControllerDelegate: NavigatorDelegate {
         return .accept
     }
 }
+
+// MARK: - CustomViewControllerDelegate
+
+/// A non-`Visitable` custom view controller. Two instances at different URLs share a type, so
+/// the navigator must rely on the stamped routed location — not type equality — to tell them apart.
+private final class CustomViewController: UIViewController {}
+
+private class CustomViewControllerDelegate: NavigatorDelegate {
+    func handle(proposal: VisitProposal, from navigator: Navigator) -> ProposalResult {
+        .acceptCustom(CustomViewController())
+    }
+}