Is this the best way to handle popping the current visitable ViewController from the backstack?

Could this be calling a delegate instead? This would allow customization of the behavior here and the Session would be less dependent on the app being structured in a specific way, ie. the Session doesn't really 'push' or 'pop' any controllers today - controllers just need to implement Visitable.

Yeah, I figured that'd probably be where we ended up, makes sense 👍

An alternative would be to implement a method in Visitable that provides the behavior as in the PR, but could be overwritten and customized in the controller that implements Visitable, ie for. controllers not living in a UINavigationController, eg.:

// Session
func webView(_ bridge: WebViewBridge, didProposeVisitToCrossOriginRedirect location: URL) {
   activatedVisitable?.removeDueToCrossOriginRedirect()
}

// Visitable
func removeDueToCrossOriginRedirect() {
  // Sensible default, but could be customized in controller implementing Visitable
  visitableViewController.navigationController?.popViewController(animated: false)
}

Would it be better to perform the visit with a REPLACE action instead of manually manipulating the backstack?

I don't think a replace visit fits the model here very well. The redirect url is for an external domain, so there won't be an in-app visit that takes place. We need to drop the current visitable from the backstack before opening the redirect url in an external app/browser, since it'll be displaying the progress indicator when the cross-origin redirect is seen.

Ah, gotcha. Then I think we should find a way to delegate to the navigator to perform the stack manipulation.

After digging into the PR, I'm leaning more towards @pfeiffer's recommendation of an additional delegate callback on SessionDelegate. This would require the developer to do the popping manually but we can handle that for them in Turbo Navigator. I like this approach because if the developer isn't using a navigation controller then they have full control of what to do.

But more importantly, if the redirected screen was opened in a modal then we need to dismiss not pop the new (blank) view controller from the screen. This will be a one-liner in Turbo Navigator because we already have logic that handles those situations.

Very good points @joemasilotti! I agree and I like the suggestion of an additional delegate callback on SessionDelegate.

After digging into the PR, I'm leaning more towards @pfeiffer's recommendation of an additional delegate callback on SessionDelegate. This would require the developer to do the popping manually but we can handle that for them in Turbo Navigator. I like this approach because if the developer isn't using a navigation controller then they have full control of what to do.

But more importantly, if the redirected screen was opened in a modal then we need to dismiss not pop the new (blank) view controller from the screen. This will be a one-liner in Turbo Navigator because we already have logic that handles those situations.