[chromium] Check if the acceleratedCompositingEnabled setting is active in WebViewImpl::graphicsContext3D()

[James Robinson]

[chromium] Check if the acceleratedCompositingEnabled setting is active in WebViewImpl::graphicsContext3D()

[James Robinson]

Created attachment 68325 [details]
Patch

[Kenneth Russell]

Comment on attachment 68325 [details]
Patch

If I'm reading the diff correctly, you're missing a return statement if accelerated compositing isn't enabled for the page. The patch looks good otherwise, but r- for this serious issue.

[Vangelis Kokkevis]

Comment on attachment 68325 [details]
Patch

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

> WebKit/chromium/src/WebViewImpl.cpp:2437
> +    if (m_page->settings()->acceleratedCompositingEnabled()) {

For good measure you could in addition also call allowsAcceleratedCompositing() which will return false if the compositor failed to initialize on this webview.  This will prevent a page with multiple canvas' to continuously try to create a GL context and fail.

[James Robinson]

(In reply to comment #2)
> (From update of attachment 68325 [details])
> If I'm reading the diff correctly, you're missing a return statement if accelerated compositing isn't enabled for the page. The patch looks good otherwise, but r- for this serious issue.

Oh snap, good catch.  I really need to crank up the warning level for the chromium linux build.  Will fix.

[James Robinson]

Created attachment 68332 [details]
Patch

[Kenneth Russell]

Comment on attachment 68332 [details]
Patch

Looks good.

[James Robinson]

Comment on attachment 68332 [details]
Patch

Clearing flags on attachment: 68332

Committed r68015: <http://trac.webkit.org/changeset/68015>

[James Robinson]

All reviewed patches have been landed.  Closing bug.