Closed Bug 2063456 Opened 10 days ago Closed 8 days ago

newtab trainhop CI jobs on Beta 155 aren't installing the 156 XPI

Categories

(Firefox :: New Tab Page, defect)

Product:
Firefox
Component:
New Tab Page
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
156 Branch
Tracking Flags:
Tracking Status
firefox155 + fixed
firefox156 --- fixed

People

(Reporter: mconley, Assigned: mconley, NeedInfo)

References

Details

Attachments

(4 files, 1 obsolete file)

Bug 2063456 - Part 1: Make the "any" sentinel value work for AboutNewTabResourceMapping again. r=#home-newtab-reviewers!
48 bytes, text/x-phabricator-request
Details | Review
Bug 2063456 - Part 2: Update trainhop test jobs to add the new preference for the co-enrolling mode. r?ahal!
48 bytes, text/x-phabricator-request
Details | Review
Bug 2063456 - Part 1: Make the "any" sentinel value work for AboutNewTabResourceMapping again.
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-beta+
Details | Review
Bug 2063456 - Part 2: Update trainhop test jobs to add the new preference for the co-enrolling mode.
48 bytes, text/x-phabricator-request
phab-bot
: approval-mozilla-beta+
Details | Review

Check out Maxx's try push here: https://treeherder.mozilla.org/jobs?repo=try&revision=15b25f404d4e45adae30210aebeb7d13975108a7&selectedTaskRun=aHC0ujdERs-Sada0uQBslw.0

He's orange-ing on the Beta variants of these tests. Looking through the log, I see:

[task 2026-08-13T21:17:04.348+00:00] 21:17:04     INFO - GECKO(2206) | console.log: AboutNewTabResourceMapping: "Mapping newtab resources from built-in add-on version 155.2.0 on application version 155.0b1"

So it looks like the XPI isn't being properly installed. It's possible this is fallout from bug 1995391, and the changes to AboutNewTabResourceMapping are making the XPI install go awry. We should prioritize investigating and fixing this as soon as possible.

Triage (static analysis only — not built or run):

