[webkit-reviews] review denied: [Bug 29826] Add support for run-time flag for 3D canvas : [Attachment 40259] Patch adding run-time flag covering 3D canvas functionality.

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Kenneth Russell
<kbr at google.com>'s request for review:
Bug 29826: Add support for run-time flag for 3D canvas
https://bugs.webkit.org/show_bug.cgi?id=29826

Attachment 40259: Patch adding run-time flag covering 3D canvas functionality.
https://bugs.webkit.org/attachment.cgi?id=40259&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
Mostly procedural nits.

> +2009-09-28  Kenneth Russell	<kbr at google.com>
> +
> +	   Reviewed by dglazkov at chromium.org(?), cmarrin at apple.com(?).

You probably shouldn't remove the OOPS from here until reviewed. The pre-commit
hook makes it your last line of defense.
However -- you don't have a commit bit, so my point is Moooo-t :)

> +
> +	   Add support for run-time flag for 3D canvas
> +	   https://bugs.webkit.org/show_bug.cgi?id=29826
> +
> +	   * DerivedSources.make: Conditionally add
> +	Settings::setExperimentalWebGLEnabled to exported symbol list.
Weird indentation here. Tabs maybe?

> +	   * WebCore.WebGL.exp: Added.
> +	   * html/HTMLCanvasElement.cpp:
> +	   (WebCore::HTMLCanvasElement::getContext): Check page settings for
> +	experimental WebGL flag before returning 3D graphics context.

Ditto.

> +
> +	   * WebView/WebView.mm:
> +	   (-[WebView _preferencesChangedNotification:]): Enable experimental
> +	WebGL flag when 3D_CANVAS is enabled in the build.

Ditto.

>  
> +#if ENABLE(3D_CANVAS)
> +    settings->setExperimentalWebGLEnabled(true);
> +#endif  // ENABLE(3D_CANVAS)

I didn't realize Win was a supported platform. If it's not, should we do this
for all other ports, then?