Lazily create common sandbox broker policy
Categories
(Core :: Security: Process Sandboxing, enhancement, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox80 | --- | fixed |
People
(Reporter: jld, Assigned: jld)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
The SandboxBrokerPolicyFactory was created as a place to cache the parts of the content sandbox filesystem policy that are shared by all content processes (i.e., don't depend on the pid or if it's a file:/// process, etc.). This was especially important on B2G, when the policy was a simple set of paths (no prefix rules, etc.) and populating it included recursive filesystem traversal on a low-end phone, but there's still work that probably makes sense to cache, especially with Fission increasing the number of content processes.
However, it's initialized at a point before user-changed prefs are available, so pref-derived settings were being recomputed for every process (as seen in bug 1621231). And now bug 1640345 would make a lot more settings pref-derived that currently aren't.
I have patches to make the common policy construction happen lazily the first time a content process policy is requested, and move as much as possible back into it.
|
||
Updated•5 years ago
|
|
Assignee | |
Comment 1•5 years ago
|
||
Not strictly necessary, but I noticed this while I was making changes:
AddDynamicPathList can be a simple static function instead of a private
static method, and doesn't need to be in the header.
|
|
Assignee | |
Comment 2•5 years ago
|
||
When the SandboxBrokerPolicyFactory is constructed, prefs aren't
available, which constrains the cached subset of the content process
policy to entries that don't depend on prefs. Delaying the computation
until a content process is started removes that restriction.
|
|
Assignee | |
Comment 3•5 years ago
|
||
Now that filesystem broker policy entries that depend on prefs can be
cached in the "common" policy object, let's do this wherever possible.
Should also fix bug 1621231.
|
||
Comment 5•5 years ago
|
||
Backed out 4 changesets (Bug 1644917, Bug 1640345) for causing failures in browser_preferences_usage.js CLOSED TREE
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307967950&repo=autoland&lineNumber=7884
Backout: https://hg.mozilla.org/integration/autoland/rev/adc328596e28636b03fabe701ec6a4d07054e5af
|
|
Assignee | |
Comment 6•5 years ago
|
||
I thought I'd run Try on this patch stack but it looks like I never did; sorry about that.
The error message is a little confusing — the test was expecting that certain sandbox prefs would be accessed too much (49-55 times after 50 cross-site navigations in Fission mode, vs. a default limit of 40), but now they're completely absent from the pref stats, because the number of accesses during the test is now zero.
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/517da8786030
https://hg.mozilla.org/mozilla-central/rev/4ae86938c750
https://hg.mozilla.org/mozilla-central/rev/c19de94931de
Description
•