Bug 76675 – [chromium] Remove setLayerTreeHost nonsense on lost context

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[James Robinson]

[chromium] Remove setLayerTreeHost nonsense on lost context

[James Robinson]

Created attachment 123226 [details]
Patch

[James Robinson]

Pretty simple.  This codepath used to do more back when layers managed their own textures, but since everything either uses managed textures (content/image/video layers) or has its own context lost management (webgl/plugin) we don't have to do anything special when losing a context.

setLayerTreeHost() is now only called when:
*) A layer enters a tree
*) A layer leaves a tree
*) A layer moves from one tree to another (for example moves from being inside a tab to a window.open()'d popup)

[James Robinson]

Tested by:

platform/chromium/compositing/*lost-context*

manually opening poster circle, a page with html5 video, and a webgl sample and killing the GPU process

[Nat Duca]

Comment on attachment 123226 [details]
Patch

Looks highly amusing to me. Which should rhyme with LGTM but doesnt.

[Vangelis Kokkevis]

Comment on attachment 123226 [details]
Patch

Do we end up calling evictAndDeleteAllTextures and if so are we trying to delete textures in a context that no longer exists?

[James Robinson]

(In reply to comment #5)
> (From update of attachment 123226 [details])
> Do we end up calling evictAndDeleteAllTextures and if so are we trying to delete textures in a context that no longer exists?

We do and we are (relying on all GL calls on a lost context being no-ops, which they are).  We could add a path that goes through the normal eviction path but doesn't actually issue any GL calls if we were concerned about this.

[Adrienne Walker]

LGTM2.  Now that all layer types are using ManagedTexture, this should be safe to do.

[Alok Priyadarshi]

(In reply to comment #2)
> Pretty simple.  This codepath used to do more back when layers managed their own textures, but since everything either uses managed textures (content/image/video layers) or has its own context lost management (webgl/plugin) we don't have to do anything special when losing a context.
> 
> setLayerTreeHost() is now only called when:
> *) A layer enters a tree
> *) A layer leaves a tree
> *) A layer moves from one tree to another (for example moves from being inside a tab to a window.open()'d popup)

This is a good change. I just wanted point out a slightly related bug: https://bugs.webkit.org/show_bug.cgi?id=77135

setLayerTreeHost() also get called even when a layer remains in the same tree:
1. Moving a layer within the tree. See WebCore::RenderLayer::removeOnlyThisLayer().
2. GraphicsLayer::setChildren() removes-adds the layers common in the current child-list and the new child-list

Since setLayerTreeHost(0) also triggers resource cleanup, these two cases sometimes severely affect performance due to unnecessary work.

[Kenneth Russell]

Comment on attachment 123226 [details]
Patch

r=me based on enne's review.

[James Robinson]

Committed r106048: <http://trac.webkit.org/changeset/106048>