Expose in extension background service workers WebExtensions API webidl bindings
Categories
(WebExtensions :: General, task, P1)
Tracking
(firefox91 fixed)
Tracking | Status | |
---|---|---|
firefox91 | --- | fixed |
People
(Reporter: rpl, Assigned: rpl)
References
(Blocks 1 open bug)
Details
(Whiteboard: [mv3-m2])
Attachments
(15 files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
The extension Manifest V3's background service worker should have access to the same set of extension APIs currently available to the extensions background page with Manifest V2 (minus some API methods that can't be made available on the worker threads, e.g. browser.extension.getViews, which does return the active window objects related to the other extension window globals that are part of the same extension).
After some discussions with interested parties (in particular with :asuth, for the DOM workers perspective, and :baku, for the WebIDL perspective) we have determined that the webidl bindings are the preferred approach to provide the WebExtensions API namespaces on the worker thread.
Implementation strategy
Given that a lot of the WebExtensions framework is currently implemented in privileged JS code running on the main thread, we agreed to proceed by initially integrating the WebExtensions API WebIDL bindings with the rest of the existing WebExtensions framework and the existing APIs implementation ext-*.js
modules using the following strategy:
-
webextensions webidl stub methods shared by all WebExtensions APIs:
A good amount of the API methods provided by the WebExtensions API webidl bindings will be implemented by stubs methods (shared by all WebExtensions API webidl bindings) that are going to forward the "API requests" (e.g. API methods being called, API event listeners being added or removed, or API properties being accessed) from the worker thread to the main thread: -
JSONSchema-based parameter validations: the API requests received on the main thread will be validated using the WebExtensions API JSONSchema data ([1])
-
creating WebIDL definitions from the existing WebExtensions JSONSchema files: to simplify creating the API namespaces WebIDL definitions from the JSONSchema files we plan to provide an in-tree (python-based) script.
-
API Requests handling: on the main thread an API requests handler will receive the API requests forwarded by the worker thread and (after validating the parameter using the JSONSchema metadata) the API requests to be:
- forwarded to the parent process, when the API is completely implemented in the parent process
- forwarded to the related child/ext-*.js API module, when the API is completely implemented in the child process
-
API - Special cases: there will be a subset of API methods that can't forward their parameters to the main thread (e.g. because they accept parameters that aren't "structured cloneable") and/or they will have to do part of their work on the worker thread , in those cases:
- those API method will not be handled by one of the "shared stub methods", but an actual custom method will be implemented to cover the particular case
- in some cases the same API method may still have to call the "API requests" handler on the main thread to cover part of the work ([2]), and in that case the method should be able to reuse the same set of underlying helpers that the "stub methods" shared by all WebExtensions APIs will be using internally
-
Testing coverage:
- creating a new set of unit tests to explicitly cover the shared stub methods and the underlying "API request forwarding" mechanism
- adapting existing WebExtensions API tests to also run the same set of tests using a background service worker instead of a background page
- creating additional new tests for testing scenario specifically related to the background service worker
[1] the metadata we have in the WebExtensions API JSONSchema files doesn't unfortunately translate 1-1 with the WebIDL definitions, and there are some other out of the tree components that are currently parsing and using them (the addons-linter in particular, which is used to validate the extension when they are submitted to AMO)
[2] e.g. to validate the subset of the parameters that can be structured cloned, or to delegate to the WebExtensions framework running on the main thread of the child or parent process some parts of the work needed (e.g. a good chunk of the browser.tests.assertRejects
method will have to run on the worker thread because some of the parameters can't be structured cloned, but the outcome of the assertion will be forwarded to the API request handler to be sent to the test harness running in the main process).
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
-
Extension API WebIDL to be part of a new dom/extensions-webidl directory
(and all webidl in this directory associated with WebExtensions::General
bugzilla component) -
Extension API C++ implementation in a new toolkit/components/extensions/webidl-api
directory -
Lock Extensions API WebIDL bindings provided to extensions workers global on:
- the preference "extensions.backgroundServiceWorker.enabled" being set to true
- checking explicitly that the worker is an extension service worker declared
in the extension manifest.json file.
-
Changes to WorkerPrivate, WorkerScope.h/.cpp to expose the WebIDL
bindings to the extension service workers (if the service worker has been
detected as the background service worker specified in the manifest),
plus small changes to RemoteWorkerChild.cpp to detect if the worker
is the background service worker (and mark it as so in the WorkerPrivate
instance associated to it)
Assignee | ||
Comment 2•4 years ago
|
||
Define a new WebExtensionStub extended attribute in the WebIDL generator
to be used in the WebExtensions API WebIDL definitions.
Depends on D70372
Assignee | ||
Comment 3•4 years ago
|
||
An ExtensionAPIBase class which provides a collection of shared stub methods and
shared helpers and used as a base class for the actual WebExtensions API webidl classes.
Depends on D84681
Assignee | ||
Comment 4•4 years ago
|
||
WebIDL interface and c++ class to be used by all WebExtension API webidl bindings
to expose the expected WebExtensions API events (e.g. browser.tabs.onUpdated etc.)
Depends on D84682
Assignee | ||
Comment 5•4 years ago
|
||
New XPCOM idl interfaces to define the components used to:
- represent an API request (call to API methods, add/remove API event listener,
get the value of an API property getter) - define the API request handler defined by privileged JS code running on the
main thread
New ExtensionAPIBase shared stub methods and helper methods to create and forward
the API requests from the worker to the main thread and translate the results
got from the API request handler in the value to return or resolve (or the errors
to be raised) to the extension code running on the worker thread.
Depends on D80604
Assignee | ||
Comment 6•4 years ago
|
||
An ExtensionEventListener xpcom interface that wraps an API event callback
and included in the API requests forwarded to the API request handler.
The ExtensionEventListener xpcom interface provides a method to forward
to the worker thread the calls to the wrapped API event callpack originated
from the WebExtensions framework running on the main thread.
Depends on D75311
Assignee | ||
Comment 7•4 years ago
|
||
Depends on D80609
Assignee | ||
Comment 8•4 years ago
|
||
Depends on D80610
Assignee | ||
Comment 9•4 years ago
|
||
The ExtensionPort represents the webidl definitions of the runtime.Port WebExtensions API object.
A webidl dictionary will represent the serializable definition of the port properties, sent from
the API request handler to provide all the internal properties needed to create an instance
of the ExtensionPort webidl interface on the worker thread (which will be returned to the
extension code as the return value of the runtime.connect/connectNative methods and as a
parameter of the calls to the runtime.onConnect API event listeners).
Depends on D84684
Assignee | ||
Comment 10•4 years ago
|
||
Depends on D84685
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D84686
Assignee | ||
Comment 12•4 years ago
|
||
ExtensionMockAPI is a fake WebExtensions API, locked behind a pref and
used in unit tests related to the API requests handling and WebExtensionStub
methods without relying on a specific WebExtension API.
Depends on D84687
Assignee | ||
Comment 13•4 years ago
|
||
- Added a new set of xpcshell-tests in toolkit/components/extensions/test/xpcshell/webidl-api
for unit tests related to the WebIDL bindings API request forwarding
Depends on D99886
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 14•4 years ago
|
||
Depends on D99887
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 15•3 years ago
|
||
Comment 16•3 years ago
|
||
Backed out for mbu failures.
backout: https://hg.mozilla.org/integration/autoland/rev/11f571b7506c574cb869523b017bfe10db1fc612
failure log: https://treeherder.mozilla.org/logviewer?job_id=342295625&repo=autoland&lineNumber=740
[task 2021-06-09T19:42:26.863Z] 0:20.35 ============================= test session starts ==============================
[task 2021-06-09T19:42:26.863Z] 0:20.35 platform linux -- Python 3.6.9, pytest-3.6.2, py-1.10.0, pluggy-0.6.0 -- /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenvs/python-test/bin/python
[task 2021-06-09T19:42:26.864Z] 0:20.35 rootdir: /builds/worker/checkouts/gecko, inifile: /builds/worker/checkouts/gecko/config/mozunit/mozunit/pytest.ini
[task 2021-06-09T19:42:26.865Z] 0:20.35 collecting ... collected 5 items
[task 2021-06-09T19:42:26.865Z] 0:20.35
[task 2021-06-09T19:42:26.866Z] 0:20.35 python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_browser TEST-UNEXPECTED-FAIL
[task 2021-06-09T19:42:26.867Z] 0:20.35 python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_extensions PASSED
[task 2021-06-09T19:42:26.867Z] 0:20.35 python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_js PASSED
[task 2021-06-09T19:42:26.868Z] 0:20.35 python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_memory PASSED
[task 2021-06-09T19:42:26.868Z] 0:20.35 python/mozbuild/mozbuild/test/configure/lint.py::Lint::test_mobile_android TEST-UNEXPECTED-FAIL
[task 2021-06-09T19:42:26.869Z] 0:20.35
Assignee | ||
Comment 17•3 years ago
|
||
ouch... linting error due to a missing '{Enable|Disable}' part in the new config option help message :-(
D104707 just updated accordingly.
Push to try confirms that with that one line change the mbu
job does now pass as expected:
Comment 18•3 years ago
|
||
Comment 19•3 years ago
|
||
Backed out 14 changesets (Bug 1682632) for causing hazard bustages in ExtensionEventManager.cpp
Backout link: https://hg.mozilla.org/integration/autoland/rev/3a8309d8fd8b49e7de3ba7ab89936854ad293385
Push with failures, failure log.
Assignee | ||
Comment 20•3 years ago
|
||
(In reply to Alexandru Michis [:malexandru] from comment #19)
Backed out 14 changesets (Bug 1682632) for causing hazard bustages in ExtensionEventManager.cpp
Backout link: https://hg.mozilla.org/integration/autoland/rev/3a8309d8fd8b49e7de3ba7ab89936854ad293385
Push with failures, failure log.
I'm looking into it (at the moment figuring out how to run that tests locally so that I can work on it more easily).
Assignee | ||
Comment 21•3 years ago
|
||
This change is meant to handle the mach hazards
failure that triggered a backout of this stack of patches,
ExtensionEventManager::AddListener/RemoveListener should use a Rooted object as a key in the GCHashMap
(as the inline comment right above class GCHashMap
is also pointing out explicitly).
The GCHashMap has been added into ExtensionEventManager as part of D80610 ("part2.2: ExtensionEventListener xpcom interface
base implementation"), and so we could as well absorb this patch into D80610 if the approach looks ok from the reviewers
perspective and we prefer that.
(I've also double-checked locally that these changes should not be triggering the mach hazards
failure anymore
and confirmed that all the unit tests part of this stack of patches are completing successfully).
Depends on D104707
Updated•3 years ago
|
Comment 22•3 years ago
|
||
Comment 23•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/433f69c6d2ad
https://hg.mozilla.org/mozilla-central/rev/d15ff67b02ff
https://hg.mozilla.org/mozilla-central/rev/c40eaa3c0050
https://hg.mozilla.org/mozilla-central/rev/6b39a6bc633f
https://hg.mozilla.org/mozilla-central/rev/364fa6fe0b31
https://hg.mozilla.org/mozilla-central/rev/591a2b97be13
https://hg.mozilla.org/mozilla-central/rev/40821b435c18
https://hg.mozilla.org/mozilla-central/rev/de38a1aa514f
https://hg.mozilla.org/mozilla-central/rev/09ab8ace3537
https://hg.mozilla.org/mozilla-central/rev/53fed3c3fa6a
https://hg.mozilla.org/mozilla-central/rev/a502d01a3975
https://hg.mozilla.org/mozilla-central/rev/10cd26277351
https://hg.mozilla.org/mozilla-central/rev/9c836dd75d4c
https://hg.mozilla.org/mozilla-central/rev/00cf51e23d67
https://hg.mozilla.org/mozilla-central/rev/cd87dcdae2f3
Comment 24•3 years ago
|
||
πππππππ
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Description
•