[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