persistent listeners are not handled correctly in upgrades
Categories
(WebExtensions :: General, enhancement, P1)
Tracking
(firefox100 fixed)
| Tracking | Status | |
|---|---|---|
| firefox100 | --- | fixed |
People
(Reporter: mixedpuppy, Assigned: mixedpuppy)
References
Details
(Whiteboard: [addons-jira])
Attachments
(1 file)
from Rob:
- Non-blocking webRequest listeners are not removed from the lz4 file if not registered again.
- Persistent listeners are persisted across updates. The logic that determines whether an event page should wake up skips that if there are persistent listeners. This is especially a problem for updates, where the set of listeners may change due to source code changes
The second situation also required the fix for the first situation so they are both included in this.
|
||
Updated•2 years ago
|
|
Assignee | |
Comment 1•2 years ago
|
||
|
||
Comment 2•2 years ago
|
||
1 is a regression of bug 1749871, but I'll just mark as see also because the second one is not directly related to it.
Issue 1 causes redundant listeners to accumulate over time, which on its own is not really nice but otherwise not a very significant issue. However, combined with issue 2, it would prevent extensions with event pages from ever updating its set of listeners when updated (unless there was another trigger for event page startup, other than https://searchfox.org/mozilla-central/rev/ad7ecfa618ec3a65db8405d9f1125059fe4a6a15/toolkit/components/extensions/parent/ext-backgroundPage.js#378-385).
The EventManager.clearPersistentListeners that are scattered all over the place don't actually clean up the listeners, for the reason explained in bug 1755589. Specifically the "final clean-up (step 4) doesn't clear events when listener.primed is void." part. The latest code is at https://searchfox.org/mozilla-central/rev/ad7ecfa618ec3a65db8405d9f1125059fe4a6a15/toolkit/components/extensions/ExtensionCommon.jsm#2464,2466, which has "// When a new listener has beed added, primed is undefined.". This comment is not fully accurate: it could also be that the module is marked as startupBlocking and that the extensionApi.primeListener method hasn't returned anything (or threw).
If startupBlocking listeners are not primed after "early startup" (i.e. no EventManager.primeListeners call), then there is nothing that would ever clean up the listeners.
Pushed by scaraveo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d705f8e9b4ee fix background startup and persistent listeners during addon upgrades r=robwu
|
||
Comment 4•2 years ago
•
|
||
Backed out for multiple failures on android
Failure logs:
- mochitest: https://treeherder.mozilla.org/logviewer?job_id=370330711&repo=autoland&lineNumber=2965
- xpcshell: https://treeherder.mozilla.org/logviewer?job_id=370326413&repo=autoland&lineNumber=5768
Backout: https://hg.mozilla.org/integration/autoland/rev/c0bb796904aa3361c306f52f8446637cd74e81a5
Pushed by scaraveo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c295e6801d39 fix background startup and persistent listeners during addon upgrades r=robwu
|
||
Comment 6•2 years ago
|
||
Backed out for causing failures at RemoteProcessMonitor.
Backout link: https://hg.mozilla.org/integration/autoland/rev/366227a706e2bdf43414250784a00a2ee8d2c039
Failure log: https://treeherder.mozilla.org/logviewer?job_id=370364156&repo=autoland&lineNumber=2451
Pushed by scaraveo@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8ca3b8d52983 fix background startup and persistent listeners during addon upgrades r=robwu
|
||
Comment 8•2 years ago
|
||
| bugherder | ||
|
||
Comment 9•2 years ago
|
||
Clearing needinfo (already handled in the meantime).
Description
•