[Webkit-unassigned] [Bug 78404] [chromium] Replace RefPtr with OwnPtr for CCLayerImpl tree structure

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=78404





--- Comment #4 from Tien-Ren Chen <trchen at chromium.org>  2012-02-13 14:46:04 PST ---
(In reply to comment #2)
> (From update of attachment 126614 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=126614&action=review
> 
> I like everything except the TreeSynchronizer change.  TreeSynchronizer shouldn't be instantiable, it should just be pure functions.
> 
> > Source/WebCore/platform/graphics/chromium/TreeSynchronizer.h:51
> > +    TreeSynchronizer() { }
> 
> why did you change this? i much prefer having tree syncing be a pure function, this is weird

I was thinking that I need some way to allow a layer to have pointers referring to other layers (for example, m_horizontalScrollbarLayer). I could do that by passing the CCLayerImplMap to LayerChromium::pushPropertiesTo. Instead of passing the map directly, it is better to encapsulate it with TreeSynchronizer so pushPropertiesTo can access the synchronizer state through its public methods. (That's why reuseOrCreateCCLayerImpl was declared public.)

Another reason is having to pass the state in every recursive call is a pain... I find it much better to write recursive calls in functor style. More clean and less error-prone to me. Well that's more of personal preference so I won't insist it.

I uploaded an alternative implementation in pure function style. Either way is fine to me.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.