[webkit-reviews] review granted: [Bug 188344] [Nicosia] Add the TextureMapper-specific ContentLayer::Impl derivative : [Attachment 346623] 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 188344: [Nicosia] Add the TextureMapper-specific ContentLayer::Impl
derivative
https://bugs.webkit.org/show_bug.cgi?id=188344

Attachment 346623: Patch

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




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

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

>
Source/WebCore/platform/graphics/nicosia/texmap/NicosiaContentLayerTextureMappe
rImpl.cpp:47
> +    : m_proxy(adoptRef(*new WebCore::TextureMapperPlatformLayerProxy))

I would add a create() to TextureMapperPlatformLayerProxy

>
Source/WebCore/platform/graphics/nicosia/texmap/NicosiaContentLayerTextureMappe
rImpl.h:53
> +    ContentLayerTextureMapperImpl(Client&);

explicit. This should only be called by the factory, right? Can we make it
private? or does it have to be public because of make_unique?

>
Source/WebCore/platform/graphics/nicosia/texmap/NicosiaContentLayerTextureMappe
rImpl.h:59
> +    const Ref<WebCore::TextureMapperPlatformLayerProxy>& proxy() const {
return m_proxy; }

Why returning a const Ref<>&? why not just const
WebCore::TextureMapperPlatformLayerProxy& instead?

>
Source/WebCore/platform/graphics/nicosia/texmap/NicosiaContentLayerTextureMappe
rImpl.h:66
> +	   Client* object { nullptr };

I would call this client instead of object, even if redundant it's less
confusing, I think.