[webkit-reviews] review denied: [Bug 63189] Fix latch deadlock when GPU process crashes or context is lost : [Attachment 98248] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has denied John Bates <jbates at google.com>'s
request for review:
Bug 63189: Fix latch deadlock when GPU process crashes or context is lost
https://bugs.webkit.org/show_bug.cgi?id=63189

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

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

The logic changes look fine but there's an important documentation update that
needs to be incorporated into this patch.

> Source/WebCore/platform/graphics/chromium/Canvas2DLayerChromium.cpp:68
> +	       && (context->getExtensions()->getGraphicsResetStatusARB() ==
GraphicsContext3D::NO_ERROR));

I made a mistake when originally adding getGraphicsResetStatusARB to
Extensions3D.h. In the real OpenGL API, this function only returns the error
state once; subsequent calls generally return NO_ERROR. In WebKit, this entry
point's state needs to be sticky; it needs to continue to return the error
state essentially forever, because context recovery is implemented by throwing
away the GraphicsContext3D instance and creating a new one. Please update the
comments in Extensions3D.h indicating that any error returned needs to be
persistent for the lifetime of the GraphicsContext3D object, unlike the
behavior of the actual GL_ARB_robustness extension.