839538 - [layers-refactoring] sort out the tear-down sequence

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Nicolas Silva [:nical]]

Getting the shut-down sequence of the new layers system right represents some work so I file this bug to identify it as a task in the list of things to do before landing.

Comment 1

[Benoit Girard (:BenWa)]

Can you elaborate more on what needs to be resolved for the tear down sequence? I did a quick test with an optimized build on mac and didn't see any problems. I'm going to run again with Debug tomorrow.

Comment 2

[Nicolas Silva [:nical]]

One of the first things is to verify that we don't leak anything. Since a lot of stuff has changed and in particular the ownership model, I would not be surprised that we didn't get everything right for cleanup (since most of the focus has been on drawing things but less in cleaning them up).

Another important point is that we need to implement something like CleanupResources and SetCompositor for all CompositableHosts and TextureHosts, because with async textures, we may tear down a layer tree while keeping alive a compositable and its textures, and then attach this Compositable to another layer tree, using another compositor (and another gl context), for example when we detach a tab with a video playing and drag-n-drop it into another window.

So what need is to recursively cleanup all the resources in all the compositors pointed to by the layer tree, including nullifying all references to the compositor.
Then, when the compositable will be attach to another layer tree, we receive on the compositor side a OpAttachCompositable message, and at this place we want to do recursively set the compositor on the Compositable host and its textures.

So it's good to keep in mind that the lifetime of Compositable/Texture is bound to their IPDL protocol, and not their layer. In the most common scenario we shut down the Shadowablelayer and its compositableClient roughly at the same time so they get destroyed mor or less at the same time as well on the compositor side, but if the compositable is using the ImageBridge protocol, it will outlive the its layer and maybe get attached to another layer at some point.

There's also the "two-steps"-ness of the sequence that probably suffered from things having been moved around quite a bit. I don't know what's the status there (reminder: the constraint is that when ~nsBaseWidget ends, all the gl resources on the compositor side must be already released, and the beginning of ~nsBaseWidget is what initiates the sequence, so we do a bunch of sync messages in there to let the system clean things safely).

Since the tear-down sequence is usually a bit tricky to get right, i'd be surprised that we did not break or leak anything in that area while refactoring without focusing on it. That said there may not be much to do and we just got it mostly right.

Comment 3

[Milan Sreckovic [:milan] (needinfo for best results)]

BenWa is now looking at this.

Comment 4

[Benoit Girard (:BenWa)]

I broke a cycle that was causing severe leaks:
http://hg.mozilla.org/projects/graphics/rev/2ae1ec52e943

Comment 5

[Nicolas Silva [:nical]]

This bug was about the tear-down related work that needed to be done before landing. Let's close it now that the refactoring has landed and file new bugs if we run into regressions in the tear-down sequence.