Open Bug 1727878 Opened 5 months ago Updated 3 months ago

[MSIX] Firefox is no longer opened after Refresh

Categories

(Toolkit :: Startup and Profile System, defect)

Desktop
Windows
defect

Tracking

()

Tracking Status
firefox92 --- affected
firefox93 --- affected
firefox94 --- affected

People

(Reporter: atrif, Unassigned)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fidedi-tikka])

Attachments

(2 files)

Attached image firefox_11.gif

Affected versions

  • 92.0b9 MSIX (20210826192006)
  • 93.0a1 MSIX (20210826213950)

Affected platforms

  • Windows 11x64

Preconditions

  • clean environment, no other firefox installed

Steps to reproduce

  1. Install Firefox MSIX ( https://treeherder.mozilla.org/jobs?repo=mozilla-beta&selectedTaskRun=UNdYfQLzSzaysQMdcgxJrg.0 / https://treeherder.mozilla.org/jobs?repo=mozilla-central&selectedTaskRun=KgLg2fyYTzi0dkmueSJpPQ.0)
  2. Enter about:support and select Refresh Nightly-> Refresh Nightly.

Expected result

  • Firefox is opened after refresh process is completed

Actual result

  • Firefox is closed and no longer opened and the refresh process is not displayed.

Regression range

  • I will search for one ASAP if there is one.

Notes

  • Attached a screen recording.
  • This only happens if no normal Firefox is installed inside the computer.
Has Regression Range: --- → no
Has STR: --- → yes

It seems that the issue is reproducible with 92.0a1 MSIX (20210728021153) as well by following the above STR. Therebefore I think that it's safe to assume that the issue is not a regression.

Has Regression Range: no → ---

Setup: Normal Run

The profile directory gets sent to the directory provider/service in XRE_mainStartup(). nsXREDirProvider::SetProfile() saves a reference to the profile root directory to return for "ProfD", but first it resolves that path (due to bug 1369670). A profile path initially looks like:

C:\Users\agash\AppData\Roaming\Mozilla\Profiles\usjl8hcn.default-beta

If that's only a virtual path, and the profile is actually in a package's private location, this resolves to a final form like:

C:\Users\agash\AppData\Local\Packages\Mozilla.Firefox.Beta_gmpnhwe7bv608\LocalCache\Roaming\Mozilla\Firefox\Profiles\usjl8hcn.default-beta

This introduces a potential inconsistency between the profile service's list of profiles and the directory service's "ProfD" [1].

Restart for Profile Reset

  1. When shutting down for restart, "ProfD" is saved and used to set XRE_PROFILE_PATH.
  2. On restart, nsToolkitProfileService::SelectStartupProfile() will try to get a profile using XRE_PROFILE_PATH.
  3. GetProfileByDir() looks for a profile with an equal root directory.
  4. Equals does some canonicalization but only up to short paths on Windows, so the paths won't match, and no profile is passed out of SelectStartupProfile().
  5. Because we're trying to do a profile reset, SelectProfile() fails.
  6. Firefox exits.

Why is this not prevented by resetSupported?

ResetProfile.resetSupported() is supposed to detect that this would happen. It checks that the current profile is known to the profile manager, using roughly the same logic. If not found it should hide the "Refresh Firefox..." button on about:support and elsewhere.

The problem is that nsXREDirProvider::SetProfile() is called with mProfD, and when it resolves the path it modifies the nsIFile that was passed in. If we had selected a default profile, this is the same nsIFile that is used to store the profile's root dir in the profile service [2]. So they'll always match in this case, even when SetProfile() modifies the path when resolving it. This can be seen in about:profiles in a package: Only the root directory of the current profile is resolved, and it is still recognized as the current profile (showing "This is the profile in use and it cannot be deleted"), though see Mode B below.

Failure Mode B

There's another failure mode, call it Mode B: After a non-profile-reset restart, e.g. "Restart normally..." in about:profiles, the profile list never gets a resolved root path for the active profile, though "ProfD" in the dirsvc is resolved, because these are different nsIFiles [3]. As a result the "Refresh Firefox..." section doesn't appear in about:support, and in about:profiles the current profile is marked as "This profile is in use by another application and it cannot be deleted." There may be other oddities. This doesn't fail to restart because we don't require a known profile unless we're restarting to reset a profile, normally we just need a directory.

Summary, solutions

In summary, a real mess. I think I can fix the immediate issue here by cloning and resolving the paths before comparing them in GetProfileForDir. That doesn't fix Mode B, for that we could separately make sure the root directory of the selected profile was resolved before we get to nsXREDirProvider::SetProfile() (though what about paths that don't exist yet?)

