Open Bug 1830507 Opened 2 years ago Updated 10 months ago

Default addons e.g. webcompat/screenshots/pictureinpicture break when application path changes, e.g. by updating Nightly MSIX package

Categories

(Toolkit :: Add-ons Manager, defect, P2)

Product:
Toolkit
Component:
Add-ons Manager
Type:
defect

Tracking

()

Status:
REOPENED

People

(Reporter: saschanaz, Assigned: saschanaz)

References

Details

(Whiteboard: [addons-jira])

Attachments

(2 obsolete files)

The log when booting Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1747267#c12

Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/formautofill@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE) Extension.jsm:818
1681998163013	addons.xpi	WARN	Exception running bootstrap method startup on formautofill@mozilla.org: Error: Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/formautofill@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE)(resource://gre/modules/Extension.jsm:818:20) JS Stack trace: readJSON/</<@Extension.jsm:818:20
onStopRequest@NetUtil.jsm:126:18
Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/pictureinpicture@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE) Extension.jsm:818
1681998163014	addons.xpi	WARN	Exception running bootstrap method startup on pictureinpicture@mozilla.org: Error: Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/pictureinpicture@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE)(resource://gre/modules/Extension.jsm:818:20) JS Stack trace: readJSON/</<@Extension.jsm:818:20
onStopRequest@NetUtil.jsm:126:18
Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/screenshots@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE) Extension.jsm:818
1681998163014	addons.xpi	WARN	Exception running bootstrap method startup on screenshots@mozilla.org: Error: Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/screenshots@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE)(resource://gre/modules/Extension.jsm:818:20) JS Stack trace: readJSON/</<@Extension.jsm:818:20
onStopRequest@NetUtil.jsm:126:18
Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/webcompat-reporter@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE) Extension.jsm:818
1681998163014	addons.xpi	WARN	Exception running bootstrap method startup on webcompat-reporter@mozilla.org: Error: Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/webcompat-reporter@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE)(resource://gre/modules/Extension.jsm:818:20) JS Stack trace: readJSON/</<@Extension.jsm:818:20
onStopRequest@NetUtil.jsm:126:18
Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/webcompat@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE) Extension.jsm:818
1681998163015	addons.xpi	WARN	Exception running bootstrap method startup on webcompat@mozilla.org: Error: Error while loading 'jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/webcompat@mozilla.org.xpi!/manifest.json' (NS_ERROR_FAILURE)(resource://gre/modules/Extension.jsm:818:20) JS Stack trace: readJSON/</<@Extension.jsm:818:20
onStopRequest@NetUtil.jsm:126:18

And the path is wrong: https://bugzilla.mozilla.org/show_bug.cgi?id=1747267#c14

Okay, so there's no path like C:/Program Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/ as the real path includes Mozilla.MozillaFirefoxNightly_114.2304.1921.0_x64__gmpnhwe7bv608. For some reason it refers to a path of some old build.

Summary: Default addons e.g. webcompat/screenshots/pictureinpicture break when updating MSIX package → Default addons e.g. webcompat/screenshots/pictureinpicture break when updating Nightly MSIX package

Changing to see-also since bug 1429838 is more about profile path and this one is AFAICT about the binary path.

No longer depends on: 1429838
See Also: → 1429838

Relevant comment from https://bugzilla.mozilla.org/show_bug.cgi?id=1747267#c18:

(In reply to Nick Alexander :nalexander [he/him] from comment #15)

(In reply to Kagami [:saschanaz] from comment #14)

Okay, so there's no path like C:/Program Files/WindowsApps/Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608/ as the real path includes Mozilla.MozillaFirefoxNightly_114.2304.1921.0_x64__gmpnhwe7bv608. For some reason it refers to a path of some old build. Does this ring a bell?

It rings no bells. My guess is that the per-profile addons DB is baking the installation path into its storage in some manner, and that is breaking as the installation path moves around. That was a reasonable approach that only changed perhaps a year or two ago and was certainly not appreciated until much more recently. (E.g., https://bugzilla.mozilla.org/show_bug.cgi?id=1818606.)

Rob: do you know if the above is true (or possible)? Could you redirect if there's a person more likely to know?

This is certainly a possibility. When profile directories are moved around, something similar happens (bug 1429838), because the absolute path to the add-on location (xpi file) is stored in addonStartup.json.lz4 and extensions.json.

BTW, the path being pinned to Mozilla.MozillaFirefoxNightly_114.2304.1303.0_x64__gmpnhwe7bv608, perhaps that means the path only updates when the version number changes? 2023-04-10 was the merge date and the path indicates the build date 2023-04-13.

(The same path is still being used even when I'm now on 2023-04-27 build.)

This workaround also works here: https://bugzilla.mozilla.org/show_bug.cgi?id=1429838#c30

A quick manual fix:

  1. navigate to about:profiles
  2. Open the folder listed as Root Directory under the currently used profile ("This is the profile in use...")
  3. Remove the file addonStartup.json.lz4
  4. Restart firefox twice.
Severity: -- → S3
Priority: -- → P2
Summary: Default addons e.g. webcompat/screenshots/pictureinpicture break when updating Nightly MSIX package → Default addons e.g. webcompat/screenshots/pictureinpicture break when application path changes, e.g. by updating Nightly MSIX package
Whiteboard: [addons-jira]

I can confirm that the path updates when the major version bumps.

(In reply to Kagami [:saschanaz] from comment #5)

I can confirm that the path updates when the major version bumps.

And setting extensions.lastAppVersion can trigger this. I guess we can have extensions.lastBinPath for this bug's purpose.

This triggers regenerating addonStartup.json.lz4 when binary path changes, as the path includes build date in Windows MSIX installation.

Services.dirsvc.get("XREExeF", Ci.nsIFile).path
> "C:\\Program Files\\WindowsApps\\Mozilla.MozillaFirefoxNightly_116.2306.909.0_x64__gmpnhwe7bv608\\VFS\\ProgramFiles\\MozillaFirefoxNightly Package Root\\firefox.exe"
Assignee: nobody → krosylight
Status: NEW → ASSIGNED

(In reply to Kagami [:saschanaz] from comment #6)

(In reply to Kagami [:saschanaz] from comment #5)

I can confirm that the path updates when the major version bumps.

And setting extensions.lastAppVersion can trigger this. I guess we can have extensions.lastBinPath for this bug's purpose.

Your current approach is to fake a major version bump without a change to the major version. That approach looks too blunt to me, especially when there is already an existing mechanism designed for detecting and repairing database inconsistencies, see XPIProvider.checkForChanges and its implementation - https://searchfox.org/mozilla-central/rev/1d43d9f3d0ffcbdb619cfd1e2911fb22d1456657/toolkit/mozapps/extensions/internal/XPIProvider.jsm#3047,3057-3059

I was thinking of a fix in the direction of: when the path of the xpi is dependent on a "constant" location (e.g. the app binary path), and that "constant" location changes, then the path should be recomputed. That general fix would cover this bug and bug 1429838.

(In reply to Rob Wu [:robwu] from comment #8)

Your current approach is to fake a major version bump without a change to the major version. That approach looks too blunt to me, especially when there is already an existing mechanism designed for detecting and repairing database inconsistencies, see XPIProvider.checkForChanges and its implementation - https://searchfox.org/mozilla-central/rev/1d43d9f3d0ffcbdb619cfd1e2911fb22d1456657/toolkit/mozapps/extensions/internal/XPIProvider.jsm#3047,3057-3059

IMO it's okay to be "blunt" here as it's not something happening for every startup, isn't it? At most it should only happen daily after update, and rechecking after update is not something entirely unexpected.

I was thinking of a fix in the direction of: when the path of the xpi is dependent on a "constant" location (e.g. the app binary path), and that "constant" location changes, then the path should be recomputed. That general fix would cover this bug and bug 1429838.

I see two issues:

  1. Currently XPIProvider.scanForChanges ignores everything by default when aAppChanged is false.
  2. Even if it's true, somehow the addonChanged check is faulty; both file.path and xpiState.path uses the right path that is not actually used to load the extension, so it fails to detect the change. For example,
    • both point to C:\Program Files\WindowsApps\Mozilla.MozillaFirefoxNightly_116.2306.1609.0_x64__gmpnhwe7bv608\VFS\ProgramFiles\MozillaFirefoxNightly Package Root\browser\features\webcompat-reporter@mozilla.org.xpi which exists,
    • ... while the load error message points to jar:file:///C:/Program%20Files/WindowsApps/Mozilla.MozillaFirefoxNightly_116.2306.1421.0_x64__gmpnhwe7bv608/VFS/ProgramFiles/MozillaFirefoxNightly%20Package%20Root/browser/features/webcompat-reporter@mozilla.org.xpi!/manifest.json which does not exist.

Thoughts?

Flags: needinfo?(rob)

(In reply to Kagami [:saschanaz] from comment #9)

I see two issues:

  1. Currently XPIProvider.scanForChanges ignores everything by default when aAppChanged is false.

Not everything is ignored. Whenever the application has a meaningful update, its buildId changes. That triggers the code path where the ignoreSideloads flag is effectively ignored. Here is a step-by-step evaluation:

  • if (ignoreSideloads && !(loc.scope & startupScanScopes)) { with loc.scope = 4 (aka SCOPE_APPLICATION) and startupScanScopes = 31 (aka SCOPE_ALL)
  • if (ignoreSideloads && !(l4 & 31) {
  • if (ignoreSideloads && !(4)) {
  • if (ignoreSideloads && false) {
  • if (false) {

To simulate the behavior without app changes, set the "extensions.startupScanScopes" preference to 31 to unconditionally simulate the same behavior. I highly recommend creating a xpcshell unit test that sets this flag, in order to test this behavior.

  1. Even if it's true, somehow the addonChanged check is faulty; both file.path and xpiState.path uses the right path that is not actually used to load the extension, so it fails to detect the change.

If it worked, then we wouldn't have this bug report ;)

Here are some pointers:

I would explore the following:

  • From where is the add-on loaded? You could use the Browser Toolbox to put a breakpoint in the XPIProvider.sys.mjs module and install a xpi file from addons.mozilla.org to try and get a better understanding of how an add-on is started.
  • Can DirectoryLocation detect inconsistencies in its path and the add-ons that are allegedly part of it, and repair the path as needed?
    • (I believe that the answer is yes, in restore or the methods/classes that is calls)
  • Can we repair without doing repeated expensive file I/O checks?
    • (again, I believe that the answer is yes)
  • Create xpcshell unit test to reproduce the issue.

I hope this helps!

Flags: needinfo?(rob)

Amazing, deeply thank you for the detailed information! I think I got alternative solution. One question:

(In reply to Rob Wu [:robwu] from comment #10)

(In reply to Kagami [:saschanaz] from comment #9)

I see two issues:

  1. Currently XPIProvider.scanForChanges ignores everything by default when aAppChanged is false.

Not everything is ignored. Whenever the application has a meaningful update, its buildId changes. That triggers the code path where the ignoreSideloads flag is effectively ignored. Here is a step-by-step evaluation:

I totally missed the build id check, and your explanation makes total sense! But from my understanding it still can't solve bug 1429838 as moving profile does not change build id, right? It's out of scope here, I just want to double check since you mentioned it in comment #8.

Flags: needinfo?(rob)

(In reply to Kagami [:saschanaz] from comment #11)

(In reply to Rob Wu [:robwu] from comment #10)

Not everything is ignored. Whenever the application has a meaningful update, its buildId changes. That triggers the code path where the ignoreSideloads flag is effectively ignored. Here is a step-by-step evaluation:

I totally missed the build id check, and your explanation makes total sense! But from my understanding it still can't solve bug 1429838 as moving profile does not change build id, right? It's out of scope here, I just want to double check since you mentioned it in comment #8.

If the fix were to depend on iterating over all directory entries on disk, then a fix here would not fix bug 1429838 at once.

But if the fix consists of rewriting saved.path before/while constructing XPIState, then that would also fix the other bug. As the mechanism to generate the paths is consistent, it should be possible to check whether rootURI still matches location.dir, and if not, to extract the leaf name and rebuild rootURI with location.dir.

Flags: needinfo?(rob)

Note for myself:

For some reason SystemDefaultsLoc() fails and thus app-system-defaults is being skipped in xpcshell tests, but fixing that is out of scope here when this bug is theoretically not limited to default addons. Try testing on app-profile instead.

Attachment #9339633 - Attachment description: WIP: Bug 1830507 - Update extension.json when addon location path changes r=robwu → Bug 1830507 - Update startup data when addon location path changes r=robwu
Attachment #9338290 - Attachment is obsolete: true
Attachment #9339633 - Attachment is obsolete: true

Somehow this is fixed 👍

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → WORKSFORME

Restarted some time ago...

Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: