Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2272703005:
Mac Keychain store name should be based on branding. (Closed)
DescriptionMac Keychain store name should be based on branding.
Committed: https://crrev.com/56e79bb7360d551d1ccdd3bcf527e4c1f825ea1d
Cr-Commit-Position: refs/heads/master@{#415575}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 19 (7 generated)
thestig@chromium.org changed reviewers: + cfroussios@chromium.org, mark@chromium.org, vabr@chromium.org
The ifdef was added a while back in https://codereview.chromium.org/200623002, but there was another case of OFFICIAL_BUILD vs GOOGLE_CHROME_BUILD confusion. Do we care about potential Mac Chromium users who built their own "official" Chromium binaries?
The CQ bit was checked by thestig@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM. The behavior change for “unbranded official” or “branded unofficial” is no big deal.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Not that you need my LGTM, but you have it, thanks for fixing this. Unlike the recent KDE case, this one is indeed even less of a concern, given the absence of different distros in the Mac environment compared to GNU/Linux. Cheers, Vaclav
It's not clear to me why this is a smaller issue than the KDE case. A key argument there was that the bug hadn't made it to stable. Here, it's definitely in stable. Judging by the code (I don't have a mac to verify this), the changed strings are used to find passwords. The only way this will have no impact is if there is no Chromium on mac that is build with the official flag. https://codereview.chromium.org/2272703005/diff/1/components/os_crypt/keychai... File components/os_crypt/keychain_password_mac.mm (right): https://codereview.chromium.org/2272703005/diff/1/components/os_crypt/keychai... components/os_crypt/keychain_password_mac.mm:66: service_name.data(), This won't find the same password.
+Vasilii FYI, because unlike on Linux, on Mac passwords are encrypted via os_crypt. If this lands and we see issues reported about passwords inaccessible, asking about the exact Chromium build (branded? optimised?) might help. Cheers, Vaclav
On 2016/08/25 09:41:19, cfroussios wrote: > It's not clear to me why this is a smaller issue than the KDE case. A key > argument there was that the bug hadn't made it to stable. Here, it's definitely > in stable. My guess is: unlike Linux, where distros package Chromium and there is a user base, there's no one packaging and distributing Chromium on Mac.
On 2016/08/26 09:31:00, Lei Zhang wrote: > My guess is: unlike Linux, where distros package Chromium and there is a user > base, there's no one packaging and distributing Chromium on Mac. I found this, which doesn't look bogus. http://www.macupdate.com/app/mac/36244/chromium The interest for Chromium on Mac is small, but not exactly non-existent.
On 2016/08/26 10:53:30, cfroussios wrote: > On 2016/08/26 09:31:00, Lei Zhang wrote: > > My guess is: unlike Linux, where distros package Chromium and there is a user > > base, there's no one packaging and distributing Chromium on Mac. > > I found this, which doesn't look bogus. > http://www.macupdate.com/app/mac/36244/chromium > The interest for Chromium on Mac is small, but not exactly non-existent. Ok, but do they build with the official flag? I guess one can download it and find out, but unlike Linux distros which have some trust because they also made the OS some of us are running, I'm not sure I trust some random Chromium binary.
I maintain that we don’t care either way.
The CQ bit was checked by thestig@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Mac Keychain store name should be based on branding. ========== to ========== Mac Keychain store name should be based on branding. Committed: https://crrev.com/56e79bb7360d551d1ccdd3bcf527e4c1f825ea1d Cr-Commit-Position: refs/heads/master@{#415575} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/56e79bb7360d551d1ccdd3bcf527e4c1f825ea1d Cr-Commit-Position: refs/heads/master@{#415575} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
