1733999 - Make remoting treat app packages/MSIX packages as distinct remoting targets

Status: VERIFIED FIXED

(Firefox :: Installer, enhancement)

Description

[Nick Alexander :nalexander [he/him]]

Right now, non-MSIX Firefox and Firefox Beta are treated as identical remoting targets, since each has RemotingName "firefox". This doesn't make sense for MSIX packages: we never want MozillaFirefox and MozillaFirefoxBeta to be considered the same remoting target.

This ticket tracks addressing that. I propose to extend the mechanisms of Bug 1709131 to keep the package identity in some usable form, and then to include it when appropriate in the "remoting class name" used to group instances: https://searchfox.org/mozilla-central/rev/b022ae1fc071ad7a29f64f281bc19b7b093df538/toolkit/components/remote/RemoteUtils.h#12.

One wrinkle is that it sure looks like the existing place keeping this information, namely WinUtils/sysinfo, is initialized well after the remote server is configured. So we may need to rearrange some deckchairs to cache this early and then access it from that location. I can help as needed.

Comment 1

[Nick Alexander :nalexander [he/him]]

Ah, I think the helper already exists: https://searchfox.org/mozilla-central/rev/01adc17c9a41d9f7975de170acc78634bd743609/widget/windows/WinUtils.h#231. I was confused with https://searchfox.org/mozilla-central/source/widget/windows/WinUtils.h#219, which stores information rather than just getting it on demand.

Comment 2

[Nick Alexander :nalexander [he/him]]

bhearsum: is this at, or near the top, of your list? Let me know if I can help or if you'd rather it off your plate. Thanks!

Comment 3

[[e-mail] (:bhearsum)]

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

bhearsum: is this at, or near the top, of your list? Let me know if I can help or if you'd rather it off your plate. Thanks!

Yes, I'm planning to take a first look this week. Sorry for the delay.

Comment 4

[[e-mail] (:bhearsum)]

Attachment: Bug 1733999: Make remoting treat app packages/MSIX packages as distinct remoting targets. r=nalexander!

If it exists, this will include the Windows packageFamilyName to the remoting class name. My C++ is extremely rusty, so I'm not sure if this is the most ergonomic way to do it. I'm happy to rework it into a nicer style if desired.

It's not clear to me whether this is the best place for this code to live, or if it should rather live in the Windows client and server code (nsWinRemote{Client,Server} at the time of writing). I chose to put it here for to keep it fully centralized, and avoid any possible risk of it diverging between the client and server.

I couldn't find any unit tests for this, but I tested on Windows by:

• Installing Firefox with the NSIS installer
• Installing my locally generated and signed MSIX build
• Launching Firefox, then MSIX installed version, and confirming they both launch independently
• Verifying that both work correctly as the default browser (I this is unrelated to remoting, but it seemed vaguely possible it may factor in)

I haven't done any explicit testing on Mac or Linux, but I think this is pretty obviously a no-op there by inspection.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f19b6755930
Make remoting treat app packages/MSIX packages as distinct remoting targets. r=nalexander

Comment 6

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7f19b6755930

Comment 7

[[e-mail] (:bhearsum)]

[Tracking Requested - why for this release]: Without this, launching Firefox installed through an MSIX package while an existing install is running will open a new window for that install, rather than launch the MSIX version. With this, the MSIX version will complain than an existing Firefox is running, and that the user must close it first. More discussion of this is in https://bugzilla.mozilla.org/show_bug.cgi?id=1728161, if needed.

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 9245800 [details]
Bug 1733999: Make remoting treat app packages/MSIX packages as distinct remoting targets. r=nalexander!

Beta/Release Uplift Approval Request

• User impact if declined: MSIX installed Firefox will not be launched correctly.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: 1) Delete all old profiles and profiles.ini

2. Install Firefox Beta through installer.exe and launch it
3. Install Firefox Beta through MSIX installer.msix and launch it

The second Firefox Beta should complain that "Firefox is currently running..."

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low risk because the only impact is around how Firefox is launched, and is effectively a no-op for non-MSIX versions.
• String changes made/needed:

Comment 9

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Hello. We verified that the Firefox is already running and not responding message is displayed by opening Firefox MSIX on a clean environment while Firefox.exe is already running. Testing was performed on Windows 11x64 with 95.0a1 (2021-10-18) MSIX and .exe builds. However, we encountered two scenarios regarding this and we don't know if they are expected:

1. When Clicking theClose Firefox button while the Firefox is already running message is displayed, the message closes and then reopens every time when the Close Firefox button is pressed.

2. Closing Firefox.exe while the Firefox is already running message is displayed and then clicking on the Close Firefox button afterward opens some Firefox instance that does not have updates enabled and no migration is performed. If this Firefox is pinned to the taskbar, then closed and reopened the migration is performed and the MSIX build is displayed.

Attached a screen recording covering both scenarios: link. Thank you!

Comment 10

[[e-mail] (:bhearsum)]

Thanks for testing this!

