[webkit-reviews] review denied: [Bug 60817] [chromium] Composited rendering mode for plugins : [Attachment 98294] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 22 19:20:17 PDT 2011
James Robinson <jamesr at chromium.org> has denied Hin-Chung Lam
<hclam at google.com>'s request for review:
Bug 60817: [chromium] Composited rendering mode for plugins
https://bugs.webkit.org/show_bug.cgi?id=60817
Attachment 98294: Patch
https://bugs.webkit.org/attachment.cgi?id=98294&action=review
------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98294&action=review
R- for lack of tests, left a few other comments
> Source/WebCore/ChangeLog:13
> + PluginCompositingLayerChromium is defined as a hub to contain
multiple sublayers, namely PluginCompositigLayerChromium::Sublayer.
typo: PluginCompositigLayerChromium
>
Source/WebCore/platform/graphics/chromium/PluginCompositingLayerChromium.cpp:12
3
> + // FIXME: There will be different implementations of SublayerImpl so
construct the right one based on type.
> + return 0;
this function doesn't seem to be returning a sublayer of the wrong type, it's
not returning a sublayer at all.
> Source/WebCore/platform/graphics/chromium/PluginCompositingLayerChromium.h:58
> + class Sublayer : public RefCounted<Sublayer> {
Why is this refcounted? It seems that the only place this is stored is in
PluginCompositingLayerChromium::m_subLayers, where you could use an OwnPtr<>
just fine
> Source/WebCore/platform/graphics/chromium/PluginCompositingLayerChromium.h:61
> + virtual void initialize() = 0;
I still don't understand what the different overrides of this class would be.
What would they do differently, exactly?
More information about the webkit-reviews
mailing list