Proposal: Toggleable event listeners

[newRoland]

There's no good way to make event listeners optional for the background (service worker or event page).

An extension might have an optional feature that requires listening to chatty events. Always having these event listeners active is not ideal, especially when not all users enable that feature. Listening to chatty events impact performance by waking the background frequently or deprive it from rest. Currently, there's no standard method to make event listeners optional. The usual approach of attaching and detaching them is impractical for the background page.

The current workaround is a hacky process that needs to be done every time the background loads.

// on background page start, add the listener. 
browser.webNavigation.onCommitted.addListener(onListener)

// Check if the user has enabled the feature, and if not, remove the listener. 
browser.storage.local.get("someFeature").then(({someFeature}) => {
    update(someFeature) 
})

// Since the feature state can change while the background is on, need to also listen to changes
browser.storage.local.onChanged.addListener(changes => {
    if (changes.someFeature) {
         update(changes.someFeature.newValue) 
    }
})

function update(someFeature) {
     if (someFeature) {
          webNavigation.onCommitted.addListener(onListener)
     } else {
          webNavigation.onCommitted.removeListener(onListener)
     }
}

Proposal

If requiresFlag was specified, the browser will ensure that flag is true before dispatching to the listener.

browser.webNavigation.onCommitted.addListener({
    requiresFlag: "someFlag",
    callback: onListener
})

// In options page, if user enables a feature that needs it. 
browser.runtime.setListenerFlag("someFlag", true) 

// Or to disable 
browser.runtime.setListenerFlag("someFlag", false) 

[bershanskiy]

We had discussed the "chattiness" of some APIs a few times already, and so far did not come to a consensus. This specific use case, however, does have a decent workaround: the background context listens only for sync storage changes but not local changes, and the content script saves changes to sync and/or local storage, and explicitly sends a message to background context for all local changes which are relevant to the background.

Overall:

1. Background context registers event listeners for extension update, browser startup (session start), sync storage update (to get notified of remote setting changes). Background does not listen to local changes at all.
2. Content scripts, popups, options pages, DevTools pages read from/save to any storage, but whenever writing to local something relevant to background, they also send a message to background context with the change.

[newRoland]

That workaround seems to only be relevant if the chatty events are storage changes. I'm more interested in a declarative way of adding listeners so they can be disabled or enabled dynamically.

#475 This proposal seems to fix that issue you're referencing.

[tophf]

AFAICT there's no need to change addListener's signature because it already accepts an object in the second parameter for various options, so it's probably easier to add a new property there e.g. wakeUp boolean, true when invoked in the first event loop task, false otherwise, to keep the current behavior intact. As for removeListener, it should always remove the listener from the wake up registry without any options for simplicity.

[newRoland]

wakeUp boolean, true when invoked in the first event loop task, false otherwise, to keep the current behavior intact.

I'm having some difficulty understanding this suggestion. Would it allow for enabling/disabling a listener based on the user enabling/disabling a feature?

...already accepts an object in the second parameter

Only certain events (webRequests.on*, windows.on*, webNavigation.on*) seem to accept a second parameter and they all expect a different type of filter. I think documentation and implementation will be much simpler if the first parameter was used.

Proposed shape of add listener.

interface AddListenerInit<H> {
    callback: H,
    group?: string  
}

Event.addListener(callback: H | AddListenerInit<H>)

[tophf]

Would it allow for enabling/disabling a listener based on the user enabling/disabling a feature?

Yes, of course you can call chrome.foo.onBar.addListener(fn, {wakeUp: true}) at any time to register a listener that wakes the background script, and removeListener to unregister it.

Only certain events (webRequests.on*, windows.on*, webNavigation.on*) seem to accept a second parameter
I think documentation and implementation will be much simpler if the first parameter was used.

The second parameter already exists on events that allow modifying the listener's behavior, which is exactly what is being discussed, hence extending it to other events seems straightforward and intuitive. Conversely, splitting the behavioral options in two different parameters is counter-intuitive.

---

Another solution might be to use .addWakeUpListener and .removeWakeUpListener.

[newRoland]

Yes, of course you can call chrome.foo.onBar.addListener(fn, {wakeUp: true}) at any time to register a listener that wakes the background script, and removeListener to unregister it.

I'm still having trouble understanding, which might be a sign that it's not very intuitive. In the background service worker, all events need to be registered synchronously. So the concept of adding the listener at any time isn't clicking with me.

The second parameter already exists on events that allow modifying the listener's behavior, which is exactly what is being discussed, hence extending it to other events seems straightforward and intuitive. Conversely, splitting the behavioral options in two different parameters is counter-intuitive.

