[webkit-reviews] review granted: [Bug 45939] Add entry points to GraphicsContext3D needed for Chromium compositor port : [Attachment 67871] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted Kenneth Russell
<kbr at google.com>'s request for review:
Bug 45939: Add entry points to GraphicsContext3D needed for Chromium compositor
port
https://bugs.webkit.org/show_bug.cgi?id=45939

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
Looks good, just a few nits to address before landing:

- this patch uses UNUSED_PARAM() many times when it doesn't need to.  If all
permutations of a function will not use a parameter you can just avoid naming
the parameter in the .cpp. UNUSED_PARAM is more for this case:

void foo(int parameter) {
#ifdef SOMETHING
  bar(parameter);
#else
  UNUSED_PARAM(parameter);
#endif
}

Also, consider changing renderDirectlyToHostWindow from a bool to an enum.

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

> WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:103
> +    UNUSED_PARAM(renderDirectlyToHostWindow);

instead of UNUSED_PARAM() just don't name the parameter

> WebCore/platform/graphics/qt/GraphicsContext3DQt.cpp:506
> +GraphicsContext3D::GraphicsContext3D(GraphicsContext3D::Attributes attrs,
HostWindow* hostWindow, bool renderDirectlyToHostWindow)
>      : m_internal(new GraphicsContext3DInternal(attrs, hostWindow))
>  {
> +    UNUSED_PARAM(renderDirectlyToHostWindow);

instead of using UNUSED_PARAM() here, just don't name the parameter