1822735 - Event page should be awakened at extension startup when the persisted set of listeners is potentially incomplete

Status: NEW

(WebExtensions :: General, defect, P2)

Description

[Rob Wu [:robwu]]

The current mechanism of persistent event listeners has a number of issues in correctly determining whether the set of persistent listeners is accurate.

This is centered around the intent "If there are no listeners for the extension that were persisted, we need to start the event page so they can be registered." introduced in bug 1748524. It is implemented by the check !extension.persistentListeners?.size, which does not always accurately reflect the intended effect.

One case where this check fails is in event pages without any event listeners. Such event pages will always be awakened (even though Chrome does not). Wakening up more often than needed is not ideal but not the point of this bug report (but my suggestion below will fix this too).

The more serious issue is that the presence of any persistent listener prevents the event page from starting until any primed event is triggered. This is problematic in the following situations:

• When a persistent listener fails to register, its registration is dropped. If there is one (uncommon) listener that passes the check, then there are fewer primed listeners than intended, and the event page may not wake up when expected.
  • A concrete example is when the stored format is incompatible (e.g. the proposal in https://phabricator.services.mozilla.com/D171769?id=691881 triggers this situation through partial registration errors).
• event page starts and registers a persistent listener, but gets interrupted before completion (ranging from extension/runtime errors to browser errors/shutdown/crashes). If the only registered listener(s) are incomplete and rare events, then the check mentioned above will prevent the event page from starting, and the lack of triggered events will prevent the event page from ever reviving.

To fix this, we should:

• make sure that we separately track successful load of the event page (to determine whether all relevant listeners have been registered)
• use this flag as the signal to skip event page loads (instead of the current check).
• clear/ignore this flag whenever we are uncertain whether the stored data is complete. E.g. in extension updates (side note: we already force the event page to wake up after an extension update, since bug 1757855). Or e.g. in browser updates when we discover a bug that results in a non-working persistent listener (including when persistent listeners had not been implemented before - bug 1815310). Or even unconditionally on every browser update.

Given the requirement of resetting this flag in extension updates and potentially after browser updates, it could make sense to store this flag in StartupCache and use that instead of the current check.

Note: even with the suggested flag, there is still value to having the primed listeners, so that the already-primed listeners can already start capturing+suspending events while the background is loading (this is the original use case that bug 1447551 intended to serve).

( If we don't care about the loss of events between the extension startup and the (forced) event page wakeup, then we could also store everything in StartupCache instead of addonStartup.json.lz4. I think that we should care though, at least for webRequest events. )

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Request Handling' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Chris Peterson [:cpeterson]]

Rob, do we need to fix this event page bug for Android addons?

Comment 3

[Rob Wu [:robwu]]

(In reply to Chris Peterson [:cpeterson] from comment #2)

Rob, do we need to fix this event page bug for Android addons?

Yes. Not a drop-everything-fix-this priority, but very desirable to improve the reliability of event pages.