1754866 - fire.sync description in the in-tree doc page doesn't seem to match how it is actually used in the API implementation

Status: NEW

(WebExtensions :: General, task, P3)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

In the section of the "WebExtensions API development" in-tree doc page section "Implementing an event in the child process", fire.sync is being described as follows:

Events implemented in a child process have some additional methods available to dispatch listeners:

• fire.sync() This runs the listener synchronously and returns the value returned by the listener

This description makes the reader to assume that fire.sync() would only be used in child side API scripts (also given that the listener would be potentially running in a different process and so unable to synchronously return the value returned by the listener if called from the parent process).

Nonetheless, that doesn't seem to really match where it is actually being used in the actual API scripts, there are quick a number of calls that seems to be in the API scripts that are expected to only be running in the parent process:

• https://searchfox.org/mozilla-central/search?q=fire.sync&path=components%2Fextensions%2F

We'd like to double-check if there is any reason why API scripts running in the parent process are using fire.sync or if it was just overlooked, and maker sure that the documentation is in sync with how that method is used in the actual APIs implementation.

Comment 1

[Rob Wu [:robwu]]

In the parent process, it does generally not matter whether fire.sync or fire.async is used, because the event propagation to the child is asynchronous. Because of this, they are interchangeable in the parent from the functionality POV.
A minimal advantage of fire.sync over fire.async is that it's immediate instead of queued at the next microtask. Minimal advantages of fire.async over fire.sync is that it is obvious that the caller doesn't need to account for synchronous handling of the event, and the deferral of any work (e.g. IPC to the child).
It doesn't really matter when the two are used from the parent, but if relative timing between two events are important, then it would be useful to be consistent within the implementation of one API namespace.

Some history:
When the WebExtension framework was implemented, extensions initially ran in the main process. Event dispatches were mostly synchronous in practice, until Firefox 52 (patch for most APIs, remaining webNavigation/webRequest APIs in bug 1305217). An example of a consequence of this sync->async change is that the ability to block data:-URLs in webRequest broke - this would not be possible in the first place if the implementation was forcidbly async since its inception.

When the fire.async API was introduced in Firefox 54 (patch), there was already a mixt of fire.sync and fire.async uses in the parent, which also highlights that there is not necessarily a distinction between the two, according from the following excerpt from the commit message:

Note that some of them were previously synchronous, but it didn't appear to be necessary.