Skip to content
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

chore: enable v2 sandbox on mac #15647

Merged
merged 2 commits into from Dec 13, 2018
Merged

chore: enable v2 sandbox on mac #15647

merged 2 commits into from Dec 13, 2018

Conversation

nornagon
Copy link
Member

@nornagon nornagon commented Nov 8, 2018

Description of Change

This re-enables the v2 sandbox on mac (disabled by 878c5b6). That commit asserts that the v2 sandbox would prevent the mixed-sandbox mode from working, but in my testing with this change, it seems to work fine.

$ ps aux | grep Electron | grep type=renderer
# (abridged)
33083 .../Electron Helper --type=renderer [...] --no-sandbox
33082 .../Electron Helper --type=renderer [...] --enable-sandbox --seatbelt-client=53

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • tests are changed or added
  • relevant documentation is changed or added
  • PR title follows semantic commit guidelines

Release Notes

Notes:

@nornagon nornagon requested review from zcbenz, a team and tarruda November 8, 2018 22:51
@nornagon
Copy link
Member Author

marking as WIP because this has exposed some failures on linux and some tests that expect the sandbox option to do weird things like loading the sandboxed renderer client without actually sandboxing the renderer.

codebytere
codebytere previously approved these changes Nov 28, 2018
@codebytere
Copy link
Member

ah dangit i didn't intend to approve this yet, ignore the above 🤦‍♀️

@nornagon
Copy link
Member Author

nornagon commented Dec 3, 2018

Rebased on top of #15894

@deepak1556
Copy link
Member

Upstream is trying to remove the kMacV2Sandbox feature flag soon, they tried it in 71 but due to some failures its fortunately delayed 5406108

@nornagon
Copy link
Member Author

nornagon commented Dec 7, 2018

Pretty sure I can update & land this before 72. Thanks for the info.

@nornagon nornagon force-pushed the v2-sandbox branch 2 times, most recently from dfc2db0 to a0d485e Compare December 10, 2018 22:42
@nornagon
Copy link
Member Author

I believe the mac and arm failures are flakes, see #16027

@nornagon nornagon removed the wip ⚒ label Dec 12, 2018
Copy link
Member

@deepak1556 deepak1556 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@release-clerk
Copy link

release-clerk bot commented Dec 13, 2018

Release Notes Persisted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants