Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Support for ServoSession - take 2 #585

Closed
wants to merge 3 commits into from

Conversation

paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Sep 27, 2018

This is an alternative approach to #498

In this case, we do not extend GeckoSession but introduce a wrapper around all the things that need to be abstracted. This is still WIP (Servo part not done yet). But I would like to get some early feedback before finishing this.

Pros/Cons for the 2 approach:

take 1 - #498

Pros: lot less boilerplate code. Less intrusive
Cons: Will break easily with any GV update (like: https://bugzilla.mozilla.org/show_bug.cgi?id=1492771)

take 2

Pros: more solid to changes in GV
Cons: rather intrusive and requires quite a lot of boilerplate code.

@MortimerGoro What do you think?

@MortimerGoro
Copy link
Contributor

I'd use the #498 approach as the first step. I see that as an easier & non intrusive solution until Android Component API is ready to use. IMO the efforts of doing the perfect abstraction should be part of the android components.

I think #498 will be ok for GV updates. We only need to override the public API methods we are using, and nothing to do with protected/private methods or LayerSession, @snorp correct me if I'm wrong.

Regarding the code in this PR, I'd try to avoid the isGecko() kind of methods, ideally the interface should be totally agnostic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants