Firefox does not create a separate dedicated profile when launched from another install location on macOS
Categories
(Toolkit :: Startup and Profile System, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox66 | --- | unaffected |
firefox67 | --- | unaffected |
firefox68 | + | verified |
People
(Reporter: cbaica, Assigned: mozilla)
References
(Regression)
Details
(Keywords: regression)
Attachments
(1 file, 1 obsolete file)
4.21 KB,
patch
|
spohl
:
review+
|
Details | Diff | Splinter Review |
[Affected versions]:
- Fx 68.0a1
- Fx67.0b8
[Unaffected versions]:
- Fx67.0b7
[Affected platforms]:
- macOS
[Steps to reproduce]:
- Install Firefox in 2 different locations.
- Launch Firefox from one of the locations and check the about:profiles section.
- Launch Firefox from the second location and check the about:profiles section.
[Expected result]:
- Each instance has a separate profile created and in use.
[Actual result]:
- Both instances use the same profile.
[Regression range]:
- Last good build: 28-03-2019
- First bad build: 29-03-2019
This was the best we could narrow it down so far, due to mozregression not being too useful in this case.
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Using -no-remote makes it work, this is probably caused by bug 469990.
Comment 2•6 years ago
|
||
As described, this now matches the behavior on Windows and Linux and is therefore expected. Is there another reason why this was filed as a bug aside from the fact that the new behavior is different from past behavior?
Comment 3•6 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #2)
As described, this now matches the behavior on Windows and Linux and is therefore expected. Is there another reason why this was filed as a bug aside from the fact that the new behavior is different from past behavior?
Windows and Linux do not need -no-remote in order to launch a second instance.
Comment 4•6 years ago
|
||
I may have misunderstood the bug description. When the second instance launches, is this actually a separate Firefox process, running the executable in the second location? Or does this merely focus and launch a new window in the first Firefox process?
Comment 5•6 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #4)
I may have misunderstood the bug description. When the second instance launches, is this actually a separate Firefox process, running the executable in the second location? Or does this merely focus and launch a new window in the first Firefox process?
We can't run two instances with the same profile so I was assuming the latter, and this is what I see locally.
Reporter | ||
Comment 6•6 years ago
•
|
||
(In reply to Dave Townsend [:mossop] (he/him) from comment #5)
We can't run two instances with the same profile so I was assuming the latter, and this is what I see locally.
You are correct. As mentioned in the 'Actual result' part, the instances running with the same profile. On windows an linux this issue is not occurring, meaning that when I open Firefox from 2 different locations (just by double clicking) 2 different profiles are created and in use.
Comment 7•6 years ago
|
||
Yuri, is this something that you could work on? This boils down to the fact that if an instance is launched from a different path, it will be running in a separate profile and should therefore be allowed to launch as a separate instance.
Comment 8•6 years ago
|
||
Cristian, I just spotted that you mentioned that this reproduces in 67.0b8, but bug 469990 hasn't landed on 67 that I can see. Can you confirm that this reproduces there?
Comment 9•6 years ago
|
||
Hello Dave!
I tested the both the builds with the code from bug 469990 and without it(https://hg.mozilla.org/mozilla-central/rev/b1374f2163b6), I could not reproduce this issue.
When I tested the builds with/without the code from bug 1539839 (https://hg.mozilla.org/mozilla-central/rev/46b9df4a18e8), I noticed that builds without the code could not reproduce this issue, however builds with the code can reproduce this issue.
This leads me to believe that the regressor is indeed bug 1539839.
Thank you!
Comment 10•6 years ago
|
||
Thanks for isolating the issue to the correct bug!
Comment 11•6 years ago
|
||
(In reply to Cristian Comorasu [:ccomorasu], Release Desktop QA from comment #9)
This leads me to believe that the regressor is indeed bug 1539839.
Odd. Bug 1539839 should only affect builds run with MOZ_PROFILER_STARTUP set in the environment. I made a build locally with bug 1539839 backed out and could still reproduce this issue. Bug 1539839 is also not in 67 so if this actually is reproducible on 67 then something weird is going on.
Comment 12•6 years ago
|
||
So I did some tests locally and got different results to Cristian. I'm still of the opinion that this is caused by bug 469990.
First I tested two installs of Firefox 67.0b8 and could not reproduce it in any way. Given that the bugs we're discussing are not on 67 that makes sense so I think we can say this isn't happening on 67 at this point.
I made a local build of the changeset that bug 1539839 landed in and the previous changeset (in both cases plus a backout of bug 1528963 as that was busting the build at this point). I installed both builds twice. Running those gave me the following results (pre means the build from before bug 1539839 and post means the build with bug 1539839):
Run pre first and then run second install of pre: Second instance created a new profile for use.
Run pre first and then run post: Second instance created a new profile for use.
Run post first and then run pre: Second instance created a new profile for use.
Run post first and then run second install of post: Second instance created a new profile for use.
This makes sense to me since bug 1539839 should not affect builds run normally.
Then I made similar builds for the changeset where bug 469990 landed and the previous changeset. Running those gave me these results:
Run pre first and then run second install of pre: Second instance created a new profile for use.
Run pre first and then run post: Nothing seemed to happen, no new Firefox window opened, no new profile created.
Run post first and then run pre: Second instance created a new profile for use.
Run post first and then run second install of post: A new window opens in the existing instance.
Then I made builds of current inbound and inbound plus backouts of bug 469990 and bug 1540821 Running those gave me these results:
Run inbound first and then run a second install of inbound: A new window opens in the existing instance.
Run inbound first and then run after backout: Second instance created a new profile for use.
Run after backout and then run inbound: Nothing seemed to happen, no new Firefox window opened, no new profile created.
Run after backout and then a second install of after backout: Second instance created a new profile for use.
Assignee | ||
Comment 13•6 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #7)
Yuri, is this something that you could work on? This boils down to the fact that if an instance is launched from a different path, it will be running in a separate profile and should therefore be allowed to launch as a separate instance.
Attached is the patch that takes into account paths of the processes: the no-remote logic is applied only if the bundle paths of the caller and the callee match.
Comment 14•6 years ago
|
||
Assignee | ||
Comment 15•6 years ago
|
||
Thanks Stephen.
Note, that there must be exactly at least two running instances at the same path and same bundle id to set runningInstanceFound to YES. One of them will be an already existing process, the other one is the current process.
Comment 16•6 years ago
|
||
(In reply to Yuri from comment #15)
Thanks Stephen.
Note, that there must be exactly at least two running instances at the same path and same bundle id to set runningInstanceFound to YES. One of them will be an already existing process, the other one is the current process.
Oof, you're right! Thanks for catching this. Will update the patch accordingly.
Assignee | ||
Comment 17•6 years ago
|
||
And actually now that the paths must match, we don't have to check for bundle ids matches (this was added initially to prevent Thunderbird/Firefox interference). But having this check in the patch doesn't hurt either - in fact, in unlikely case the requirements change back to not expect the paths to match we will have this already working (just one extra check will have to be removed).
Comment 18•6 years ago
|
||
[Tracking Requested - why for this release]:
This breaks our dedicated profiles story.
Updated•6 years ago
|
Comment 19•6 years ago
|
||
I changed this to check for the bundle path AND that the current NSRunningApplication is different, which is essentially saying that there need to be at least two running instances. Yuri, can you take a look and confirm that this addresses your comment 15 and that we're good to go here?
Updated•6 years ago
|
Assignee | ||
Comment 20•6 years ago
|
||
Hi Stephen, Sorry for late reply.
That should be fine, provided [NSRunningApplication hash] (and, consequently, isEqual:) behaves as we would expect it to (I didn't find in documentation what is actually being hashed) - are you sure this is the case?
Otherwise isEqual simply compares object pointers, and this is not what we want (we can't expect the framework to give us the same object at the same address even if two NSRunningApplications actually represent the same app).
Comment 21•6 years ago
|
||
(In reply to Yuri from comment #20)
Hi Stephen, Sorry for late reply.
That should be fine, provided [NSRunningApplication hash] (and, consequently, isEqual:) behaves as we would expect it to (I didn't find in documentation what is actually being hashed) - are you sure this is the case?
Otherwise isEqual simply compares object pointers, and this is not what we want (we can't expect the framework to give us the same object at the same address even if two NSRunningApplications actually represent the same app).
Usually, I would have gone ahead and compared pids. The reason why I picked isEqual:
instead of pids is because Apple's documentation discourages the direct comparison of pids and recommends the use of isEqual:
. See https://developer.apple.com/documentation/appkit/nsrunningapplication/1526998-processidentifier?language=objc
Does this address your concerns?
Assignee | ||
Comment 22•6 years ago
|
||
(In reply to Stephen A Pohl [:spohl] from comment #21)
Usually, I would have gone ahead and compared pids. The reason why I picked
isEqual:
instead of pids is because Apple's documentation discourages the direct comparison of pids and recommends the use ofisEqual:
. See https://developer.apple.com/documentation/appkit/nsrunningapplication/1526998-processidentifier?language=objcDoes this address your concerns?
Yes. Thanks for the link!
Comment 23•6 years ago
|
||
Comment 24•6 years ago
|
||
Setting n-i to get independent confirmation that this is indeed fixed. The fix should be in the next Nightly.
Comment 25•6 years ago
|
||
bugherder |
Reporter | ||
Comment 26•6 years ago
•
|
||
Sorry for the late response.
The issue is verified fixed on the latest Fx68.0a1 using macOS 10.13. Firefox starts with different profiles when launched from different locations.
Updated•6 years ago
|
Description
•