Root cause. The trainhop variants install the XPI the traditional way and set the sentinel browser.newtabpage.trainhopAddon.version=any only so the value is non-empty (taskcluster/test_configs/variants.yml and #160). Bug 1995391 (changeset 5ac4db2dbec0) added a third uninstall disjunct to getPreferredMapping, Services.vc.compare(version, entitledVersion) > 0, with entitledVersion = max(trainhopAddon.version, trainhopAddonDeployment.version) — see browser/components/newtab/AboutNewTabResourceMapping.sys.mjs. ParseVP in xpcom/base/nsVersionComparator.cpp strtols "any" to numA = 0, so compare("156.x", "any") === 1: the 156 XPI looks like a downgrade relative to the entitlement, shouldUninstallXPI becomes true and we fall back to the built-in — which is exactly the logged Mapping newtab resources from built-in add-on version 155.2.0 on application version 155.0b1.

This is Beta-only because 1995391 landed 2026-08-01, during the 155 nightly cycle: Beta 155 has the new disjunct, Release 154 does not (so trainhop-release is still green, and will break when 155 ships to release).

Proposed fix. The application under test comes from the beta/release index while the harness and task config come from the push, so fixing this CI-side works today without an uplift:

  1. In taskcluster/test_configs/variants.yml (both trainhop-release and trainhop-beta), replace =any with a value that actually parses as a version and sorts above any real train-hop version, e.g. browser.newtabpage.trainhopAddon.version=999999.0, and update the explanatory comment: the value must now also be ≥ the installed XPI's version, not merely non-empty. This stays compatible with older builds, which only check for non-empty.
  2. The "is this a trainhop job" detection hardcodes the whole browser.newtabpage.trainhopAddon.version=any string in two places — testing/mochitest/runtests.py and #2321 — so it must be relaxed to match the pref name prefix, otherwise changing the value silently drops the tests/bin LD_LIBRARY_PATH and crash-reporter handling.

Optional hardening in product code (needs a 155 beta uplift to help these jobs, so not sufficient on its own): scope the new downgrade disjunct to the pref the front-end actually owns and that can decrease, i.e. apply it only when browser.newtabpage.trainhopAddonDeployment.version is non-empty, so a setPref-owned/sentinel value can never veto an already-installed XPI.

No existing automated test covers the sentinel path; the closest anchors are the xpcshell trainhop tests in browser/components/newtab/test/xpcshell/. A regression test for getPreferredMapping with a non-version entitlement value would be worth adding.

Severity suggestion: S3 — no end-user impact, but the jobs that verify the mechanism we use to ship newtab to release are silently passing through the built-in instead of the train-hopped XPI.

If you'd like to provide feedback on this comment, please use the 👍 or 👎 reaction.
If you want to categorize your feedback you can add one of the following tags: ai-triage-wrong-file, ai-triage-wrong-cause, ai-triage-hallucination, ai-triage-out-of-scope.

The trainhop-beta jobs fall back to the built-in newtab instead of the installed XPI, as seen in the log:

AboutNewTabResourceMapping: "Mapping newtab resources from built-in add-on version 155.2.0 on application version 155.0b1"

Root cause. The trainhop variants install the XPI with the traditional extension install mechanism and set browser.newtabpage.trainhopAddon.version=any, relying on the old contract that the value only had to be non-empty. Bug 1995391 (5ac4db2dbec0, 2026-08-01, fixed in 155) turned that pref into the version the client is entitled to and added a third disjunct to shouldUninstallXPI in getPreferredMapping:

Services.vc.compare(version, entitledVersion) > 0

nsVersionComparator parses each dot-part as <int><string><int><string>, so "any" has leading integer 0. compare("156.x", "any") === 1, the installed XPI looks like a downgrade, and we map the built-in instead. It is worse than just "not used": with _addonIsXPI false and no Nimbus enrollment, updateTrainhopAddonState then takes the addon_version === null && xpi_download_path === null branch and actively uninstalls the XPI.

Beta-only because 1995391 rode 155: Beta has the new disjunct, Release 154 does not, so trainhop-release is still green and would break when 155 ships to release.

Fixed CI-side rather than in product code: the application under test comes from the beta/release index while the harness and task config come from the push, so this takes effect immediately with no 155 uplift. The product's strict version semantics are correct for production (Nimbus only ever writes real versions into these prefs), so rather than teach it to accept garbage as "entitled to anything", the sentinel is now a real version.

Changes:

  1. taskcluster/test_configs/variants.yml (both trainhop-release and trainhop-beta): use =999999.0, which parses as a version and sorts above any real train-hop version. Comment updated to record that the value must now also sort >= the installed XPI's version, not merely be non-empty. Compatible with older builds, which only check for non-empty.
  2. testing/mochitest/runtests.py: the "is this a train-hop job" check hardcoded the whole ...version=any string in two places, so changing the value would have silently dropped the tests/bin LD_LIBRARY_PATH and the train-hop http server handling. Both now go through one isTrainHopJob() helper that matches the pref name, so the value can change again without breaking the harness.
  3. docs/v2-system-addon/train_hopping.md: the manual "reproduce a train-hop failure locally" steps told developers to set the pref to any, which has the same silent-fallback outcome on 155+.
  4. New test_AboutNewTabResourceMapping_entitledVersion.js: pins the contract these hand-set callers depend on. There was no coverage of a hand-set entitlement value (the Nimbus tests drive the prefs through enrollments and cannot produce a non-version value), which is why 1995391 landed green and only broke on Beta. Covers the sentinel through either pref, an exact-version match, a genuine downgrade, and the non-version "any" case that caused this bug.

Verification. The comparator results this hinges on were confirmed by running the ParseVP/CompareVP algorithm out of nsVersionComparator.cpp as a standalone program: compare("156.0","any") = 1 (the bug) and compare("156.0","999999.0") = -1 (the fix). isTrainHopJob was unit-checked against the old and new values, whitespace, empty/None, and the neighbouring trainhopAddonDeployment.version pref (correctly not matched). variants.yml was re-parsed to confirm both variants carry the new value and no =any remains.

Note: no build was available in this environment, so the new xpcshell test has not been executed — it was verified by hand-tracing getPreferredMapping for all five cases. Please confirm with a ./mach try --preset desktop-newtab-trainhop push, which is also the only way to verify the trainhop-beta job end to end.

Duplicate of this bug: 2063499

I think I'm going to go with a different solution - I'm going to try to produce a patch that lets both developers and our test framework still use "any" as the overriding pref value when installing an XPI.

Assignee: nobody → mconley

[Tracking Requested - why for this release]:

We'll need a fix for this uplifted in order to clear up the newtab trainhop CI jobs. Effectively, we need to make it so that 155+ on Beta and Release can install the newtab.xpi's from CI.

tracking-firefox155: --- → ?
Attachment #9626544 - Attachment is obsolete: true
Pushed by mconley@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/8d53b503b6b0 https://hg.mozilla.org/integration/autoland/rev/3ed685cf0b97 Part 1: Make the "any" sentinel value work for AboutNewTabResourceMapping again. r=jbrown https://github.com/mozilla-firefox/firefox/commit/e29e646b0a37 https://hg.mozilla.org/integration/autoland/rev/539f313f51af Part 2: Update trainhop test jobs to add the new preference for the co-enrolling mode. r=ahal

Note: this will not clear the oranges after landing / merging / closing, but only after being uplifted to beta, which I will request after this closes.

https://hg.mozilla.org/mozilla-central/rev/3ed685cf0b97
https://hg.mozilla.org/mozilla-central/rev/539f313f51af

Status: NEW → RESOLVED
Closed: 8 days ago
status-firefox156: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 156 Branch

This is also failing on Mochitest-browser for Newtab using latest Beta on autoland and central.
should we file another bug for it ?

Flags: needinfo?(mconley)

No - we should get this uplifted to beta, which should fix this when the next beta build occurs (see comment 10).

I'll request uplift today.

Attachment #9626965 - Flags: approval-mozilla-beta?

firefox-beta Uplift Approval Request

  • User impact if declined/Reason for urgency: None, but we lose the ability to test newtab train hops against Beta with newtab browser mochitests on autoland and main.
  • Code covered by automated testing?: yes
  • Fix verified in Nightly?: yes
  • Needs manual QE testing?: no
  • Steps to reproduce for manual QE testing:
  • Risk associated with taking this patch: low
  • Explanation of risk level: This adds back the ability for CI to install the newtab XPI.
  • String changes made/needed?: None
  • Is Android affected?: no
Attachment #9626966 - Flags: approval-mozilla-beta?
Flags: in-testsuite+
Attachment #9626965 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9626966 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: