New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: set macOS crypto keychain name earlier #34683
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks correct to me.
I'm sure it goes without saying, but one point of caution with this change is that anyone who depended on the previous behavior (however erroneous) will now get errors following this patch. |
We should probably also add the test mentioned in #34614 (comment), to make sure that this never regresses in the future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this change is causing a test failure on arm64 macs, e.g:
aceb554
to
905b970
Compare
Release Notes Persisted
|
I was unable to backport this PR to "18-x-y" cleanly; |
I have automatically backported this PR to "19-x-y", please check out #35795 |
I have automatically backported this PR to "20-x-y", please check out #35796 |
* fix: set macOS crypto keychain name earlier * spec: ensure arm64 mac tests are cleaned up
Fixes #34614
Notes: Usage of
safeStorage
now consistently uses the correct service name on macOS regardless of timing with browser window construction