1134262 - Figure out a way to encapsulate Browser's WKWebView

Status: RESOLVED INVALID

(Firefox for iOS :: General, defect)

Description

[Brian Nicholson (:bnicholson)]

Browser is a tab abstraction that has methods like goBack/goForward/loadRequest/etc. that correlate to user interactions on the tab. These methods are all wrappers on the underlying WKWebView. Browser also exposes the WKWebView itself, however, which means consumers can go around the intended tab layer, breaking encapsulation. Right now, we're just guarding against this manually, which is error prone and difficult to scale.

Because we don't want to wrap every piece of the WKWebView API that we're using, we do still need BVC to have direct access to the webView for setting up WK delegates, hooking into the back/forward list, etc. It'd be nice to figure out a way to make Browser exclusive to BVC and hidden to everyone else (similar to friends in C++), which may mean we should define BVC and Browser in the same file.

Comment 1

[Wesley Johnston (:wesj)]

I'm in favor of just making the webview private entirely if we can. Is there some real reason we need to expose it? i.e. the delegates seems like something easy to wrap. The back/forward list is something we can forward across a bridge. We can have a generic getView() method for attaching it to the BVC (yeah, that exposes it slightly, but at least attempts to make it somewhat generic).

Comment 2

[Brian Nicholson (:bnicholson)]

(In reply to Wesley Johnston (:wesj) from comment #1)
> Is there some real reason we need to expose it? i.e. the delegates seems like
> something easy to wrap.

I wouldn't say it's easy to wrap. Aside from the webView delegates themselves, we'd have to wrap all of the WK* classes used in these delegates: WKWebViewConfiguration, WKNavigationAction, WKWindowFeatures, etc...

Comment 3

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

I don't think hiding the WKWebView is going to help us at all. Instead I think we should embrace the WKWebView API and not try to abstract it away. It will only introduce extra layers and make things more complicated. I don't see a good reason to do that.

Right now the API of Browser is pretty much a copy of what WKWebView exposes. What is the point of having functions like goBack() { webView.goBack() } ... this does not add anything except for indirection.

I think if we feel a need to customize the behaviour of WKWebView, we should make Browser a WKWebView subclass. I think this is what we had in an initial revision of the app.