Subclass instead of protocol/delegate

[joemasilotti]

This PR is an experiment to solve #69.

It flips the entire usage of TurboNavigator from delegate callbacks via the TurboNavigationDelegate to subclassing. Now, to customize behavior, you provide a custom subclass of the navigator.

Example

Now, to override different functionality a developer can subclass TurboNavigator.

class SceneDelegate: UIResponder, UIWindowSceneDelegate {
    private lazy var turboNavigator = MyTurboNavigator()

    /* ... */
}

class MyTurboNavigator: TurboNavigator {
    override func handle(proposal: VisitProposal) -> ProposalResult {
        .acceptCustom(TurboWebViewController(url: proposal.url))
    }

    override func sessionDidStartRequest(_ session: Session) {
        // Custom handling for SessionDelegate callback.
    }

    override func webView(_ webView: WKWebView, runJavaScriptAlertPanelWithMessage message: String, initiatedByFrame frame: WKFrameInfo, completionHandler: @escaping () -> Void) {
        // Custom handling for WKUIDelegate callback.
    }
}

Benefits

• Less code - a default implementation doesn't even require the empty implementation of the protocol
• Developers can override any function from SessionDelegate, WKUIDelegate, or whatever we add in the future
• Subclassing might be easier to understand for developers more familiar with Rails (vs. protocols and delegates)

Drawbacks

• Turbo Navigator still needs to implement every single function of a protocol so developers can override the functionality
• Protocols/delegates feel more "Swift-y" and subclassing breaks the open-closed principle

I'm most concerned about the first drawback because it means Turbo Navigator / turbo-ios can potentially lag behind iOS releases if a new callback is added to an Apple API. This is where default implementations in extensions have a huge benefit.

[jayohms]

@joemasilotti Can you elaborate on:

I'm most concerned about the first drawback because it means Turbo Navigator / turbo-ios can potentially lag behind iOS releases if a new callback is added to an Apple API

What specific protocol would be vulnerable to lagging behind iOS?

[joemasilotti]

What specific protocol would be vulnerable to lagging behind iOS?

WKUIDelegate. If iOS 18 adds a new function we would need to update our code to declare that function with an empty implementation. Otherwise, a developer can't override it so it would never be called.

Looking ahead, if we extend this approach to other Apple frameworks the same problem would apply.

[jayohms]

Ah ok. I'm not quite sure I understand, fully. It looks like you're implementing 2 methods out of many here: https://developer.apple.com/documentation/webkit/wkuidelegate

Is that just an example of providing two capabilities, but we'd provide more empty implementations if we went down this path?

[joemasilotti]

Ah, you're right. See, I already forgot! 😆

This implementation should declare every one of the 18 functions on the protocol.

[jayohms]

Ha, yeah that won't work. We don't want to have to be in control of exposing every WKUIDelegate method. If there's not an easy way to provide a subset of default behaviors + customizability, maybe WKUIDelegate needs to be a special case where it's all-or-nothing. You provide your own or use the default implementation.

But for anything that's specific to TurboNavigator or turbo-ios (anything we control), we should allow individual opportunities for customizability without throwing away default behaviors.

[joemasilotti]

Both of those sound right to me.

I'd love to follow the guidance of the Android version of this, as you recommended earlier. For example, a new SessionNavigationDelegate class that developers could subclass to provide their own implementation of SessionDelegate functions we don't implement. (Same goes for a new WebKitDelegate class to do the same for WKUIDelegate.)

But I can't seem to break the dependency between these tertiary objects and the core of TurboNavigator. For example, sessionDidFinishFormSubmission(_:) needs to know if the Session triggering the call was the modal one so it can invalidate the cache on the root one! That requires knowing a) if a modal is presented b) which Session is which and c) a reference to the root Session.

If we don't avoid this then when a developer subclasses SessionNavigationDelegate they will need to pass in their own TurboNavigator instance. Which feels awkward and could easily create a circular dependency.

I can't tell if the current architecture is prohibiting this or we are going to run into this wall no matter what.

---

I'm leaning towards going back to the original implementation of protocol/delegate and exposing all of SessionDelegate through the TurboNavigationDelegate. Then WKUIDelegate will be optional - if a developer provides one we use it as it. Otherwise, they get default a default implementation that cannot be customized. (They can always copy-paste.)

The downside is that we will no longer "wrap" the SessionDelegate API in something more abstracted. But maybe that shouldn't have been a goal in the first place?

Edit: Implemented in #73.

[joemasilotti]

Closed in favor of #74.