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.