Use different key name for brave channel name in MacOS

[simonhong]

After recent chromium bumping, KSChannelID is disappeared.
This is strange because we add that key by brave_app_plist.
I suspect chromium code could remove that key from Info.plist because
we use same key name with chrome's KeystoneRegistration.framework and chrome_app_plist
target has code that removes that key from Info.plist.
In theory, I think chrome_app_plist could not remove it because chrome_app_list
is evaluated first and then brave_app_plist is evaluated.

This PR is workaround by using another key name and I think using different key name is more safer.

Fix brave/brave-browser#4753

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Verified that these changes build without errors on
  • Windows
  • macOS
  • Linux
• Verified that these changes pass automated tests (npm test brave_unit_tests && npm test brave_browser_tests && npm run test-security) on
  • Windows
  • macOS
  • Linux
• Verified that all lint errors/warnings are resolved (npm run lint)
• Ran git rebase master (if needed).
• Ran git rebase -i to squash commits (if needed).
• Tagged reviewers and labelled the pull request as needed.
• Request a security/privacy review as needed.
• Add appropriate QA labels (QA/Yes or QA/No) to include the closed issue in milestone
• Public documentation has been updated as necessary. For instance:
  • https://github.com/brave/brave-browser/wiki/Deviations-from-Chromium-(features-we-disable-or-remove)
  • https://github.com/brave/brave-browser/wiki/Proxy-redirected-URLs
  • https://github.com/brave/brave-browser/wiki/Fingerprinting-Protection-Mode
  • https://github.com/brave/brave-browser/wiki/Brave%E2%80%99s-Use-of-Referral-Codes
  • https://github.com/brave/brave-browser/wiki/Custom-Headers
  • https://github.com/brave/brave-browser/wiki/Web-compatibility-issues-with-tracking-protection
  • https://github.com/brave/brave-browser/wiki/QA-Guide

Test Plan:

Reviewer Checklist:

• New files have MPL-2.0 license header.
• Request a security/privacy review as needed.
• Adequate test coverage exists to prevent regressions
• Verify test plan is specified in PR before merging to source

After-merge Checklist:

• The associated issue milestone is set to the smallest version that the
  changes has landed on.
• All relevant documentation has been updated.

[simonhong]

Tested with dmg from this PR's workspace(https://staging.ci.brave.com/job/brave-browser-build-pr/job/brave_channel_id_macos/1/execution/node/60/ws/src/out/Release/unsigned_dmg/Brave%20Browser%20Nightly.dmg) and it displays nightly properly.

[mkarolin]

@simonhong, I wonder if this is related to https://cs.chromium.org/chromium/src/chrome/installer/mac/keystone_install.sh.

In chrome/installer/mac/BUILD.gn it's being included only for chrome branded builds:
if (is_chrome_branded) { sources += [ "keystone_install.sh" ] }

which is why in the new signing script I patched it out:
https://github.com/brave/brave-core/blob/master/patches/chrome-installer-mac-signing-pipeline.py.patch, but perhaps we need it as well. Though I didn't see us patch keystone_isntall.sh before and it seems pretty specific to Chrome.

[simonhong]

@mkarolin I think keystone_install.sh is not related with this. It doesn't touch KSChannelID.

I found one place that can remove KSChannelID - https://cs.chromium.org/chromium/src/chrome/installer/mac/signing/modification.py?type=cs&q=KSChannelID&sq=package:chromium&g=0&l=62
this modification.py acts during our mac signing?

However, still curious because above unsigned dmg I downloaded from CI also doesn't have KSChannelID key in Info.plist.

[mkarolin]

@simonhong, you are correct, @bsclifton and I have been looking into keystone_install.sh and it appears to be unrelated (and not needed for us).

I have been looking into KSChannelID and I see our tweak being called in BUILD.gn and is writing out in src/out/Release/gen/brave/brave_app_plist_tweaked.plist with the correct KSChannelID. I haven't traced what happens to it after that yet.

[simonhong]

Do you agree using our own key name is more safer to avoid conflicting with chromium's?

[bsclifton]

This is interesting and the fix looks good-
Checking what the behavior is on Windows...

[simonhong]

@bsclifton This is not related with Windows. only changed bundle's Info.plist.

[mkarolin]

I think it's fine since there are references in the code that only "keystone enabled" builds should have KS channel and they seem to be putting all related code under #if defined(GOOGLE_CHROME_BUILD).

I also verified that modification.py is removing KSChannelID because the config that I created in brave/build/mac/sign_brave.py doesn't have a distribution with channel set. We'd have to add something like

@property
  def distributions(self):
         return [model.Distribution("channel_name")]

to keep KSChannelD.

[bsclifton]

@mkarolin ah - nice to know the root cause 😄
@simonhong yep- definitely know this is not impacting Windows, but was curious how that code path looked. I hadn't really noticed this before! 😄

[bsclifton]

LGTM! 😄

[simonhong]

Many thanks for checking this together @bsclifton @mkarolin !!

[mkarolin]

Thanks for figuring it all out, @simonhong. Great insight!

[bsclifton]

@simonhong as you mentioned yesterday, I think we'll need to uplift back to release channel - can you create those? 😄

[simonhong]

@bsclifton Done. sir!

[bridiver]

This change is not correct and breaks crash reporting because all channels will be reported as release. I'm working on a fix right now to go back to KSChannelID