[webkit-reviews] review granted: [Bug 222593] Adding new test conditions for WebGL should be simpler : [Attachment 421925] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 2 12:22:41 PST 2021


Kenneth Russell <kbr at google.com> has granted Kimmo Kinnunen
<kkinnunen at apple.com>'s request for review:
Bug 222593: Adding new test conditions for WebGL should be simpler
https://bugs.webkit.org/show_bug.cgi?id=222593

Attachment 421925: Patch

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




--- Comment #4 from Kenneth Russell <kbr at google.com> ---
Comment on attachment 421925
  --> https://bugs.webkit.org/attachment.cgi?id=421925
Patch

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

No, it's OK to move forward with this - I can see it'll significantly reduce
boilerplate in the future. Couple of comments about documentation. r+

> Source/WebCore/html/canvas/WebGLRenderingContextBase.h:381
> +    WEBCORE_EXPORT void simulateEventForTesting(const String&);

Please add a comment documenting the valid inputs - or point to another header
where they're documented.

> Source/WebCore/platform/graphics/GraphicsContextGL.h:1302
> +    virtual void simulateEventForTesting(const String&) = 0;

Again, please point to documentation.

> Source/WebCore/testing/Internals.h:775
> +    void simulateEventForWebGLContext(const String& eventName,
WebGLRenderingContext&);

If this is the single place where these events should be documented then please
add a comment here describing what they are.

> Source/WebCore/testing/Internals.idl:794
> +    [Conditional=WEBGL] undefined simulateEventForWebGLContext(DOMString
eventName, WebGLRenderingContext context);

Or here.


More information about the webkit-reviews mailing list