1492771 - Make GeckoSession non-final

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P3)

Description

[Paul Rouget [:paul]]

Bug 1486778 made GeckoSession final, making it impossible to extend GeckoSession (necessary for Servo support).

Comment 1

[Paul Rouget [:paul]]

Attachment: 1492771-01

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

It generally does not make much sense for consumers to subclass GeckoSession, which is why I made it final. Additionally, since GeckoView is not intended to be an abstraction layer for other engines, I'm not inclined to make API changes that cater to this type of usage.

I really think Servo should reconsider GeckoView compatibility as a goal and try to solve this at a higher level. The Android Components[0] "engine" may be a good fit. 

[0] https://github.com/mozilla-mobile/android-components/

Comment 3

[Paul Rouget [:paul]]

James, understood. I'll talk to the FxReality folks and see what should be the best approach then.

I already suggested to use Android Component: https://github.com/MozillaReality/FirefoxReality/issues/80

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

I think there are some arguments for subclassing GeckoSession. Subclassing is pretty common on Android to change the behavior of Application, Activity, View, etc... GeckoView itself is non-final.

There are some use cases where subclassing would be useful. For example, if the app wants to collect some telemetry whenever `GeckoSession.loadUri` is called, or if the app wants to associate every `GeckoSession` instance with some piece of data. In those cases there are workarounds, but subclassing would be the most straightforward way.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Jim and I talked about this a little more, and while it may make sense to subclass GeckoSession for reasons similar to the ones listed in comment #4, I think we need to be cautious about the protected stuff that's exposed and which methods can be overridden. From that perspective, LayerSession is a pretty big API wart (which is why I added 'final'). We'll accept the patch to remove 'final' from GeckoSession, but do not expect to be able to override every method in the future.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a7f1d2b04291
Make GeckoSession non-final r=snorp

Comment 7

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/a7f1d2b04291