[webkit-reviews] review denied: [Bug 49946] [chromium] Implement Extensions3DChromium::getGraphicsResetStatusARB : [Attachment 74610] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has denied Alexey Marinichev
<amarinichev at chromium.org>'s request for review:
Bug 49946: [chromium] Implement Extensions3DChromium::getGraphicsResetStatusARB
https://bugs.webkit.org/show_bug.cgi?id=49946

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

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=74610&action=review

This basically looks good but there's one minor semantic bug.

> WebKit/chromium/src/Extensions3DChromium.cpp:53
> +    return m_internal->isContextLost() ?
GraphicsContext3D::CONTEXT_LOST_WEBGL : GraphicsContext3D::NO_ERROR;

This should be returning one of NO_ERROR, GUILTY_CONTEXT_RESET_ARB,
INNOCENT_CONTEXT_RESET_ARB, or UNKNOWN_CONTEXT_RESET_ARB. Since it sounds like
we can't make the determination yet it should probably be
UNKNOWN_CONTEXT_RESET_ARB.

> WebKit/chromium/src/WebGraphicsContext3DDefaultImpl.cpp:992
> +    return false;

Should we add a FIXME to implement this?