1565597 - Cannot Start multiple Profiles

Status: VERIFIED FIXED

(Toolkit :: Startup and Profile System, defect, P2)

Description

[DStaal]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:68.0) Gecko/20100101 Firefox/68.0

Steps to reproduce:

Updated Firefox to 68.0, opened instance with one profile, then attempted to start a new instance under a different profile via command line on MacOS.

Actual results:

A new tab opened with the requested page in the current profile.

Expected results:

A separate instance should start with the requested page and profile.

Comment 1

[Dave Townsend [:mossop]]

What was the command line you used?

Comment 2

[DStaal]

/Applications/Firefox.app/Contents/MacOS/firefox -P "A-Team Pass" https://URL:10001/

(URL removed.)

Comment 3

[Dave Townsend [:mossop]]

I imagine that this was caused by bug 469990

Comment 4

[Markus Keller]

Bug 1553767 comment 1 from 2 months ago contains a workaround: Add --no-remoteto the command line -- an option that is not documented in firefox-bin --help

Comment 5

[DStaal]

I can confirm that --no-remote works. (And that I don't see it in --help.)

This possibly should be combined with 1553767.

Comment 6

[Stephen A Pohl [:spohl]]

Yuri, would you like to take a look at this?

Comment 8

[Yuri Z.]

(In reply to Stephen A Pohl [:spohl] from comment #6)

Yuri, would you like to take a look at this?

Looking into these.
Not clearing the NI to keep track of them.

Comment 9

[Yuri Z.]

Attachment: -P and -p now launch a new instance

Attached is a fix that handles -P and -p, as well as --profile and -profile command line arguments. Once encountered, a new instance is launched (a --no-remote argument is assumed). Would this be the intended behavior?

Comment 10

[Yuri Z.]

Attachment: Updated to handle -profileManager as well as other profile commands.

Updated the patch to handle --profileManager and various case-alternatives of the other profile commands.

Comment 11

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #9)

Created attachment 9079694 [details] [diff] [review]
-P and -p now launch a new instance

Attached is a fix that handles -P and -p, as well as --profile and -profile command line arguments. Once encountered, a new instance is launched (a --no-remote argument is assumed). Would this be the intended behavior?

On other platforms whether a new instance is launched or not would depend on whether there are other instances using the requested profile.

Is there no way we could move OSX to using the same remoting system that other platforms use? This would remove all the special casing that you're having to add and more or less guarantee that OSX behaved the same as other platforms.

Comment 12

[Stephen A Pohl [:spohl]]

See comment 11.

Comment 13

[Yuri Z.]

When using the aforementioned unified remote mechanism, once the -no-remote is specified (which happens when ran with ./mach run, for example), the remote server/client are completely bypassed in such process and we have no control over remote logic from neither the original instance nor the duplicate instance of firefox.

So, say you have launched firefox with ./mach run. Now you cannot launch any other instance of firefox, you will always get a "A copy of Firefox is already open. Only one copy of Firefox can be open at a time." message. The same would happen, if you had "./firefox" launched, and launching another instance with "./mach run".

Essentially this means that on Mac "--no-remote" forces a single instance of Firefox to be launched, unless we explicitly handle -no-remote argument on other levels (which we already do in nsNativeAppSupportCocoa.mm).

Would you have any suggestion on how to deal with this situation, other than keep the current implementation?

Comment 14

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #13)

When using the aforementioned unified remote mechanism, once the -no-remote is specified (which happens when ran with ./mach run, for example), the remote server/client are completely bypassed in such process and we have no control over remote logic from neither the original instance nor the duplicate instance of firefox.

That is the intentional behaviour of no-remote, to turn off remoting.

So, say you have launched firefox with ./mach run. Now you cannot launch any other instance of firefox, you will always get a "A copy of Firefox is already open. Only one copy of Firefox can be open at a time." message. The same would happen, if you had "./firefox" launched, and launching another instance with "./mach run".

Only if the two instances try to use the same profile. Different installs of Firefox use different profiles by default.

Essentially this means that on Mac "--no-remote" forces a single instance of Firefox to be launched, unless we explicitly handle -no-remote argument on other levels (which we already do in nsNativeAppSupportCocoa.mm).

Would you have any suggestion on how to deal with this situation, other than keep the current implementation?

The situation you describe matches how it works on other platforms so this would make OSX behave the same as everywhere else.

Comment 15

[Yuri Z.]

Attachment: remotePatch.patch

Hello
Attached is a patch (as generated with "hg diff -g -U10") that moves the remoting logic into Unified Remote module.
Can't needinfo Stephen, so adding Dave to take a look.
Tried to keep Mozilla formatting of the code.

Comment 16

[Yuri Z.]

Dave, would you be so kind to run the tests if the code looks alright to you?

Comment 17

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #16)

Dave, would you be so kind to run the tests if the code looks alright to you?

Sure, would you mind uploading it to phabricator though?

Comment 18

[Yuri Z.]

Attachment: Unified Mozilla remoting mechanism implementation for macOS

Mozilla Remoting - is a way for a newly ran process instance to discover another already running instance upon application launch. This can be helpful to make one of them activate another one instead of launching a separate process, passing command line arguments to the original, previously ran instance.

Windows and Linux already use a unified remoting client-server model.

This patch implements a similar solution for Mac, essentially making it behave the same way on all platforms.

Comment 19

[Yuri Z.]

Hi Dave, uploaded it here:
https://phabricator.services.mozilla.com/D41762

I've never used Phabricator before, so please let me know if anything is incorrect.

Comment 20

[Yuri Z.]

Updated the patch to have a better text formatting.

Comment 21

[Yuri Z.]

Attachment: unifiedRemote14Aug.patch

Comment 22

[Ryan VanderMeulen [:RyanVM]]

Too late for 69 at this point with RC coming next week.

Comment 23

[Yuri Z.]

Stephen, can you please take a look at the fix?

Comment 24

[Stephen A Pohl [:spohl]]

Adding n-i for myself to review patch.

Comment 25

[Yuri Z.]

Thanks Stephen - please note that the latest changes are on Phabricator

Comment 28

[Yuri Z.]

Hi Stephen, was wondering if you had a chance to take a look at the patch? Thank you!

Comment 29

[Stephen A Pohl [:spohl]]

I'm finally reviewing the patch now.

Comment 30

[Yuri Z.]

Hi Stephen, just FYI, the patch had been updated per your comments.

Comment 31

[Yuri Z.]

Ping

Comment 32

[Yuri Z.]

Hi Stephen, I addressed your remarks on Phabricator, please have a look.
Thanks.

Comment 33

[Yuri Z.]

Attachment: Bug 1565597 - Universal Mac Remote implementation r=spohl

Comment 34

[Stephen A Pohl [:spohl]]

Thanks, I have reviewed the changes.

Dave, what are your thoughts about having QA draw up a test plan and give this a good test run? Since this is a rather major change with risks to our updates etc., I would feel more comfortable if we had this properly tested. You may also be in the best position to give QA guidance on the test plan.

Comment 35

[Yuri Z.]

Hi Stephen, the patch has been updated

Comment 36

[Dave Townsend [:mossop]]

(In reply to Stephen A Pohl [:spohl] from comment #34)

Thanks, I have reviewed the changes.

Dave, what are your thoughts about having QA draw up a test plan and give this a good test run? Since this is a rather major change with risks to our updates etc., I would feel more comfortable if we had this properly tested. You may also be in the best position to give QA guidance on the test plan.

We should be able to re-use some of the checks from when dedicated profiles landed, but I'm not sure what QA resources we'd be able to get before landing. We're in a soft code freeze now so I don't think we should land this until after the merge on 21st at which point we'll get 6 weeks bake time on Nightly which should be enough to shake out problems from nightly testers.

Comment 37

[Stephen A Pohl [:spohl]]

Okay. I'm adding :rstrong and :mhowell to let them know that this is incoming in case there is anything update related that we want to test.

Comment 38

[Yuri Z.]

Hi Dave, Stephen,
Do we have any QA resources available now to verify the change?

Thanks!

Comment 39

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #38)

Hi Dave, Stephen,
Do we have any QA resources available now to verify the change?

Thanks!

I'm not sure. Adrian and Ovidiu did the QA work on profile-per-install, perhaps they may be able to give this a quick skim, but they may be loaded with other work.

Comment 40

[Adrian Florinescu [:aflorinescu]]

IMO, the potential for regression here is quite high, so assuming this change will pose regression potential for mac only, which I'm not sure at this point, guessing that dedicated profiles functionality and start-up parameters functionality would be a minimum requirement.

In regards of availability, given the bug verification here would imply basically rerunning most of the dedicated profile suite + additional tests for start-up, I'd say a PI request (https://mana.mozilla.org/wiki/pages/viewpage.action?spaceKey=PI&title=PI+Request) for this fix would be best approach. As time-frame, the Fx72 is not really doable now, unless a exception path (https://docs.google.com/document/d/1jOTngDV_WWJGelTz4205ux4i5sAWfYNILHYmLLmHYeQ/edit#heading=h.f58ossjp1u3s) is to be used - I don't think this fix would be best suited to follow the exception path since I reckon it would be useful to have it baked at least a bit of time in Nightly after testing.

Comment 41

[Yuri Z.]

Hi Dave, what is the best way to move forward here to get this issue fix completed?
The change touches slightly Windows as well (minor file restructuring, fwiw, I've verified this and saw no issues).

Comment 42

[Yuri Z.]

Ping

Comment 43

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #41)

Hi Dave, what is the best way to move forward here to get this issue fix completed?
The change touches slightly Windows as well (minor file restructuring, fwiw, I've verified this and saw no issues).

My apologies, we are failing you here. I think I'm actually going to disagree with Adrian here. Given that this is moving OSX to use the same startup path as other platforms (and the code in there looks reasonably straightforward) I think the risk of regression isn't too high. This is also functionality that, in general, is nightly/beta facing and so with full cycles we should shake out any issues before release. I'm going to do a really quick smoketest of this (hopefully today, if not tomorrow) and then assuming no issues land it. We're at the start of a new nightly cycle so that will give us a bunch of time to hammer out issues.

Comment 44

[Dave Townsend [:mossop]]

Alright I did a bunch of testing this morning and hit one case where this is not behaving as on other platforms with respect to opening links from other apps.

I created two installs with this patch applied. Running each opens different instances using different profiles as expected. I made one of them the default browser so lets call the installs and profiles "default" and "non-default".

With both closed opening a link from another app opens "default". This is correct.
With both open opening a link from another app opens it in the existing "default". This is correct.
With just "default" open opening a link from another app opens it in the existing "default". This is correct.
With just "nondefault" open opening a link from another app opens it in the existing "nondefault". This is not correct.

Yuri can you look into this?

Comment 45

[Michael Froman [:mjf]]

(In reply to Dave Townsend [:mossop] (he/him) from comment #44)

Alright I did a bunch of testing this morning and hit one case where this is not behaving as on other platforms with respect to opening links from other apps.

I created two installs with this patch applied. Running each opens different instances using different profiles as expected. I made one of them the default browser so lets call the installs and profiles "default" and "non-default".

With both closed opening a link from another app opens "default". This is correct.
With both open opening a link from another app opens it in the existing "default". This is correct.
With just "default" open opening a link from another app opens it in the existing "default". This is correct.
With just "nondefault" open opening a link from another app opens it in the existing "nondefault". This is not correct.

Yuri can you look into this?

I tried these steps on macOS without this patch where Nightly is my default, and Release is non-default. My results are the same, which would make it seem like this patch isn't causing the incorrect behavior in the 4th case above.

Comment 46

[Dave Townsend [:mossop]]

(In reply to Michael Froman [:mjf] from comment #45)

(In reply to Dave Townsend [:mossop] (he/him) from comment #44)

Alright I did a bunch of testing this morning and hit one case where this is not behaving as on other platforms with respect to opening links from other apps.

I created two installs with this patch applied. Running each opens different instances using different profiles as expected. I made one of them the default browser so lets call the installs and profiles "default" and "non-default".

With both closed opening a link from another app opens "default". This is correct.
With both open opening a link from another app opens it in the existing "default". This is correct.
With just "default" open opening a link from another app opens it in the existing "default". This is correct.
With just "nondefault" open opening a link from another app opens it in the existing "nondefault". This is not correct.

Yuri can you look into this?

I tried these steps on macOS without this patch where Nightly is my default, and Release is non-default. My results are the same, which would make it seem like this patch isn't causing the incorrect behavior in the 4th case above.

Sure, but the expectation I have is that the patch should fix this behaviour.

Comment 47

[Michael Froman [:mjf]]

(In reply to Dave Townsend [:mossop] (he/him) from comment #46)

Sure, but the expectation I have is that the patch should fix this behaviour.

Ah, I thought you were saying the patch changed the behavior. Sorry about that! :-)

Comment 48

[Yuri Z.]

(In reply to Dave Townsend [:mossop] (he/him) from comment #44)

Alright I did a bunch of testing this morning and hit one case where this is not behaving as on other platforms with respect to opening links from other apps.

I created two installs with this patch applied. Running each opens different instances using different profiles as expected. I made one of them the default browser so lets call the installs and profiles "default" and "non-default".

With both closed opening a link from another app opens "default". This is correct.
With both open opening a link from another app opens it in the existing "default". This is correct.
With just "default" open opening a link from another app opens it in the existing "default". This is correct.
With just "nondefault" open opening a link from another app opens it in the existing "nondefault". This is not correct.

Yuri can you look into this?

Hi Dave,
You created two copies of the Nightly.app in different locations, and set one of them to be default browser? (when I try to do that in about:settings I see both of them "Nightly is your default browser").
Also, I understand you assigned a different profile for each of the .app copies? How can I do that?
I can launch an instance from command line with --profile profilename specified, but that sets the specified profile as the default one, so any other Nightly instance will use this new default profile as well.
Can you please elaborate on how can I reproduce this?

Comment 49

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #48)

(In reply to Dave Townsend [:mossop] (he/him) from comment #44)

Alright I did a bunch of testing this morning and hit one case where this is not behaving as on other platforms with respect to opening links from other apps.

I created two installs with this patch applied. Running each opens different instances using different profiles as expected. I made one of them the default browser so lets call the installs and profiles "default" and "non-default".

With both closed opening a link from another app opens "default". This is correct.
With both open opening a link from another app opens it in the existing "default". This is correct.
With just "default" open opening a link from another app opens it in the existing "default". This is correct.
With just "nondefault" open opening a link from another app opens it in the existing "nondefault". This is not correct.

Yuri can you look into this?

Hi Dave,
You created two copies of the Nightly.app in different locations, and set one of them to be default browser? (when I try to do that in about:settings I see both of them "Nightly is your default browser").

OSX is pretty bad about letting you choose a specific app to be the default, all we can see from the outside is the bundle ID of the default which is the same for both. That raises a thought though, this may be OSX causing the issue.

Also, I understand you assigned a different profile for each of the .app copies? How can I do that?

Since Firefox 67 every different install of Firefox gets its own default profile. Just launch one and it should create a profile. Launching the other should create itself a different profile.

Comment 50

[Yuri Z.]

Since Firefox 67 every different install of Firefox gets its own default profile. Just launch one and it should create a profile. Launching the other should create itself a different profile.

Thanks. Whenever I launch any of the instances, the profile that corresponds to that instance, becomes default (Nightly 73.0a1 (2019-12-07) (64-bit)). Not sure if this is expected behavior.

That raises a thought though, this may be OSX causing the issue.
according to https://developer.apple.com/documentation/coreservices/1448824-lscopydefaultapplicationurlforur, it looks like Launch Services doesn't expect to have multiple apps with the same bundle id registered in it as a default browser. You are correct that they are identified by bundle ids, not by the app path.

Dave, could you please check if there are other issues? Or should we wait until the current nightlies merge into the mainline, and if anything else pops up in between, fix that? What is the process?

Comment 51

[DStaal]

As another way to launch multiple profiles: You can wrap a single install with Automator actions, to pass options on the command line. Then you do not need multiple copies of the binaries.

Comment 52

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #50)

That raises a thought though, this may be OSX causing the issue.
according to https://developer.apple.com/documentation/coreservices/1448824-lscopydefaultapplicationurlforur, it looks like Launch Services doesn't expect to have multiple apps with the same bundle id registered in it as a default browser. You are correct that they are identified by bundle ids, not by the app path.

Dave, could you please check if there are other issues? Or should we wait until the current nightlies merge into the mainline, and if anything else pops up in between, fix that? What is the process?

I'll re-do the test with different app bundle IDs and see if that makes a difference. I want to understand what is going on properly here before we land. I'm trying to avoid work this weekend though so I'll do the test on Monday.

Comment 53

[Yuri Z.]

(In reply to Yuri from comment #50)

Since Firefox 67 every different install of Firefox gets its own default profile. Just launch one and it should create a profile. Launching the other should create itself a different profile.

Thanks. Whenever I launch any of the instances, the profile that corresponds to that instance, becomes default (Nightly 73.0a1 (2019-12-07) (64-bit)). Not sure if this is expected behavior.

I tried to reproduce this. Is there a way to start a new instance of a browser from command line, without automatically setting its profile to be default? whenever an instance gets launched with a certain profile, that profile becomes default automatically. Unless I am doing something wrong.

Comment 54

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #53)

(In reply to Yuri from comment #50)

Since Firefox 67 every different install of Firefox gets its own default profile. Just launch one and it should create a profile. Launching the other should create itself a different profile.

Thanks. Whenever I launch any of the instances, the profile that corresponds to that instance, becomes default (Nightly 73.0a1 (2019-12-07) (64-bit)). Not sure if this is expected behavior.

I tried to reproduce this. Is there a way to start a new instance of a browser from command line, without automatically setting its profile to be default? whenever an instance gets launched with a certain profile, that profile becomes default automatically. Unless I am doing something wrong.

Running with -P <profilename> should not set it as the default.

Comment 55

[Yuri Z.]

(In reply to Dave Townsend [:mossop] (he/him) from comment #54)

Running with -P <profilename> should not set it as the default.

Thanks

I'll re-do the test with different app bundle IDs and see if that makes a difference. I want to understand what is going on properly here before we land. I'm trying to avoid work this weekend though so I'll do the test on Monday.

If you do that, the behavior will change to always running the default instance of the browser when you click a link, regardless of whether the profile is default or not.

Comment 56

[Yuri Z.]

Dave, have you had a chance to verify your hypothesis?

A bit more info on the issue.
You can find in what format macOS stores default url handlers, by typing:

defaults read ~/Library/Preferences/com.apple.LaunchServices/com.apple.launchservices.secure.plist

Also, it seems that Launch Services picks an app instance from multiple instances registered in its database (I am not sure what logic it follows. There's some kind of ranking mechanism - but they are left "Default" for all of the instances at the moment) - so even if I make a Nightly.app to be default browser from one location, another instance opens up.

All the .app instances (including their paths and bundle ids) are stored inside lsregister database, and you can view it via
/System/Library/Frameworks/CoreServices.framework/Versions/A/Frameworks/LaunchServices.framework/Versions/A/Support/lsregister -dump

Please correct me if I wrong, but without hacking into the system, at least at this point I am not sure we can influence which instance opens up a url, and the current solution with opening a link in the currently active instance is the best one for now.

Comment 57

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #56)

Dave, have you had a chance to verify your hypothesis?

A bit more info on the issue.
You can find in what format macOS stores default url handlers, by typing:

defaults read ~/Library/Preferences/com.apple.LaunchServices/com.apple.launchservices.secure.plist

Also, it seems that Launch Services picks an app instance from multiple instances registered in its database (I am not sure what logic it follows. There's some kind of ranking mechanism - but they are left "Default" for all of the instances at the moment) - so even if I make a Nightly.app to be default browser from one location, another instance opens up.

Yeah this is a bit frustrating but I guess there is not a lot we can do about it right now and it isn't making things worse in this respect. Let's go ahead and land this as-is and see what the response is.

Do you mind rebasing your patch to a more recent mozilla-central, I think there were a couple of merge issues to be sorted out, then I'll be able to land it.

Comment 58

[Yuri Z.]

Attachment: Bug 1565597 - Mozilla Remoting implementation for macOS, r=mossop

Comment 59

[Yuri Z.]

Rebased to newer mozilla-central. Not sure if it is possible to do it as part of older submission, so I created a new submission. Hope this works. Please let me know if you need anything else from my side.

Comment 60

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #59)

Rebased to newer mozilla-central. Not sure if it is possible to do it as part of older submission, so I created a new submission. Hope this works. Please let me know if you need anything else from my side.

Once a patch has been pushed to phabricator any updates should generally just update that one. Might be that whatever tool you are using isn't correctly picking up the association.

Comment 61

[Yuri Z.]

I see. Thanks for accepting the patch. May I ask what is the process now? Does it still need to go through QA? And when would it merge into Firefox/Thunderbird, if everything is alright with it?

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7c1213a74d28
Mozilla Remoting implementation for macOS, r=mossop

Comment 63

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #61)

I see. Thanks for accepting the patch. May I ask what is the process now? Does it still need to go through QA? And when would it merge into Firefox/Thunderbird, if everything is alright with it?

The process is I've just landed it. Assuming no test failures it will be in a nightly build today or tomorrow. From then it will ride the trains unless issues significant enough to warrant backing out are found.

Thanks for all your hard work here and sorry it took so long.

Comment 64

[Noemi Erli[:noemi_erli]]

Backed out changeset 7c1213a74d28 (Bug 1565597) for causing Linux bustages in RemoteUtils.h CLOSED TREE

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=7c1213a74d2844a2da97228e4660ffd8711b9346&selectedJob=280955444

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280955444&repo=autoland&lineNumber=32909

Backout: https://hg.mozilla.org/integration/autoland/rev/172a5c2edd6e46ad5863ca043d7c6af2bdd92e20

Comment 65

[Yuri Z.]

The compiler doesn't like BuildClassName() not being used by Linux and throws an error. The function is used by Mac and Win, though.
I am going to wrap it in
#if defined(XP_WIN) || defined(XP_DARWIN) #endif
pair if that's ok

Comment 66

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #65)

The compiler doesn't like BuildClassName() not being used by Linux and throws an error. The function is used by Mac and Win, though.
I am going to wrap it in
#if defined(XP_WIN) || defined(XP_DARWIN) #endif
pair if that's ok

That's fine. I think they are identical but XP_MACOSX is more prevalent so please use that. Also I had to fix some clang format issues in the patch before landing so make sure to pick those up too!

Comment 67

[Yuri Z.]

Dave, apologies for a stupid question - but how do I pull the change that you submitted to Phabricator?
is it via moz-phab patch?

Comment 68

[Dave Townsend [:mossop]]

(In reply to Yuri from comment #67)

Dave, apologies for a stupid question - but how do I pull the change that you submitted to Phabricator?
is it via moz-phab patch?

That would work, or you could just run clang-format locally or apply the patch shown on phabricator.

Comment 69

[Yuri Z.]

Thanks. Updated the revision. Also ran a build and a quick check on macOS.

Comment 70

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c836766f6842
Mozilla Remoting implementation for macOS, r=mossop

Comment 71

[Raul Gurzau (:RaulG)]

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

Comment 72

[Anca Soncutean, Desktop QA]

I've managed to reproduce this issue with Firefox 68.0 on macOS 10.14.
The issue is verified fixed with 73.0b12 and 74.0a1 (2020-01-30) on macOS 10.14 and macOS 10.15.