In bug 1513172 comment 3, :bobowen suggested we might avoid resolving the path for "ProfD", but that might be tricky, and at this point I don't know if we want to "fix" it, as there may be installs.ini or profiles.ini that rely on this by now.

At the same time, I'm concerned that this inconsistency could have some latent issues. Testing so far seems to only have turned up this one, there's also Mode B. Some may only be exposed when a significant population is on MSIX, until then it'd be rare to have profile or install paths that change when resolved. Maybe this or something similar is also responsible for bug 1721488 (jumplists, as it relates to install paths) or bug 1728161 (overeager remoting, as it may relate to profile paths).


[1] Note that this doesn't happen if the package creates its profile after an unpackaged Firefox had already run. If the top level AppData\Roaming\Mozilla path already existed on the real filesystem, then that will be used instead of the package's private location, so the final path would be the same (up to shortnames).

[2] The nsIFile is passed through SelectProfile, SelectStartupProfile, and GetDefaultProfile, though there are a whole lot of other possibilities in SelectStartupProfile and XRE_mainStartup which get the nsIFile from different places. For instance [3] with a non-profile-reset restart.

[3] In this case mProfD had been made fresh from XRE_PROFILE_PATH, passed back from SelectStartupProfile here.

Assignee: nobody → agashlin
Status: NEW → ASSIGNED
Component: Installer → Startup and Profile System
Product: Firefox → Toolkit
See Also: → 1513172

Resolving a clone before comparing in GetProfileByDir() allows the reset to work, but then there's a problem: The new profile paths for the install that get written into profiles.ini and installs.ini are the fully resolved paths, while the profile itself still has the unresolved path in profiles.ini, so when Firefox next starts up it can't choose a default and the profile picker is shown.

I'm struggling to find a way to deal with this inconsistency. Maybe if we always resolve early, as soon as things come out of the INIs or environment variables or special folders, we could just always be dealing with the resolved paths. There's a bootstrapping problem, as the per-package paths won't resolve until the files they refer to exist, and resolving %appdata% itself may never yield the per-package root. And if there is already a real %appdata%\Mozilla dir, then these dirs are getting merged and it gets even more confusing.

Or maybe we could never resolve, dealing with this instead when we need to build the sandboxing rules, though I fear that this will break for some existing users.

We could always have file comparisons use resolved paths, but this is probably going to be fairly expensive, and other file operations are probably still messed up.

Maybe it would be safest to just disable the profile refresh interfaces when packaged. That still leaves Failure Mode B, as I see was also noted in bug 1731759.

We have bug 1695556 (which I hope to fix soon) for removing a check and making some other changes to allow symbolic links in the sandbox rules.

These checks were there to protect against mount point reparse points, which originally could be created to anywhere, but have since been changed so that you can only create them to somewhere that you already have access. This fix was backported, so it should be safe to remove.

agashlin - would that fix this issue?

Flags: needinfo?(agashlin)

To be clear this would hopefully mean that we don't need to resolve those directories any more.

