[webkit-reviews] review granted: [Bug 187693] [Nicosia] Add Nicosia::PlatformLayer, Nicosia::CompositionLayer classes : [Attachment 345088] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 187693: [Nicosia] Add Nicosia::PlatformLayer, Nicosia::CompositionLayer
classes
https://bugs.webkit.org/show_bug.cgi?id=187693

Attachment 345088: Patch

https://bugs.webkit.org/attachment.cgi?id=345088&action=review




--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 345088
  --> https://bugs.webkit.org/attachment.cgi?id=345088
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=345088&action=review

> Source/WebCore/platform/graphics/nicosia/NicosiaPlatformLayer.h:45
> +    PlatformLayer(uint64_t);

explicit. If this is abstract, it's probably better to make the constructor
protected.

> Source/WebCore/platform/graphics/nicosia/NicosiaPlatformLayer.h:48
> +    virtual bool isCompositionLayer() const { return false; }

Should this be pure virtual instead?

> Source/WebCore/platform/graphics/nicosia/NicosiaPlatformLayer.h:62
> +    CompositionLayer(uint64_t);

explicit. Since this is refcounted, I think it would be better to add a create
function returning a Ref<> and make the constructor private.

> Source/WebCore/platform/graphics/nicosia/NicosiaPlatformLayer.h:70
> +		   : value(0)

Could we init this below?

> Source/WebCore/platform/graphics/nicosia/NicosiaPlatformLayer.h:89
> +		   uint32_t value;

with { 0 } here?

> Source/WebCore/platform/graphics/nicosia/NicosiaPlatformLayer.h:112
> +		   uint32_t value;

Same here, and only init in the constructor the values that are true, is that
possible?