1363567 - Remove plugin-related methods from GeckoInterface

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Comment 1

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

Attachment: 1. Remove addPluginView/removePluginView methods (v1)

Remove the addPluginView and removePluginView methods from
GeckoInterface. Instead, move the JNI calls directly to GeckoApp itself.
GeckoApp then uses GeckoActivityMonitor to find the current activity,
instead of using GeckoAppShell.getGeckoInterface().

Comment 2

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

Attachment: 2. Move onFullScreenPluginHidden to GeckoApp (v1)

Move the native call onFullScreenPluginHidden from GeckoAppShell to
GeckoApp itself.

Comment 3

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

Attachment: 3. Remove GeckoInterface.getPluginContainer (v1)

GeckoInterface.getPluginContainer is no longer used anywhere and can be
removed. r=me for trivial patch.

Comment 4

[Jan Henning [:JanH]]

Before you land, a question: If the GeckoActivityMonitor didn't exist, what would you have done instead? I'm asking because Sebastian wasn't entirely happy about it when I added it in bug 1352997, and since I've found a way to no longer need it for that purpose, I was intending to remove it again in bug 1359531.

Comment 5

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

We would need some other way to keep track of the active GeckoApp if we don't have GeckoActivityMonitor. What are the arguments against GeckoActivityMonitor?

Comment 6

[Jan Henning [:JanH]]

Correction: The GAM was added in bug 1351739.

That aside, you'd have to ask Sebastian what his objections were exactly, and whether they were more general, or just specific to the context of my patch.

Since you're just interested in the current activity if it's something GeckoApp-based, a more limited version of the activity monitor could be recreated by building on our current onActivityPause/Resume-tracking in GeckoApplication (https://dxr.mozilla.org/mozilla-central/rev/73b3fc64525b6816842c737e104ef2ac5482d217/mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java#144-182).

Having said that, even if soon the activity monitor won't be needed for web app/custom tab activity switching any more, our application-background/-foreground tracking in GeckoApplication has some gaps and to be honest I'm pretty undecided on whether the GAM could help there or not. So I'm no longer quite as certain about immediately removing it without experimenting a little in that direction ...

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/06142f9787e7
1. Remove addPluginView/removePluginView methods; r=rbarker
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f9d8749eb5e
2. Move onFullScreenPluginHidden to GeckoApp; r=rbarker
https://hg.mozilla.org/integration/mozilla-inbound/rev/49c90446a20f
3. Remove GeckoInterface.getPluginContainer; r=me

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/06142f9787e7
https://hg.mozilla.org/mozilla-central/rev/8f9d8749eb5e
https://hg.mozilla.org/mozilla-central/rev/49c90446a20f

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

We talked on IRC briefly yesterday.

My objections were more general than about this particular implementation. There are a bunch of edge cases that make tracking this hard and it can lead to wrong assumptions (e.g. a paused activity doesn't mean that it's not visible). For tab switching we are able to avoid it - if we need it for other features then I guess we will have to keep it for now.