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

Expose crash reporter to sandbox #8956

Merged
merged 5 commits into from Mar 27, 2017

Conversation

Projects
None yet
2 participants
@tarruda
Contributor

tarruda commented Mar 17, 2017

Had to make a few extra changes:

  • Exposed more non-sandbox APIs to sandbox. I've only added what is necessary to expose and test crashReporter.
  • Browserify was leaking the require function. Fixed by adding a function parameter with same name which will "catch" the assignment done by browserify(see out/D/gen/js2c/preload_bundle.js).
@tarruda

This comment has been minimized.

Show comment
Hide comment
@tarruda

tarruda Mar 21, 2017

Contributor

I've split indentation in api-crash-reporter-spec.js into a separate commit to make it easier following the changes. If required I can squash into the previous commit.

Contributor

tarruda commented Mar 21, 2017

I've split indentation in api-crash-reporter-spec.js into a separate commit to make it easier following the changes. If required I can squash into the previous commit.

tarruda added some commits Mar 21, 2017

Make sandbox APIs more compatible with normal renderers
- Expose remote shortcuts for the `fs`, `os` and `child_process` modules.
- Expose the `url` and `timers` modules(the browserify versions)
- Add `process.crash` and `process.platform`
Prevent browserify from leaking the require function
Define a "require" argument in the wrapper functions that runs browserify
bundles, which will prevent browserify from leaking the require function.

Note that this doesn't affect the isolated renderer script, only when `-r` flag
is passed to browserify command it will export a require function. It is still
added to isolated renderer script to prevent future mistakes(doesn't hurt
defining a "require" local).
Run the crash reporter specs with sandbox option.
- Create a function that accepts BrowserWindow options and generates a suite
  that contains the renderer-specific tests.
- Run the function twice to execute the tests with and without sandbox option.

@kevinsawicki kevinsawicki self-assigned this Mar 27, 2017

@kevinsawicki kevinsawicki merged commit 6a2cdcf into master Mar 27, 2017

6 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
electron-mas-x64 Build #3700 succeeded in 8 min 17 sec
Details
electron-osx-x64 Build #3701 succeeded in 8 min 50 sec
Details
electron-win-ia32 Build #2704 succeeded in 8 min 16 sec
Details
electron-win-x64 Build #2676 succeeded in 8 min 19 sec
Details

@kevinsawicki kevinsawicki deleted the expose-crash-reporter-to-sandbox branch Mar 27, 2017

@kevinsawicki

This comment has been minimized.

Show comment
Hide comment
@kevinsawicki

kevinsawicki Mar 27, 2017

Contributor

Thanks for this 👍 ⚡️ 💥

Contributor

kevinsawicki commented Mar 27, 2017

Thanks for this 👍 ⚡️ 💥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment