fix: use xib file to construct macOS Menu

[codebytere]

Description of Change

Resolves #20503.

When we removed the .xib file menu construction, it unfortunately nuked the recentDocuments role, which unexpectedly is near-impossible to replicate manually with obj-c without doing terrifying hacky things or using private APIs (see this blog post). As a result, our only real path forward here was to revert to xib-based functionality removed originally removed in this commit. I needed to re-implement the xib template in GN removed upstream but otherwise I mostly just needed to re-add the xib file and revert to the older recentDocuments menu role business logic in the menu_controller.

Tested to work with this repro case.

cc @MarshallOfSound @nornagon (& @bigtimebuddy!)

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• PR title follows semantic commit guidelines
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Fixed a regression in the recentDocuments role on macOS.

[deepak1556]

Not a review but some thoughts , the reason chrome is doing this refactor is they are trying to remove dependency on system XCode and make builds deterministic https://bugs.chromium.org/p/chromium/issues/detail?id=330262, which again aligns with one of our upcoming projects from security WG , making electron builds deterministic.

Once this transition is complete on chrome end and we still deviate from it then its going to be a technical debt similar to libc++ one which I would like to avoid at all cost. So reiterating on this PR, I would suggest to look into the programmatic implementation or revisit the position of this api through the API WG.

[nornagon]

Thanks for the context @deepak1556! Since this is just a dependency on ibtool, not a dependency on any compiler/linker tool from Xcode, do you think it would be OK? A quick test on my machine produced identical output from multiple runs of ibtool on identical inputs, showing that it's at least timestamp-independent. I also tested running in a different directory (different pwd, input and output paths) and got identical bytes out of ibtool.

In general I agree with wanting to drop Xcode as a dependency, and aligning with Chromium, but this doesn't seem like it would impact our ability to produce deterministic builds. Additionally, doing this programmatically requires calling private APIs which may or may not be rejected by MAS.

[codebytere]

Additionally, doing this programmatically requires calling private APIs which may or may not be rejected by MAS.

This is also my primary consideration here; i'd rather have the system handle this where possible and not endanger users' ability to ship via the mac app store.

At the very least, i'd like to get this into versions where it's supposed to work and then perhaps consider deprecating it in future versions when Chromium does totally drop Xcode 🤔

[deepak1556]

Sorry for the delayed response, comments inline.

Since this is just a dependency on ibtool, not a dependency on any compiler/linker tool from Xcode, do you think it would be OK?

Yeah dependency on ibtool is fine, it used to be non-deterministic in the past. I was originally worried about a situation when chromium gets rid of all its xib files, they would start shipping the minimal set of binaries and SDK for the build. We would have to rely on the system installation for this one, but since it doesn't change the determinism of the build, should be ok!

Additionally, doing this programmatically requires calling private APIs which may or may not be rejected by MAS.

Moving forward, implementations that require this should be considered by the API WG and see if it can be made as a userland module or expose the right parts to make it possible.

[codebytere]

Moving forward, implementations that require this should be considered by the API WG and see if it can be made as a userland module or expose the right parts to make it possible.

@deepak1556 do you wish for the API group to vote on this PR, or are you comfortable with this PR as it stands and wish for us to vote on future PRs affecting similar build components?

[deepak1556]

@codebytere sorry my comment wasn't clear, I was talking about future PR's . This one is good as it stands 👍

[nornagon]

lgtm with nits

[deepak1556]

LGTM

[release-clerk]

Release Notes Persisted

Fixed a regression in the recentDocuments role on macOS.

[trop]

I was unable to backport this PR to "6-1-x" cleanly;
you will need to perform this backport manually.

[trop]

I have automatically backported this PR to "7-0-x", please check out #20670

[bigtimebuddy]

Could someone manually back-port this to 6.1.x? Doesn't look like it showed up in the latest release.

[trop]

@codebytere has manually backported this PR to "6-1-x", please check out #20885