Destroy BrowsingContexts from crashed processes
Categories
(Core :: DOM: Content Processes, defect, P2)
Tracking
()
Tracking | Status | |
---|---|---|
firefox70 | --- | fixed |
People
(Reporter: nika, Assigned: farre)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
When a subframe process crashes, we don't currently detach the browsing contexts related to that subframe from the tree, this can mean that we unfortunately have tree entries with no backing process.
When a process crashes, we should detach all BrowsingContexts which are owned by those processes, and cache their children (as they may still be owned by non-dead browsing contexts)
This can cause parent-process crashes when subframes crash due to interactions with the subframe crash actors.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
Is it really this simple? If we compare with what would happen today if a subframe's process would crash we would unconditionally tear down the entire bc tree. If we detach only the bc (and the subtree rooted at that bc) would we need to unbind all corresponding frame elements as well. I mean the one's in the subtree that wasn't in the process that crashed.
Reporter | ||
Comment 2•5 years ago
|
||
(In reply to Andreas Farre [:farre] from comment #1)
Is it really this simple? If we compare with what would happen today if a subframe's process would crash we would unconditionally tear down the entire bc tree. If we detach only the bc (and the subtree rooted at that bc) would we need to unbind all corresponding frame elements as well. I mean the one's in the subtree that wasn't in the process that crashed.
So all out-of-process subframes will be torn down by the crash killing the BrowserBridgeParent
already, so we don't need to worry about subframes which are not owned by the process which crashed. All frames owned by the process which crashed won't be cleaned up though. We probably want to first cache all of their children, and then trigger a discard on the BrowsingContext
objects which are owned by the process which just crashed.
The <iframe> elements themselves which directly reference the crashed BrowsingContext should be transitioned to point to an in-process process-crashed page, which is what the SubframeProcessCrash code is currently trying to do. We might need to pull that behaviour into the platform layer though.
Assignee | ||
Comment 3•5 years ago
|
||
Right, so a couple of more questions:
Will we be keeping the tree above the crashed bc alive? I can't seem to find SubframeProcessCrash. Won't that be kind of a security leak. An attacking page that embeds a cross origin site in an iframe that it can somehow get to crash could maybe detect this.
Caching, detaching children and all of this sounds very much alike what docshells do when we navigate them. Would it be an idea to instead implement BrowsingContext::Naviagate
, and implement this case by navigating browsing contexts (or really only the browsing contexts highest up in the tree of browsing contexts) in a crashed process to the process-crashed page?
Reporter | ||
Comment 4•5 years ago
|
||
(In reply to Andreas Farre [:farre] from comment #3)
Right, so a couple of more questions:
Will we be keeping the tree above the crashed bc alive? I can't seem to find SubframeProcessCrash. Won't that be kind of a security leak. An attacking page that embeds a cross origin site in an iframe that it can somehow get to crash could maybe detect this.
Current plan was to keep the tree above the bc alive, as it seems not-great to crash an entire tab just because an ad inside of it crashed. Not sure about the security leak potential. Might be worth asking :tjr if it's something worth worrying about, but I'm a bit skeptical right now.
Caching, detaching children and all of this sounds very much alike what docshells do when we navigate them. Would it be an idea to instead implement
BrowsingContext::Naviagate
, and implement this case by navigating browsing contexts (or really only the browsing contexts highest up in the tree of browsing contexts) in a crashed process to the process-crashed page?
Perhaps, that actually is probably the correct behaviour - navigate to the process-crashed page, just without bothering to unload the previous document.
Updated•5 years ago
|
Comment 5•5 years ago
|
||
(In reply to Andreas Farre [:farre] from comment #3)
I can't seem to find SubframeProcessCrash.
The SubframeCrashHandler / SubframeCrashChild JS Window Actor mechanism was added in bug 1533955 - here's the SubframeCrashHandler:
and here's the SubframeCrashChild actor:
Assignee | ||
Comment 6•5 years ago
|
||
Updated•5 years ago
|
Assignee | ||
Comment 7•5 years ago
|
||
Comment 9•5 years ago
|
||
bugherder |
Description
•