1543755 - [meta] Support browser.tabs in GeckoView

Status: NEW

(GeckoView :: Extensions, enhancement, P3)

Description

[[ex-Mozilla] Agi Sferro | :agi]

GeckoView does not have the concept of tabs so we need to delegate this at least partially to the app.

https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/tabs

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

:agi, can you clarify here, according to https://github.com/mozilla-mobile/fenix/issues/1561#issuecomment-482199912, we are able to run raptor with reference-browser and geckoview-example, but not fenix. All 3 are built on top of geckoview. Do both the example app and reference-browser implement tabs on their own?

Comment 2

[[ex-Mozilla] Agi Sferro | :agi]

GeckoView doesn't know about tabs, so all browsers have to implement them (or just have one tab like geckoview_example).

I'll look a little more into why it breaks on Fenix and clear the ni.

Comment 3

[Stuart Philp :sphilp]

It seems like there were a few workarounds implemented on r-b? Things like https://github.com/mozilla-mobile/reference-browser/pull/707 and https://github.com/mozilla-mobile/reference-browser/pull/460

Would similar workarounds be appropriate to unblock this for now so we can move forward with capturing pageload numbers? I know nalexander had concerns about those implementation and it may be the wrong thing to do for the long term, but short term to move forward. Anything to add Nick?

Comment 4

[Nick Alexander :nalexander [he/him]]

(In reply to Stuart Philp :sphilp from comment #3)

It seems like there were a few workarounds implemented on r-b? Things like https://github.com/mozilla-mobile/reference-browser/pull/707 and https://github.com/mozilla-mobile/reference-browser/pull/460

So #460 introduces a new Activity just for browser (performance) testing; its equivalent already exists in Fenix. It's not my preferred way to do this but it's here and it works, so no issue there.

Would similar workarounds be appropriate to unblock this for now so we can move forward with capturing pageload numbers? I know nalexander had concerns about those implementation and it may be the wrong thing to do for the long term, but short term to move forward.

#707 is odd. It "kinda-sorta" sets up r-b specially in that Activity from #460. This is what we should limit, but it shouldn't be needed. If we make:

1. Make the Activity from #460 (or equivalent) pass through to the regular flow, and
2. Start with -n .../Activity -a android.intent.action.VIEW -d URL

then the App better connect Gecko for driving, 'cuz that's just a well-supported path that needs to work. That's what Fenix does right now for Marionette/geckodriver driving, and it works fine.

Anything to add Nick?

Not really. Whatever work-arounds are needed are fine by me; we can't and shouldn't block on perfection! Sorry that I didn't have a chance to look into this on Friday. I gather rwood is back on it today.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Rob is back from PTO today, but refactoring Raptor to use Marionette/geckodriver is not a trivial task. We should consider other options that allow us to integrate Raptor with Fenix in a shorter timeframe.

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Dave Hunt [:davehunt] [he/him] ⌚️UTC from comment #5)

Rob is back from PTO today, but refactoring Raptor to use Marionette/geckodriver is not a trivial task. We should consider other options that allow us to integrate Raptor with Fenix in a shorter timeframe.

Oh, right -- not saying we should use Marionette for this at all. This is strictly about starting Fenix such that their is a tab/GeckoSession that the browser knows about. Get the Raptor extension into the profile directory, start with the pass-through activity, and Raptor should Just Work without anything like r-b's #707.

Comment 7

[Robert Wood [:rwood]]

Christian and I looked into this and figured out the issue. Raptor is launching the android app (mozdevice ADBDevice) using default intent of 'android.intent.action.Main' but for Fenix we need to specify the intent of 'android.intent.action.VIEW' instead. We tried that locally and with that change Raptor page-load runs fine on Fenix using the one tab / tabs.update. I'll file a Raptor bug and make a patch.

[0] https://searchfox.org/mozilla-central/rev/1b2636e8517aa48422ed516affe4d28cb7fa220a/testing/mozbase/mozdevice/mozdevice/adb.py#3095

Comment 8

[Chris Peterson [:cpeterson]]

James says most of the browser.tabs functionality could/should be handled by WebExtensions' own content scripts.

Comment 9

[[ex-Mozilla] Agi Sferro | :agi]

Mass moving bugs to the Extension component.