1875512 - Cleanup Colorways specific logic from XPIProvider and AOM internals

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, task, P3)

Description

[Luca Greco [:rpl] [:luca] [:lgreco]]

As part of the cleanup tracked by Bug 1815896 metabug, there is some logic specific to Colorways themes in the XPIProvider and AOM internals that should be also cleaned up.

The logic related to ColorWays from about:addons page internals have been instead all removed as part of Bug 1775332.

Comment 1

[Luca Greco [:rpl] [:luca] [:lgreco]]

Attachment: Bug 1875512 - Remove colorway builtin themes special handling from AddonManager and XPIProvider internals.

The set of changes included in this patch includes:

• all special handling related to colorway builtin themes removed from AddonManager and XPIProvider,
  besides the AddonWrapper getter named isBuiltinColowayTheme, because that is still technically being
  used by BuiltInThemes.sys.mjs which is part of what this bug is tracking and should be part of
  separate changes built on top of the finalized version of this patch
• remove test cases that were covering behaviors that are part of the special handling code being removed
• keeps the test coverage related to the non-colorway builtin themes localization, which is still supported,
• extend the test coverage by adding an explicit test case to verify that if a builtin theme is not
  fully removed from a user profile while its fluent strings are already gone, will make the AddonWrapper
  getters to fallback to the value set on the manifest property (e.g. user profiles where a colorway
  builtin is still retained and failed to migrate to the AMO hosted colorway theme will hit this issue
  and so this test coverage ensures that we handle that corner case more gracefully)

At this stage the colorways themes have not been removed from the tree and the BuiltInThemeConfig.sys.mjs,
and so those themes will still load just fine and the migration to an AMO hosted theme is not going to be
happening anymore.

In the patches meant to be completing the Colorways builtin themes cleanups we will have to:

• Remove all colorways builtin themes from BuiltInThemesConfig
• Remove all colorways builtin themes resources from the builds
• (At this stage the colorway theme will not be able to fully load correctly anymore, but for a user profile
  where the colorway builtin theme was the active theme the colors associated to the colorway theme are already
  stored in the AOM startup data and so the Firefox windows will still be using the expected set of colors
  even when the manifest.json file of the builtin colorway theme is not available anymore and cannot be loaded
  correctly anymore)
• After the browser startup has been completed, we should them either check if the active theme is still
  a builtin colorway theme, and if it is then message the user to let them know that the colorway builtin theme
  has been removed and they can install it from AMO if they still want to use that theme, and uninstall the
  builtin colorway theme
• (uninstalling the builtin colorway theme explicitly is needed also to clear the remaining bits of that
  builtin from the AddonManager startup data and AddonManager DB)
• it may also be worth considering to go through all themes and remove any other colorway theme that is still
  builtin and retained as a disabled theme (in the end those cannot be enabled anymore after their manifest
  files are not included in the build anymore).

Comment 2

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

There is an r+ patch which didn't land and no activity in this bug for 2 weeks.
:rpl, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 3

[Luca Greco [:rpl] [:luca] [:lgreco]]

The patch attached to this bugzilla issue is meant to land along (or at least in the same nightly cycle as) Bug 1815898, and so we will wait for Bug 1815898 patch to be also signed off and ready to be pushed to autoland.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0dfa3908de2
Remove colorway builtin themes special handling from AddonManager and XPIProvider internals. r=willdurand

Comment 5

[agoloman]

Backed out for causing bc failures @browser_BuiltInThemes_installs.js.

• Backout link
• Push with failures
• Failure Log

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a699cd919476
Remove colorway builtin themes special handling from AddonManager and XPIProvider internals. r=willdurand

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to agoloman from comment #5)

Backed out for causing bc failures @browser_BuiltInThemes_installs.js.

• Backout link
• Push with failures
• Failure Log

Clearing the needinfo, I replied with some more details about the reason behind this failure in Bug 1815898 comment 6.

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/a699cd919476