[webkit-reviews] review denied: [Bug 87602] [BlackBerry] Make it possible to manipulate layers on the compositing thread : [Attachment 144244] Patch

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Arvid Nilsson <anilsson at rim.com>'s
request for review:
Bug 87602: [BlackBerry] Make it possible to manipulate layers on the
compositing thread
https://bugs.webkit.org/show_bug.cgi?id=87602

Attachment 144244: Patch
https://bugs.webkit.org/attachment.cgi?id=144244&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=144244&action=review


Ok, can be cleaned up a bit more I think.

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:394
> +    layer->setSuperlayer(this);

Would it make sense for setSuperlayer to call removeFromSuperlayer internally?

> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.cpp:498
> +    if (m_override) {

Should probably use early return style here, code will be more readable.

>> Source/WebCore/platform/graphics/blackberry/LayerCompositingThread.h:77
>> +	Vector<RefPtr<LayerAnimation> > m_animations;
> 
> All members don't have "m_" prefix but m_animations. Should be in same style?

> 
> More thought: how to avoid people from setting those members directly and
forgetting to set the flags?
> 
> I would rather make them private, and provide read-only get() methods for
them.

Agree with Yong, m_ is a good prefix to use.