Bug 80339 – [chromium] Notify CCLayerImpl tree of context loss and restoration

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Kenneth Russell]

There are some situations where CCLayerImpls don't use managed textures, typically because doing so would either be unwieldy or not really correct from an accounting standpoint. CCPluginLayerImpl, which is now used to render Core Animation plugins on Mac OS, is one example. http://crbug.com/113125 describes a situation where forcing loss of context while Flash is rendering causes bad breakage because the CCPluginLayerImpl continues to reference a stale texture ID. After offline discussion it sounds like the best solution is to give CCLayerImpls the possibility of explicitly responding to context loss and recreation.

[Kenneth Russell]

Created attachment 130241 [details]
Patch

[James Robinson]

Comment on attachment 130241 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130241&action=review

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:712
> +    for (size_t i = 0; i < current->children().size(); ++i)

this won't hit mask and replica layers - is that OK (i.e. can plugins ever be masks or reflections or inside masks or reflections?). you can hit these by recurring on the mask/replica pointers in addition to children

we also have iterator types in CCLayerIterator.h. They are more complicated, but if you use them you don't need to worry about exactly how the tree structure is organized, you can just use the iterator and rely on it to hit everything

[Dana Jansens]

Comment on attachment 130241 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130241&action=review

>> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:712
>> +    for (size_t i = 0; i < current->children().size(); ++i)
> 
> this won't hit mask and replica layers - is that OK (i.e. can plugins ever be masks or reflections or inside masks or reflections?). you can hit these by recurring on the mask/replica pointers in addition to children
> 
> we also have iterator types in CCLayerIterator.h. They are more complicated, but if you use them you don't need to worry about exactly how the tree structure is organized, you can just use the iterator and rely on it to hit everything

The iterators need a Surface-Layer list from calcDrawTransformsEtc, so I'm not sure it's relevant here. This looks like a simple layer-tree walk.

[Kenneth Russell]

(In reply to comment #3)
> (From update of attachment 130241 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130241&action=review
> 
> >> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:712
> >> +    for (size_t i = 0; i < current->children().size(); ++i)
> > 
> > this won't hit mask and replica layers - is that OK (i.e. can plugins ever be masks or reflections or inside masks or reflections?). you can hit these by recurring on the mask/replica pointers in addition to children
> > 
> > we also have iterator types in CCLayerIterator.h. They are more complicated, but if you use them you don't need to worry about exactly how the tree structure is organized, you can just use the iterator and rely on it to hit everything
> 
> The iterators need a Surface-Layer list from calcDrawTransformsEtc, so I'm not sure it's relevant here. This looks like a simple layer-tree walk.

Yes, looking at the uses of CCLayerIterator it looks like some other entity is responsible for building up the LayerList through which it iterates. I think this patch is what we want.

[James Robinson]

Comment on attachment 130241 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=130241&action=review

>>>> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:712
>>>> +    for (size_t i = 0; i < current->children().size(); ++i)
>>> 
>>> this won't hit mask and replica layers - is that OK (i.e. can plugins ever be masks or reflections or inside masks or reflections?). you can hit these by recurring on the mask/replica pointers in addition to children
>>> 
>>> we also have iterator types in CCLayerIterator.h. They are more complicated, but if you use them you don't need to worry about exactly how the tree structure is organized, you can just use the iterator and rely on it to hit everything
>> 
>> The iterators need a Surface-Layer list from calcDrawTransformsEtc, so I'm not sure it's relevant here. This looks like a simple layer-tree walk.
> 
> Yes, looking at the uses of CCLayerIterator it looks like some other entity is responsible for building up the LayerList through which it iterates. I think this patch is what we want.

It's what you want if it's OK to skip masks and replicas (since they aren't in the children list) - is that the desired behavior or not?

[Kenneth Russell]

(In reply to comment #5)
> (From update of attachment 130241 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130241&action=review
> 
> >>>> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:712
> >>>> +    for (size_t i = 0; i < current->children().size(); ++i)
> >>> 
> >>> this won't hit mask and replica layers - is that OK (i.e. can plugins ever be masks or reflections or inside masks or reflections?). you can hit these by recurring on the mask/replica pointers in addition to children
> >>> 
> >>> we also have iterator types in CCLayerIterator.h. They are more complicated, but if you use them you don't need to worry about exactly how the tree structure is organized, you can just use the iterator and rely on it to hit everything
> >> 
> >> The iterators need a Surface-Layer list from calcDrawTransformsEtc, so I'm not sure it's relevant here. This looks like a simple layer-tree walk.
> > 
> > Yes, looking at the uses of CCLayerIterator it looks like some other entity is responsible for building up the LayerList through which it iterates. I think this patch is what we want.
> 
> It's what you want if it's OK to skip masks and replicas (since they aren't in the children list) - is that the desired behavior or not?

Per our offline discussion it's a fair point that the recursion loop should probably traverse mask and replica layers. I was mainly talking about the complexity of building up a LayerList. New patch coming up.

[Kenneth Russell]

Created attachment 130261 [details]
Patch

[James Robinson]

Comment on attachment 130261 [details]
Patch

R=me

[WebKit Review Bot]

Comment on attachment 130261 [details]
Patch

Clearing flags on attachment: 130261

Committed r109839: <http://trac.webkit.org/changeset/109839>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.