Make sure app-builtin-addons location is scanned for missing builtin addons early during the application startup in case of missing, corrupted or stale addonStartup.json.lz4 data
Categories
(Toolkit :: Add-ons Manager, defect, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox140 | --- | fixed |
People
(Reporter: rpl, Assigned: rpl)
References
Details
(Whiteboard: [addons-jira])
Attachments
(3 files)
While investigating the issue that the NewTab built-in add-on has hit for a subset of release users in 138 through the logs shared by the reporter in Bug 1963537 comment 7, we have noticed that the NewTab built-in add-on was detected and then installed and enabled as expected but not started up.
During an application startup related to an application version / build id being changed, we do scan all locations early on application startup (location scan originated from the call to XPIProivder.checkForChanges part of the XPIProvider.startup method) and then right after we hit the XPIProvider logic that ensure enabled extensions are started up early in the XPIProvider and application startup.
On subsequent application startups, the call to XPIProivder.checkForChanges part of the XPIProvider.startup method will instead do only scan locations based on the value set on the extensions.startupScanScopes pref, which in Firefox Desktop builds defaults to 0 and translates into not scanning any location (on purpose, to reduce the impact of the work that the XPIProvider startup needs to do on every application startup).
But the add-ons installed in the app-builtin-addons location are built-in addons that provide features integrated into Firefox (e.g. NewTab built-in add-on provides the about:newtab and about:home pages) and in case of a missing, corrupted or stale addonStartup.json.lz4 file the XPIProvider should still make sure that app add-ons expected in the app-builtin-addons location are detected, installed, enabled and started up early in the application startup, independently if the application startup is the one for an application version change or one of the subsequence application startups.
NOTE: behaviors related to the builtins auto-installed in the “app-builtin-addons” XPIProvider location are mainly covered by xpcshell tests (e.g. in particular test_system_builtins.js, and the other test_system_*.js xpcshell tests from the same mozilla-central directory). AddonTestUtils.init is setting the extensions.startupScanScope pref to 15 here and that forces all locations to be scanned at XPIProvider startup, while on Firefox Desktop builds extensions.startupScanScope pref is set to 0 by default and so it does not scan any location by default (unless the application startup belongs to an application version change). New test cases should recreate a missing/corrupted/stale addonStartup.json.lz4 file while also setting extensions.startupScanScope pref to 0 to recreate a more realistic scenario.
|
||
Updated•8 months ago
|
|
Assignee | |
Comment 1•8 months ago
|
||
|
||
Updated•8 months ago
|
|
|
Assignee | |
Comment 2•8 months ago
|
||
|
|
Assignee | |
Comment 3•7 months ago
|
||
|
||
Comment 6•7 months ago
|
||
Backed out for causing mochitests failures in browser_headless_screenshot_3.js.
- Backout link
- Push with failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | browser/components/shell/test/browser_headless_screenshot_3.js | Firefox process should exit with code 0 - Got 11, expected +0
|
|
Assignee | |
Comment 7•7 months ago
|
||
(In reply to Serban Stanca [:SerbanS] from comment #6)
Backed out for causing mochitests failures in browser_headless_screenshot_3.js.
- Backout link
- Push with failures
- Failure Log
- Failure line: TEST-UNEXPECTED-FAIL | browser/components/shell/test/browser_headless_screenshot_3.js | Firefox process should exit with code 0 - Got 11, expected +0
I was able to reproduce the same kind of failure locally and so I spent some time this morning on doing a deeper investigation to pin point what was going on.
The issue seems to be due to the XPIProvider shutdown blocker getting stuck when some extensions are being started up closely enough to when the application shutdown is initiated, by looking more closely into where the Extension startup method was remaining stuck I noticed that while Services.startup.shuttingDown was still false when we are calling the Extension startup method, Services.startup.shuttingDown became true by the time the Extension startup method got to the call to clearCacheForExtensionPrincipal and then that call was never being resolved (resulting on the "XPIProvider shutdown" blocker getting stuck and not completing in the remaining time available to it).
I filed Bug 1967273 to track the underlying issue described right above and pushed to try (restricted to the kind of job that was hitting a failure) here:
- with the patches attached to this bug + Bug 1967273:
- with only the patches attached to this bug (for comparison and confirming that the job consistently hit the failure without Bug 1967273 patch):
|
||
Comment 9•7 months ago
|
||
Backed out for causing Mn failures
Backout link: https://hg.mozilla.org/integration/autoland/rev/ecb40f89c980f2d576b5c40e89b8782067e070a1
|
||
Comment 10•7 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/1fc159d0dbcc
https://hg.mozilla.org/mozilla-central/rev/46bb574ff26f
https://hg.mozilla.org/mozilla-central/rev/68e0ad5fa245
|
|
Assignee | |
Comment 11•7 months ago
|
||
Re-opening, the patch stack attached to this bugzilla issue has been backout along with the child revision from Bug 1967273, the revert commit mention only Bug 1967273 and so that is likely why this bugzilla issue has been mistakenly closed as resolved:
https://hg-edge.mozilla.org/mozilla-central/rev/ecb40f89c980f2d576b5c40e89b8782067e070a1
Revert "Bug 1967273 - Reject clearCacheForExtensionPrincipal when application shutdown is already initiated. r=willdurand" for causing Mn failures.
This reverts commit a56c3cd30be8f09a69e24c6a724d1a836d5fe3c8.
Revert "Bug 1964408 - Add additional test coverage for stale/missing xpistate with system-signed updates installed. r=willdurand"
This reverts commit 6c7851b741c6134df6bd1751d3ccceeac418c277.
Revert "Bug 1964408 - Scan application scopes early on startup if expected app-system-builtins entries are missing from the addonStartup.json.lz4 data. r=willdurand"
This reverts commit 6121e746674b7dd9ef5d695498534483eb78133a.
Revert "Bug 1964408 - Scan application scopes early on startup if app-builtin-addons location is missing from addonStartup.json.lz4 data. r=baku,willdurand"
This reverts commit e799258ba7ad7038139a22281761da76c7079779.
I was able to reproduce the marionette failure locally, which is triggered by a new warning message that one of the patches in this stack is introducing.
Needinfo-ing myself to signal I'm looking into it.
|
||
Updated•7 months ago
|
|
|
Assignee | |
Comment 12•7 months ago
|
||
I've just updated one of the patches in the stack attached to this bugzilla issue with a small tweak (https://phabricator.services.mozilla.com/D248117?vs=1034671&id=1038254#toc) that prevents the marionette failure hit by the previous push to autoland (the failure linked from comment 9).
I'll push the stack to autoland again soon.
|
||
Comment 13•7 months ago
|
||
|
||
Comment 14•7 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/1a0a247d88c6
https://hg.mozilla.org/mozilla-central/rev/d20547891f16
https://hg.mozilla.org/mozilla-central/rev/fa5e673ca88c
Description
•