Second parameter is for filters specific to that event. The first parameter would be an init that's applicable to all events. I personally think it's more suitable for the first parameter, but I would be ok with both options.

[tophf]

I'm still having trouble understanding, which might be a good indicator it's not very friendly.

Usually it's an indicator of a bias/preconception.

In the background service worker, all events need to be registered synchronously.

This requirement has been always terribly counter-intuitive due to the implicit magical behavior of having the listener being registered in the first turn of the event loop, it's also regularly misunderstood by developers as it's hard to describe (the documentation even incorrectly stated for 10+ years that the listeners must be declared at the top level of the script).

I suggest an explicit method of indicating the intent, thus solving both the suggested case and the current implicit counter-intuitive magical voodoo system.

[newRoland]

Could you give a usage example, similar to my proposal's example?

browser.webNavigation.onCommitted.addListener({
    group: "someGroupName",
    callback: onListener
})

// In options page
// if user enables or disables someFeature, you can enable or disable the intensive listeners.  
browser.runtime.disableListenerGroup("someGroupName")

[tophf]

Oh. I didn't account for your suggestion to control the behavior from another page, so my suggestion was assuming you send a message to the service worker, which will toggle listener registration accordingly, just like we do currently with the only difference of using an explicit wakeUp parameter in the options. The SW can use runtime.onMessage or runtime.onConnect or self.onmessage + navigator.serviceWorker.postMessage.

To incorporate your suggestion, my idea may be modified to add wakeUpId: 'foo123' instead of wakeUp boolean:

• chrome.webNavigation.onCommitted.addListener(fn, {wakeUpId: 'nav1', /* ...other options */}) in the background script at any time including asynchronously, i.e. not just in the first turn of the event loop
• chrome.webNavigation.onCommitted.removeListenerId('nav1') in any extension context.

[newRoland]

Let's say it's through a message, will it look like this?

browser.onMessage.addListener(msg => {
    if (msg.action === 'activateListener') {
        browser.webNavigation.onCommitted.addListener(onListener, {wakeUp: true})
    }
})

Event listeners need to be added each iteration of the service worker, but we're only adding it when we receive a message. Is the wakeUp parameter a way to indicate the you want to add that listener in perpetuity? If so, maybe sticky: true is better name.

If that's the case, it seems like a good proposal, but I think it would be difficult to implement. My suggestion is pretty easy all things considered. All the browser has to do is ensure a listener's group isn't disabled before dispatching to the listener.

[tophf]

Indeed, wakeUp and late registration either won't work or won't be easy to implement. My revised suggestion ended up being a cosmetic alternative to yours: use wakeUpId: 'string' or just id in the options object as shown in my previous comment.

[hanguokai]

Currently there is no way to register listeners dynamically in the background. Your workaround should work (but has a race condition).

Anyway (no matter what the API looks like), to support dynamic listeners in the background, the current implementation logic must be modified (a lot). At present, the browser doesn't remember what listeners to trigger, it fires events to all listeners that are registered at the first event loop when the service worker wakes up.

[tophf]

The dynamically registered listeners idea was my wrong initial suggestion, which I finally corrected. The actual idea is that all listeners are registered synchronously with an id specified via group in the first parameter (original suggestion) or wakeUpId or id in the second parameter (my suggestion). Toggling occurs at a different time, usually in a different place such as the popup or the options dialog, but it can also happen in the background script - it's fine because it doesn't influence the initial behavior of the already running background script, i.e. it's only for the subsequent events.

Hence, there should be no race conditions because a) the browser won't wake up the background script for a disabled id, b) it won't send the event to an already running script, c) the list of enabled ids will be known at the start of the script as it'll be passed in the internal message that contains the data for the event that woke the script. Of course, it's possible to introduce raciness, as the API to disable an id is asynchronous, but it's not specific to this case.

BTW, it means we should add a way to re-enable the id/group e.g. chrome.runtime.enableListenerGroup or chrome.webNavigation.onCommitted.enableListenerId.

[newRoland]

I added an alternative proposal so the interface accepts key for local storage. If specified, the browser will ensure that key is set to true before dispatching to the listener.

Update: I ended up removing it because there's no precedence for that kind of thing.

[fregante]

I don’t think this can be dependent on random storage keys and their format. What storage area should this be read from? Local? Sync? Managed?

They should probably have their own grouping/namespace instead:

browser.webNavigation.onCommitted.addListener({
    group: 'my-events',
    callback: onListener
});

browser.webNavigation.onCommitted.toggle('my-events', true)

