Closed
Bug 1121713
Opened 10 years ago
Closed 10 years ago
[e10s] CompositorParent doesn't seem to clear itself out of LayerTreeState when destroyed
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla38
Tracking | Status | |
---|---|---|
e10s | + | --- |
People
(Reporter: billm, Assigned: billm)
Details
Attachments
(1 file, 1 obsolete file)
1.40 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
I've been trying to debug bug 1120898 and I think I found a problem in how the cross process compositors are managed. The test in that bug opens and closes windows very quickly. Here's what I see happening:
1. Window is closed. The widget is torn down along with the CompositorChild and CompositorParent for that window.
2. The child process sends up a ForceComposite message.
3. The parent receives the ForceComposite message and it's routed to CrossProcessCompositorParent::ForceComposite. That code finds the CompositorParent that was deleted in step 1 and then tries to use it.
I don't see anything that is designed to prevent this from happening. I think the only reason my patch in bug 567058 triggered this is that it made it possible to create windows a little faster in e10s.
Marking s-s in case this also affects b2g.
Assignee | ||
Comment 1•10 years ago
|
||
This fixes the immediate problem. However, I'm a little worried about the other places where we do MOZ_ASSERT(state->mParent). I left them alone, but we might want to do null checks there instead.
Assignee: nobody → wmccloskey
Status: NEW → ASSIGNED
Attachment #8549210 -
Flags: review?(nical.bugzilla)
Comment 2•10 years ago
|
||
Wonder if this is the same root cause as bug 1120331 and bug 1120485, though I appreciate the stacks are not necessarily the same...
Flags: needinfo?(nical.bugzilla)
Comment 3•10 years ago
|
||
Comment on attachment 8549210 [details] [diff] [review]
compositor-fix
Review of attachment 8549210 [details] [diff] [review]:
-----------------------------------------------------------------
Did you attach the correct patch?
Assignee | ||
Comment 4•10 years ago
|
||
No I did not.
Attachment #8549210 -
Attachment is obsolete: true
Attachment #8549210 -
Flags: review?(nical.bugzilla)
Attachment #8549214 -
Flags: review?(nical.bugzilla)
Comment 5•10 years ago
|
||
Comment on attachment 8549214 [details] [diff] [review]
compositor-fix v2
Review of attachment 8549214 [details] [diff] [review]:
-----------------------------------------------------------------
Not sure about the other assertions you mentionned. Ideally we'd not have the possibility to shut a compositor down and still try to send updates to it (rather than null checking when it happens).
Attachment #8549214 -
Flags: review?(nical.bugzilla) → review+
Comment 6•10 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #2)
> Wonder if this is the same root cause as bug 1120331 and bug 1120485, though
> I appreciate the stacks are not necessarily the same...
I think that they are different things.
Flags: needinfo?(nical.bugzilla)
Assignee | ||
Comment 7•10 years ago
|
||
On further thought, b2g doesn't have multiple OS windows, so I don't think this is security sensitive.
Group: core-security
Assignee | ||
Updated•10 years ago
|
tracking-e10s:
--- → +
Assignee | ||
Comment 8•10 years ago
|
||
Ooh, let's backport this as far as possible!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•