[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 #17 from Tien-Ren Chen <trchen at chromium.org>  2012-02-24 18:09:16 PST ---
(In reply to comment #11)
> > Source/WebKit/chromium/tests/CCLayerImplTest.cpp:69
> > +    OwnPtr<CCLayerImpl> opChild = CCLayerImpl::create(2);
> 
> it's fairly unusual to have an "op" prefix.  we use the "pop" prefix for PassOwnPtr<> when we put the result immediately into a local and never use the prefixed variable again. another issue with this construction is that the .release() call nulls out opChild, but it's still sitting in the scope tempting someone to write code that incorrectly tries to use it
> 
> here's another way to write this that would be more consistent with other WebKit code and a bit less error-prone:
> 
> OwnPtr<CCLayerImpl> root = CCLayerImpl::create(1);
> root->addChild(CCLayerImpl::create(2));
> child->addChild(CCLayerImpl::create(3));
> CCLayerImpl* child = root->children()[0];
> CCLayerImpl* grandChild = child->children()[0];
> 
> etc.
> 
> same goes for rest of the tests

I find this results in behavior change in some case, for example when the layer properties need to be configured before added to the tree. In those case I'll use leakPtr() then adoptPtr() again. I think this approach is not too error-prone as the dirty part is limited in between leakPtr/adoptPtr pair only (which is usually short).

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