And then you can add your own storage.onChanged listener if you want to link it to a specific group:

browser.storage.local.onChanged.addListener(changes => {
    if (changes.someFeature) {
         browser.webNavigation.onCommitted.toggle('my-events', changes.someFeature.newValue) 
    }
})

This probably isn't as clean as you hoped though.

[newRoland]

If there's only one, local 100%. Sync is complicated. Session doesn't allow you set to set a default value and requires initialization.

You could also have an option for both session and local.

interface AddListenerInit<H> {
    callback: H,
    requiresLocalFlag?: string  
    requiresSessionFlag?: string  
}

Event.addListener(callback: H | AddListenerInit<H>)

They should probably have their own grouping/namespace instead:

I agree, but I think the alternative proposal might be simpler to implement. I will include both.

Update: Ended up removing it.

[fregante]

If there's only one, local 100%. Sync is complicated. Session doesn't allow you set to set a default value and requires initialization.

Options are often in the sync storage, so if I want to make this dependent on an option, it's impossible. Also it's impossible when the options are stored as a single complex object.

What you're asking is to overload storage.local with behavior related to other APIs.

I like your browser.runtime.updateListenerGroup suggestion

[hanguokai]

At present, an extension Event object has these method:

addListener()
removeListener()
hasListener()
hasListeners()

addRules()
getRules()
removeRules()

For dynamically registered events for service worker, I proposal these new methods instead of changing existing methods:

subscribe(function-name: String)
unsubscribe(function-name: String)
hasSubscribed(function-name: String)

For example:

browser.webNavigation.onCommitted.subscribe("myFunctionName");
browser.webNavigation.onCommitted.unsubscribe("myFunctionName");

"FunctionName" is a global function name that declared in service worker, rather than a function object, and doesn't need to be registered in the first event loop when service worker wakes up. This allows the browser to remember both the event and this specific function independently of service worker's lifecycle, not just remember the event like addListener() does.

These new methods can be called in both service-worker context and non-service-worker context dynamically, but only trigger events in service worker context, not in other contexts.

After calling event.subscribe(function-name), the browser triggers related events in service worker. If the service worker is inactive, wake up it first. Then looks for the function to execute.

I'd love to hear from the browser implementation perspective, such as whether it's possible or what problems it will encounter.

[fregante]

@hanguokai That sounds like an entirely new proposal

[newRoland]

@hanguokai Interesting approach, but there might be a few issues involving bundlers (webpack, vite, etc). You can work around these, but it might stump some people.

1. Bundlers encapsulate the top level into a function so defining a global function will require using globalThis.functionName = func syntax.
2. Tree shaking: bundlers and optimizers might remove the function because it might appear it's not being used.
3. Bundlers and optimizers often minimize function names so that might also have an impact.

[hanguokai]

@fregante @newRoland
At present, this feature is in the early stages of discussion, and we first need to determine the feasibility of the feature (i.e. whether it can be implemented) and whether browser vendors are willing to support it (in whatever form). Then come the specific API and other peripheral issues.

I am not a browser engineer. So I also raised this issue to the Service Worker community yesterday, asking if they think it can be implemented w3c/ServiceWorker#1698 . If the function can be implemented, it will probably take several years before we can use it.

[newRoland]

@hanguokai

it will probably take several years before we can use it.

Wow, that long? Service worker proposals probably take much longer than a purely web extension proposal, so I want to clarify that my proposal doesn't require changing the service worker spec.

Terminology
Event Dispatcher: The code responsible for dispatching web extension events.

My proposal is to register an event listener with a group name. The group name is like a flag. The Event Dispatcher will check if that group name is enabled before dispatching to that listener. This proposal applies to event.addListener in all contexts (content script, options page, background ,etc).

[hanguokai]

Wow, that long?

The time I'm talking about is for adding a new extension behavior or API like this, especially the proposals coming from developers rather than browser vendors. I also don't want to change Web service worker standard, just asking for advice there.

My proposal is ……

Thanks for the explanation. So your proposal still needs to add event listeners in the first event loop in service worker, not for dynamically adding event listeners. What is the default state (enable or disable) when addListener() with a group name if developer doesn't set the state before?

[newRoland]

🤔 Haven't really thought about it, but off by default seems like a better choice. On a related note, I'm souring on my naming choices. Group sounds too generic so I'll switch to requiresFlag.

browser.webNavigation.onCommitted.addListener({
    requiresFlag: "someFlag",
    callback: onListener
})

// In options page, enable/disable on demand. 
browser.runtime.setListenerFlag("someFlag", true)