fix: support chrome extensions in sandboxed renderer #16218
Conversation
|
💖 Thanks for opening this pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix. Examples of commit messages with semantic prefixes:
Things that will help get your PR across the finish line:
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can. |
5f700df to
d70ccaf
Compare
f666408 to
da677d3
Compare
|
Currently blocked by #16746 for writing tests. |
|
Builds seem to be failing due to unrelated tests. The added content script tests run and pass though. |
They won't be unrelated, the same 13 tests fail on all 3 platforms. My guess is you've included a file (content-scripts-injector) in the sandbox bundle that includes files further down the chain that aren't sandbox compatible. Can you look into those failures? |
@MarshallOfSound This might be accurate and #16686 has potential to fix this. |
cad1ba0 to
3deed89
Compare
| } catch (error) { | ||
| // TODO(samuelmaddock): Run scripts in isolated world, see chromium script_injection.cc | ||
| console.error(`Error running content script JavaScript for '${extensionId}'`) 10000 | ||
| console.error(error) |
There was a problem hiding this comment.
An error will be thrown when a CSP is set with 'unsafe-eval'. runInThisContext is a wrapper around eval.
3be93e6 to
78a2d75
Compare
|
Can you rebase on |
Since we're keeping track of which frames we've injected the bundle into, this logic is redundant.
This can occur in a rendered sandbox when a CSP is enabled. We'll need to switch to using isolated worlds to fix this.
78a2d75 to
2fdc8cf
Compare
|
@MarshallOfSound if you want to merge this, i can go ahead and integrate the |
Sounds good to me 👍 |
|
Release Notes Persisted
|
* Add content script injector to sandboxed renderer * Fix 'getRenderProcessPreferences' binding to the wrong object * Pass getRenderProcessPreferences to content-scripts-injector * Emit document-start and document-end events in sandboxed renderer * Use GetContext from RendererClientBase * Prevent script context crash caused by lazily initialization * Remove frame filtering logic for onExit callback Since we're keeping track of which frames we've injected the bundle into, this logic is redundant. * Add initial content script tests * Add contextIsolation variants to content script tests * Add set include * Fix already loaded extension error * Add tests for content scripts 'run_at' options * Catch script injection eval error when CSP forbids it This can occur in a rendered sandbox when a CSP is enabled. We'll need to switch to using isolated worlds to fix this. * Fix content script tests not properly cleaning up extensions * Fix lint and type errors
Description of Change
The sandboxed renderer doesn't currently load any content scripts included with chrome extensions. A few changes were needed to fix this:
content-script-injector.jstolib/sandboxed_renderer/init.jsgetRenderProcessPreferencesincontent-script-injector.js(not bound toprocessin sandboxed lib).document-startanddocument-endevents in sandboxed renderer.Content scripts don't currently work in subframes when context isolation is enabled. #16804 will need to be fixed first.
Checklist
npm testpassesRelease Notes
Notes: Fixed Chrome extension content scripts not loading in sandboxed renderer.