[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