[webkit-reviews] review granted: [Bug 84120] [chromium] Convert WebPluginContainerImpl over to use WebExternalTextureLayer : [Attachment 137469] rebased

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Apr 18 10:37:16 PDT 2012


Adrienne Walker <enne at google.com> has granted James Robinson
<jamesr at chromium.org>'s request for review:
Bug 84120: [chromium] Convert WebPluginContainerImpl over to use
WebExternalTextureLayer
https://bugs.webkit.org/show_bug.cgi?id=84120

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

------- Additional Comments from Adrienne Walker <enne at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=137469&action=review


Thanks for splitting this up into two patches.	That made it way easier to
review.

> Source/Platform/chromium/public/WebExternalTextureLayer.h:38
>  class WebExternalTextureLayerImpl;

Remove me.

> Source/Platform/chromium/public/WebExternalTextureLayer.h:59
>      // Sets the texture id that represents the layer, in the namespace of
the
>      // compositor context.
>      WEBKIT_EXPORT void setTextureId(unsigned);
> -    WEBKIT_EXPORT unsigned textureId() const;
> +
> +    // Sets the IO surface id that represents this layer. setTextureId() and
setIOSurfaceProperties() are mutually
> +    // exclusive - a layer can be either backed by a texture or an IO
surface, but not both.
> +    WEBKIT_EXPORT void setIOSurfaceProperties(const WebSize&, unsigned
ioSurfaceId);

A random design aside.	It's always struck me as a little bit of a wart that we
have this class that gets used in these two modes, and these modes infect its
usage all the way down, even though one path (seemingly?) is only used by npapi
plugins and the other only by ppapi.  I kind of wish it wouldn't cost so much
boilerplate to have them be two separate classes.  It doesn't have to be fixed
here, but do you have any thoughts on how to make this better?


More information about the webkit-reviews mailing list