1539144 - Add support for browser.tabs.create in geckoview browsers

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P3)

Description

[Robert Wood [:rwood]]

Are there any plans to support the browser.tabs.create API in geckoview? The Raptor performance framework webext ideally should be creating a new tab on browser/app startup each time. Currently that is not supported (geckoview example app, and reference browser - I haven't tried Fenix), so Raptor uses the same/one tab and uses browser.tabs.update to change the URL. Note that Fennec does support tabs.create.

Trying to use browser.tabs.create with the geckoview example app (or reference browser) results in this error:

03-26 09:13:36.026 5330 5346 E GeckoConsole: [JavaScript Error: "BrowserApp.addTab is not a function" {file: "chrome://geckoview/content/ext-tabs.js" line: 301}]
03-26 09:13:36.026 5330 5346 E GeckoConsole: create@chrome://geckoview/content/ext-tabs.js:301:38
03-26 09:13:36.026 5330 5346 E GeckoConsole: call/result</<@resource://gre/modules/ExtensionParent.jsm:950:49
03-26 09:13:36.026 5330 5346 E GeckoConsole: withPendingBrowser@resource://gre/modules/ExtensionParent.jsm:604:26
03-26 09:13:36.026 5330 5346 E GeckoConsole: call/result<@resource://gre/modules/ExtensionParent.jsm:949:24
03-26 09:13:36.026 5330 5346 E GeckoConsole: withTiming@resource://gre/modules/ExtensionParent.jsm:916:14
03-26 09:13:36.026 5330 5346 E GeckoConsole: call@resource://gre/modules/ExtensionParent.jsm:948:25

Comment 1

[Nick Alexander :nalexander [he/him]]

The core GV proposition is that the consuming App handles tab management. That's not what Marionette (and CDP/RDP expect). That means that there needs to be a layer of communication between GV and the App for tab creation (and window creation, which is -- AFAIK -- a known issue). Just for Marionette, I don't think this is really worth it: it's a lot of plumbing. But pretty soon window creation is going to be required for Fenix (if it's not already in place), and then lots of things get more feasible.

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

Rob tells me this isn't critical.

Comment 3

[Krzysztof Jan Modras [:chrmod]]

Have a patch for this ready. Need to figure out how to commit it in.

Comment 4

[Chris Peterson [:cpeterson]]

(In reply to Krzysztof Jan Modras from comment #3)

Have a patch for this ready. Need to figure out how to commit it in.

Awesome! You will need to upload your patch to Phabricator, Mozilla's current code review system. Here are instructions for logging into Phabricator and configuring Mercurial to push to Phabricator:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

https://phabricator.services.mozilla.com/

Comment 5

[Krzysztof Jan Modras [:chrmod]]

Please review: https://phabricator.services.mozilla.com/D32796

Comment 6

[Krzysztof Jan Modras [:chrmod]]

Attachment: Bug 1539144 - GeckoView extensions support chrome.tabs.create

Comment 7

[Cristian Brindusan [:cbrindusan]]

It seems that the patch could not be landed due to:

"Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
applying /tmp/tmpYfL54k mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md
Hunk #2 FAILED at 345. 1 out of 2 hunks FAILED -- saving rejects to file mobile/android/geckoview/src/main/java/org/mozilla/geckoview/doc-files/CHANGELOG.md.rej abort: patch command failed: exited with status 256"

Krzysztof, can you please take a look?

Comment 8

[Krzysztof Jan Modras [:chrmod]]

I've rebased with central - please verify

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aebbb469f1d5
GeckoView extensions support chrome.tabs.create r=snorp,agi,geckoview-reviewers,robwu,rpl

Comment 10

[Cristian Brindusan [:cbrindusan]]

Backed out changeset aebbb469f1d5 (Bug 1539144) for ESlint failure at background.js:2:1 and checkstyle bustage.

Backout: https://hg.mozilla.org/integration/autoland/rev/fdeb6bc00b0f2ed42777469f10b53b6d79bda4b0

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=aebbb469f1d5ca7f427d44d5b5d0d9324440c0ea&selectedJob=255915876

Failure log
ESlint:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255915876&repo=autoland&lineNumber=303
Checkstyle bustage:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=255915869&repo=autoland&lineNumber=3068

Comment 11

[Krzysztof Jan Modras [:chrmod]]

it was prettier eslint error:

mozilla-unified/mobile/android/geckoview/src/androidTest/assets/web_extensions/tabs/background.js
  2:1  error  Delete `⏎`  prettier/prettier (eslint)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0155f87f9982
GeckoView extensions support chrome.tabs.create r=snorp,agi,geckoview-reviewers,robwu,rpl

Comment 13

[Rob Wu [:robwu]]

Attachment: Bug 1539144 - Follow-up checkstyle fix in WebExtensionController.java

Verified with ./mach android checkstyle

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7dba641f8200
Follow-up checkstyle fix in WebExtensionController.java

Comment 15

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/0155f87f9982
https://hg.mozilla.org/mozilla-central/rev/7dba641f8200

Comment 16

[Chris Peterson [:cpeterson]]

69=wontfix. Fenix doesn't need browser.tabs.create yet, so we don't need to uplift to GV 69 Beta.

Comment 17

[Krzysztof Jan Modras [:chrmod]]

Comment on attachment 9068017 [details]
Bug 1539144 - GeckoView extensions support chrome.tabs.create

Beta/Release Uplift Approval Request

• User impact if declined:
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change fixes broken browser.tabs.create webextension API and it's implementation is not altering any behaviors of GeckoView.
• String changes made/needed:

Comment 18

[Krzysztof Jan Modras [:chrmod]]

Comment on attachment 9068017 [details]
Bug 1539144 - GeckoView extensions support chrome.tabs.create

holding the uplift request until all necessary patches are landed