Bug 111953 – Possible incorrect cleanup in RenderLayerBacking destructor

• Component: Layout and Rendering
• Status: RESOLVED INVALID

[Shawn Singh]

The code in RenderLayerBacking::destroyGraphicsLayers()  (which is private and only called from destructor) does the following:
  1. detaches m_graphicsLayer from its possible parent
  2. releases the refs of all possible graphicsLayers that may exist.

It looks like step 1 has existed since this code was added way back in 2009.  Was there a reason do implement step 1 this way?

It seems to me that the purpose of step 1 is to remove this Backing's GraphicsLayers from the composited layer tree, right?  If so, then I think it should actually be accessing childForSuperLayers(), and detaching that layer from it's parent.

Or optionally, if we want to remove the assumption that the layer is in a "good state" when its being destroyed, we could just removeFromParent for all possible values of childForSuperLayers(), or even for all GraphicsLayers, knowing it will be a no-op (though it may cause more churn)

Simon, what do you think?

I don't have any examples of this causing crashes or leaks, but it's conceivable to me that it can cause crashes.  In practice I suspect that problems don't occur because RenderLayerBackings are typically de-allocated just before a new RenderLayerBacking tree is rebuilt, at which point such dangling subtrees would disappear shortly after, anyway.

[Shawn Singh]

Well, sorry for the noise - it looks like other functions that run just before that, in the destructor, would clean up those layers already.   So this is WONTFIX - I'll wait until tomorrow just in case anyone has anything to say.