1555094 - GeckoView: browser.tabs.onUpdated support

Status: RESOLVED INACTIVE

(GeckoView :: Extensions, defect, P3)

Description

[Krzysztof Jan Modras [:chrmod]]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

On top of those changes: https://phabricator.services.mozilla.com/D32871

Steps to reproduce:

• start geckoview_example
• connect remote debugger
• switch context to dummy addon background
• assign tabs listener browser.tabs.onUpdated.addListener(() => {})
• create new tab browser.tabs.create({ url: 'https://mozilla.org' })

Actual results:

Following error is thrown

this.window.BrowserApp is undefined ext-utils.js:105
ProgressListenerWrapper chrome://geckoview/content/ext-utils.js:105
addProgressListener chrome://geckoview/content/ext-utils.js:196
_addWindowListener chrome://extensions/content/parent/ext-tabs-base.js:1731
_handleWindowOpened chrome://extensions/content/parent/ext-tabs-base.js:1748
_handleWindowOpened self-hosted:1003
handleEvent chrome://extensions/content/parent/ext-tabs-base.js:1592
create chrome://geckoview/content/ext-tabs.js:265
AsyncFunctionNext self-hosted:839

Expected results:

Problems is the window object passed from Services.ww is not accessible in extension partent context.

Comment 1

[Krzysztof Jan Modras [:chrmod]]

I was wrong about window object not being accessible, it is accessible, just BrowserApp is not there yet.

Have checked desktop implementation of addProgressListener and they use gBrowser which on GeckoView is the same as BrowserApp - but somehow on destkop this object is already initialized.
So I see two way forward:

1. make sure that all GeckoView modules get instantiated before window load event fires - that is how I believe desktop works
2. wait for BrowserApp to be available

Any suggestions?

Comment 2

[Krzysztof Jan Modras [:chrmod]]

BTW. I've verified that option 2. works, but I don't like as I've used Services.tm.spinEventLoopUntil(() => window.BrowserApp);

Comment 3

[Rob Wu [:robwu]]

I was wrong about window object not being accessible, it is accessible, just BrowserApp is not there yet.

I already suspected this when I suggested the following in #mobile on IRC:

One of my thoughts is to check whether the following code runs before you try to use it: https://searchfox.org/mozilla-central/rev/7556a400affa9eb99e522d2d17c40689fa23a729/mobile/android/modules/geckoview/GeckoViewTab.jsm#31

I think that the preferred approach is not try hard and emulate BrowserApp in GeckoView, but to separate the implementation of GeckoView and Fennec, so that GeckoView does not need to emulate parts of Fennec. I'll bring this up with my team today, and if there are no objections I will open a bug and submit patches.

Comment 4

[Chris Peterson [:cpeterson]]

Linking to meta bug 1543755 for the browser.tabs API

Comment 5

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

As of Bug 1553515 we should be loading all the modules before the chrome window has finished loading, so hopefully we don't have a problem here now?

Comment 6

[Krzysztof Jan Modras [:chrmod]]

@snorp this revision enables tabs.onUpdated and covers it with tests quite well https://phabricator.services.mozilla.com/D36575

Comment 7

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

Mass moving bugs to the Extension component.

Comment 10

[Rob Wu [:robwu]]

I'm closing this bug because the originally reported issue has been fixed, but note that the state of support for tabs.onUpdated on Android is currently very limited, to url, title and status changes. Filters are not supported (bug 1713819). There are many fields with limited support (bug 2032928), once implemented we should implement onUpdated for that field as needed (e.g. here is a bug for discarded: bug 1865366).