(In reply to Bob Owen (:bobowen) from comment #4)

We have bug 1695556 (which I hope to fix soon) for removing a check and making some other changes to allow symbolic links in the sandbox rules.

These checks were there to protect against mount point reparse points, which originally could be created to anywhere, but have since been changed so that you can only create them to somewhere that you already have access. This fix was backported, so it should be safe to remove.

Oh that's neat, do you recall when was that fixed in Windows?

(In reply to Bob Owen (:bobowen) from comment #5)

To be clear this would hopefully mean that we don't need to resolve those directories any more.

I think that would fix most of theses issues, yeah. I'm a little concerned that there may be profiles.ini, installs.ini, or other stuff out there already with those resolved paths saved, which might break default profile selection, maybe that's an acceptable risk?

If I disable resolving the profile dir, I don't see any issues, and these restart issues are fixed. I don't know what you were referring to in bug 1369670 comment 3, maybe it isn't an issue anymore? Or maybe I just didn't hit it, or it's subtle. I'll give it a run on try, there's probably something I'm missing.

Flags: needinfo?(agashlin)

Seems that this issue is fixed on 94.0b1, refreshing Firefox will work as expected.

I'm still seeing this issue in 94.0b1. Note that this only happens if refreshing a profile that was created when there were no non-packaged Firefox profiles, you may not have tested on a clean system. In any case, I'm not going to be able to address this.

Assignee: agashlin → nobody
Status: ASSIGNED → NEW

Tried again with the build you linked in Comment 8, did some history on a .zip Firefox and have an .exe installed as well before installing and refreshing it, still works. Using official release of Win 11, import was active both when I installed and ran the build and on refresh too. Don't know what needs to be done in order to reproduce it now.

  1. First delete or temporarily rename/move these directories under C:\Users\sasca: AppData\Local\Mozilla, AppData\LocalLow\Mozilla, and AppData\Roaming\Mozilla. If those folders exist, then they will be used for profiles instead of the per-package special locations which will have true paths under C:\Users\sasca\AppData\Local\Packages\Mozilla.MozillaFirefoxBeta_gmpnhwe7bv608\LocalCache\ and then this won't reproduce.
  2. Run the packaged beta
  3. Refresh

This still reproduces for me on Win 11 (though I still only have the last beta channel preview) and Win 10.

Yep, I was able to reproduce it after deleting the folders in AppData and ProgramData. Thank you!

(In reply to Bob Owen (:bobowen) from comment #4)

We have bug 1695556 (which I hope to fix soon) for removing a check and making some other changes to allow symbolic links in the sandbox rules.

These checks were there to protect against mount point reparse points, which originally could be created to anywhere, but have since been changed so that you can only create them to somewhere that you already have access. This fix was backported, so it should be safe to remove.

agashlin - would that fix this issue?

bobowen: sorry to leave this for a Long Time. Given the complexity of addressing this in the profile service in full generality, it is our preference that we address Bug 1695556 and then remove the reparse point handling code (per https://bugzilla.mozilla.org/show_bug.cgi?id=1727878#c6).

The ticket linked above has some patches that are 6 months old and have ground out; can we kick start those? And how risky is this change? We'd like to have it hit Firefox 94 (now halfway through Beta) but understand if that's not feasible.

Flags: needinfo?(bobowencode)

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

(In reply to Bob Owen (:bobowen) from comment #4)

We have bug 1695556 (which I hope to fix soon) for removing a check and making some other changes to allow symbolic links in the sandbox rules.

These checks were there to protect against mount point reparse points, which originally could be created to anywhere, but have since been changed so that you can only create them to somewhere that you already have access. This fix was backported, so it should be safe to remove.

agashlin - would that fix this issue?

bobowen: sorry to leave this for a Long Time. Given the complexity of addressing this in the profile service in full generality, it is our preference that we address Bug 1695556 and then remove the reparse point handling code (per https://bugzilla.mozilla.org/show_bug.cgi?id=1727878#c6).

The ticket linked above has some patches that are 6 months old and have ground out; can we kick start those? And how risky is this change? We'd like to have it hit Firefox 94 (now halfway through Beta) but understand if that's not feasible.

Sorry didn't understand the timescales here as this isn't affecting release currently.
After more information from chromium (I was waiting on this, which stalled that bug), we should be able to fix this by removing these checks altogether, because of more recent Windows security measures.
I'd already planned on tackling it this half for other reasons.
However, I'm afraid I don't think this is the sort of change we should drop into the last two weeks of Beta.

Flags: needinfo?(bobowencode)
Whiteboard: [fidedi-tikka]

(In reply to Bob Owen (:bobowen) from comment #13)

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

(In reply to Bob Owen (:bobowen) from comment #4)

We have bug 1695556 (which I hope to fix soon) for removing a check and making some other changes to allow symbolic links in the sandbox rules.

These checks were there to protect against mount point reparse points, which originally could be created to anywhere, but have since been changed so that you can only create them to somewhere that you already have access. This fix was backported, so it should be safe to remove.

agashlin - would that fix this issue?

bobowen: sorry to leave this for a Long Time. Given the complexity of addressing this in the profile service in full generality, it is our preference that we address Bug 1695556 and then remove the reparse point handling code (per https://bugzilla.mozilla.org/show_bug.cgi?id=1727878#c6).

The ticket linked above has some patches that are 6 months old and have ground out; can we kick start those? And how risky is this change? We'd like to have it hit Firefox 94 (now halfway through Beta) but understand if that's not feasible.

Sorry didn't understand the timescales here as this isn't affecting release currently.
After more information from chromium (I was waiting on this, which stalled that bug), we should be able to fix this by removing these checks altogether, because of more recent Windows security measures.
I'd already planned on tackling it this half for other reasons.
However, I'm afraid I don't think this is the sort of change we should drop into the last two weeks of Beta.

Re: timelines, this is my responsibility: I thought we were going to go in a different direction before reaching out. The timeline (Firefox 94) is to have the changes in place for the first release of Firefox to the Microsoft Store. Given that it's risky, we'll go to market as is; the impacted functionality isn't critical.

Hope to have something up for review on Bug 1695556 early next week.

You need to log in before you can comment on or make changes to this bug.