[webkit-reviews] review denied: [Bug 79871] [BlackBerry] Upstream LayerWebKitThread and its derived classes : [Attachment 129422] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Feb 29 10:46:56 PST 2012
Rob Buis <rwlbuis at gmail.com> has denied Robin Cao
<robin.cao at torchmobile.com.cn>'s request for review:
Bug 79871: [BlackBerry] Upstream LayerWebKitThread and its derived classes
https://bugs.webkit.org/show_bug.cgi?id=79871
Attachment 129422: patch
https://bugs.webkit.org/attachment.cgi?id=129422&action=review
------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=129422&action=review
Looks good, still some things to improve.
> Source/WebCore/platform/graphics/blackberry/LayerData.h:156
> + bool includeVisibility()
Could be made const.
> Source/WebCore/platform/graphics/blackberry/LayerData.h:192
> + LayerProgramShader m_layerProgramShader;
Could move this one so all the bools can become a bitfield, saving some mem.
Not sure how often LayerData is created though.
> Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.cpp:426
> + return m_superlayer;
Why not inline this one?
> Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.h:47
> +#include <wtf/text/StringHash.h>
Are all of these included needed?
> Source/WebCore/platform/graphics/blackberry/LayerWebKitThread.h:156
> + bool isDrawable() { return m_isDrawable; }
const
> Source/WebCore/platform/graphics/blackberry/PluginLayerWebKitThread.cpp:79
> + m_pluginView->updateBuffer(IntRect(IntPoint(0, 0),
m_pluginView->size()));
IntPoint::zero
> Source/WebCore/platform/graphics/blackberry/VideoLayerWebKitThread.cpp:58
> + // clear hole punch rect
Not a sentence.
> Source/WebCore/platform/graphics/blackberry/VideoLayerWebKitThread.cpp:90
> + setHolePunchRect(IntRect(0, 0, m_bounds.width(), m_bounds.height()));
Probably more efficient to do IntRect(IntPoint::zero(), m_bounds);
> Source/WebCore/platform/graphics/blackberry/VideoLayerWebKitThread.cpp:94
> + if (frameView)
Can be combined.
More information about the webkit-reviews
mailing list