(In reply to Alexandru Trif, QA [:atrif] from comment #9)

Hello. We verified that the Firefox is already running and not responding message is displayed by opening Firefox MSIX on a clean environment while Firefox.exe is already running. Testing was performed on Windows 11x64 with 95.0a1 (2021-10-18) MSIX and .exe builds. However, we encountered two scenarios regarding this and we don't know if they are expected:

1. When Clicking theClose Firefox button while the Firefox is already running message is displayed, the message closes and then reopens every time when the Close Firefox button is pressed.

I believe we expected this from this change, and considered it acceptable, although I will double check.

2. Closing Firefox.exe while the Firefox is already running message is displayed and then clicking on the Close Firefox button afterward opens some Firefox instance that does not have updates enabled and no migration is performed. If this Firefox is pinned to the taskbar, then closed and reopened the migration is performed and the MSIX build is displayed.

This one is new to me. I'll dig into it more.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9245800 [details]
Bug 1733999: Make remoting treat app packages/MSIX packages as distinct remoting targets. r=nalexander!

Needed for the MSIX installer MVP work shipping in Fx94. The issues found by QA seem edge-case enough to live with for now. Approved for 94.0b8.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/03c2adbe889b

Comment 13

[[e-mail] (:bhearsum)]

(In reply to Alexandru Trif, QA [:atrif] from comment #9)

2. Closing Firefox.exe while the Firefox is already running message is displayed and then clicking on the Close Firefox button afterward opens some Firefox instance that does not have updates enabled and no migration is performed. If this Firefox is pinned to the taskbar, then closed and reopened the migration is performed and the MSIX build is displayed.

A couple of things I'd like clarify:

• Updates are supposed to be disabled for the MSIX build. I don't think this affects profile migration in any way
• I also don't see how or why taskbar pinning is involved here - are you sure that's part of the STR?

In my own testing with your STR, this is what I found:

• After closing the running Firefox, then clicking "Close Firefox" on the dialog, the MSIX version was launched and did import the profile on most attempts (eg: I can see the history from the other profile). However, it did this without showing the migration wizard.
• After closing the now running MSIX version and reopening it, the migration wizard was shown, and I continue to see the history from the other profile.

My theory for the one time the profile did not import on the first attempt was that the original Firefox was still in the process of shutting down when the MSIX version started, and it was unable to import the profile because of this -- although I haven't been able to verify this theory.

The other thing I noticed here was that with your STR, the first launch of the MSIX version is in a bit of a weird state. Notably, the About dialog doesn't contain "Windows MSIX package", as it does normally (or even on subsequent launches in this scenario). I'm not certain why, but between the profile migration UI not showing, and this, it makes me wonder what else may be broken in this scenario. I'll dig into it more.

Comment 14

[[e-mail] (:bhearsum)]

(In reply to [e-mail] (:bhearsum) from comment #10)

Thanks for testing this!

(In reply to Alexandru Trif, QA [:atrif] from comment #9)

Hello. We verified that the Firefox is already running and not responding message is displayed by opening Firefox MSIX on a clean environment while Firefox.exe is already running. Testing was performed on Windows 11x64 with 95.0a1 (2021-10-18) MSIX and .exe builds. However, we encountered two scenarios regarding this and we don't know if they are expected:

1. When Clicking theClose Firefox button while the Firefox is already running message is displayed, the message closes and then reopens every time when the Close Firefox button is pressed.

I believe we expected this from this change, and considered it acceptable, although I will double check.

Nick, do you have any thoughts to add here? This seems like perhaps something that will be handled in https://bugzilla.mozilla.org/show_bug.cgi?id=1734000?

Comment 15

[Alexandru Trif, Desktop Test Engineering [:atrif]]

(In reply to [e-mail] (:bhearsum) from comment #13)

(In reply to Alexandru Trif, QA [:atrif] from comment #9)

2. Closing Firefox.exe while the Firefox is already running message is displayed and then clicking on the Close Firefox button afterward opens some Firefox instance that does not have updates enabled and no migration is performed. If this Firefox is pinned to the taskbar, then closed and reopened the migration is performed and the MSIX build is displayed.

A couple of things I'd like clarify:

• Updates are supposed to be disabled for the MSIX build. I don't think this affects profile migration in any way
• I also don't see how or why taskbar pinning is involved here - are you sure that's part of the STR?

In my own testing with your STR, this is what I found:

• After closing the running Firefox, then clicking "Close Firefox" on the dialog, the MSIX version was launched and did import the profile on most attempts (eg: I can see the history from the other profile). However, it did this without showing the migration wizard.
• After closing the now running MSIX version and reopening it, the migration wizard was shown, and I continue to see the history from the other profile.

My theory for the one time the profile did not import on the first attempt was that the original Firefox was still in the process of shutting down when the MSIX version started, and it was unable to import the profile because of this -- although I haven't been able to verify this theory.

This is what I was thinking as well, maybe Closing normal Firefox and clicking the Close Firefox button too fast, happens when normal Firefox is in the process of closing as you said, but I did not test this.

The Pin to taskbar step is done just to see what instance of Firefox was opened after clicking the Close Firefox button. The step was added there only for that. I wasn't sure how to show that for sure inside the screen recording.

Comment 16

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Verified that the Firefox is already running and not responding window is displayed by opening Firefox MSIX on a clean environment while Firefox.exe is already running. Testing was performed on Windows 11x64 with 95.0a1 (2021-10-18) and 94.0b8 MSIX and .exe builds.

Also, we opened bug 1736832 and bug 1736833 for the above scenarios to track them there.