There are a few "unclear" (pun blatantly intended) function names - clearRenderSurface() and clearRenderSurfacesOnCCLayerImplRecursive() and their corresponding functions on CCLayerImpl, too. For readability, we should use more descriptive function names... - LayerRendererChromium::clearSurfaceContents() - LayerRendererChromium::clearRenderSurfacePointers() - CCLayerImpl::clearRenderSurfacePointer()
Created attachment 135377 [details] Patch
Hi James, just one more clean-up for today, if it meets your approval =) Thanks in advance
Comment on attachment 135377 [details] Patch These functions have the same name, but they're in very different types so I'm not sure there is much risk of confusion. I think clearFoo() is fairly well established for clearing an m_foo pointer. I think a better way to disambiguate LayerRendererChromium would be to move this function up to CCLayerTreeHostImpl, where it would fit better
OK sounds good. That means that LRC::close() will also get removed.
Yes, exactly. LayerRendererChromium should be dealing only with GL calls, so 'clear' in that file should always mean something like "do a glClear()". All layer tree lifetime / ownership issues for the impl thread should be dealt with up on CCLayerTreeHostImpl, and CCLayerTreeHostImpl shouldn't know anything about GL, so 'clear' up there should always be doing something to a smart pointer type.
Created attachment 135679 [details] Patch Tested all layout tests and unit tests on linux, no obvious regressions
Comment on attachment 135679 [details] Patch Nice
Comment on attachment 135679 [details] Patch Thanks for reviewing!
Comment on attachment 135679 [details] Patch Clearing flags on attachment: 135679 Committed r113248: <http://trac.webkit.org/changeset/113248>
All reviewed patches have been landed. Closing bug.