[webkit-reviews] review requested: [Bug 114639] [CSS Shaders] Extract validation logic out of CustomFilterValidatedProgram into a CustomFilterValidator class : [Attachment 200998] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 7 17:04:10 PDT 2013


Max Vujovic <mvujovic at adobe.com> has asked  for review:
Bug 114639: [CSS Shaders] Extract validation logic out of
CustomFilterValidatedProgram into a CustomFilterValidator class
https://bugs.webkit.org/show_bug.cgi?id=114639

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

------- Additional Comments from Max Vujovic <mvujovic at adobe.com>
Some areas in the patch I'd like feedback on:

(1) CustomFilterValidator is a class with only static methods, and it should
never be initialized. I've made its constructor private. I'm thinking I should
additionally disallow the copy constructor and assignment. I could only find
the macro "DISALLOW_IMPLICIT_CONSTRUCTORS" defined in wtf/dtoa/utils.h. It
doesn't seem correct to include that file here, since the file is all about
double to string conversion. I'm wondering if we have some convention in WebKit
for static classes.

(2) I used only file static methods in CustomFilterValidator, since I try to
keep things out of headers. Any thoughts on that? I could instead use class
static methods or some mix between the two.

(3) Does the static local initialization of ANGLEWebKitBridge in
CustomFilterValidator.cpp look ok? I didn't want to use the DEFINE_STATIC_LOCAL
macro because it only supports sending arguments to the ANGLEWebKitBridge
constructor. I need to both send arguments to the constructor *and* perform
some initialization after running the constructor (see the
createShaderValidator function).


More information about the webkit-reviews mailing list