Closed Bug 1539144 Opened 8 months ago Closed 4 months ago

Add support for browser.tabs.create in geckoview browsers

Categories

(GeckoView :: General, enhancement, P3)

Unspecified
Android
enhancement

Tracking

(firefox69 wontfix, firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox69 --- wontfix
firefox70 --- fixed

People

(Reporter: rwood, Assigned: chrmod)

References

(Blocks 3 open bugs)

Details

Attachments

(2 files)

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

Blocks: 1537941

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.

Rob tells me this isn't critical.

Priority: -- → P3
Blocks: 1543755
No longer blocks: webext-geckoview

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

(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/

Assignee: nobody → krzysztof.modras
Blocks: 1507167
Keywords: checkin-needed

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?

Flags: needinfo?(krzysztof.modras)
Keywords: checkin-needed

I've rebased with central - please verify

Flags: needinfo?(krzysztof.modras) → needinfo?(cbrindusan)
Keywords: checkin-needed

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

Keywords: checkin-needed
Flags: needinfo?(cbrindusan)

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)
Flags: needinfo?(krzysztof.modras) → needinfo?(cbrindusan)
Keywords: checkin-needed

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

Keywords: checkin-needed
Flags: needinfo?(cbrindusan)

Verified with ./mach android checkstyle

Pushed by cbrindusan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7dba641f8200
Follow-up checkstyle fix in WebExtensionController.java
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

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

OS: All → Android

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:
Attachment #9068017 - Flags: approval-mozilla-beta?
Attachment #9077368 - Flags: approval-mozilla-beta?
Regressions: 1565410

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

holding the uplift request until all necessary patches are landed

Attachment #9068017 - Flags: approval-mozilla-beta?
Attachment #9077368 - Flags: approval-mozilla-beta?
Blocks: 1562844
Regressions: 1567314
You need to log in before you can comment on or make changes to this bug.