Always use either glXCreateNewContext or glXCreateContextAttribsARB for GLX

Always use either glXCreateNewContext or glXCreateContextAttribsARB for GLX

Explicitly create the GLX window for onscreen surfaces.

Use glXCreateContextAttribsARB to create GLX contexts when GLX_ARB_create_context is supported, otherwise use glXCreateNewContext.

This will allow us to explicitly destroy hibernated GLX windows, which will hopefully plug some GPU memory leaks that were causing instability.

BUG=145600
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=172122

[ccameron, 2012-12-08 02:11:27]

The last take at this,
https://codereview.chromium.org/11467008/
was reverted because it assumed that the client had GLX_ARB_create_context,
which is not always the case. This removes that assumption.

Of note is that the failures were not crashes (as I would have expected) because
GLX always returns a valid function pointer (that does nothing), even for
unsupported extensions.

[piman, 2012-12-08 02:15:12]

lgtm

[Ken Russell (switch to Gerrit), 2012-12-08 20:10:57]

lgtm

https://codereview.chromium.org/11474045/diff/1/ui/gl/gl_context_glx.cc
File ui/gl/gl_context_glx.cc (left):

https://codereview.chromium.org/11474045/diff/1/ui/gl/gl_context_glx.cc#oldco...
ui/gl/gl_context_glx.cc:94: // Get the visuals for the X drawable.
If the last patch didn't work on all clients then presumably they were going
down this code path for on-screen surfaces. Are you sure it isn't needed any
more?

[ccameron, 2012-12-10 00:28:06]

Thanks!!

https://codereview.chromium.org/11474045/diff/1/ui/gl/gl_context_glx.cc
File ui/gl/gl_context_glx.cc (left):

https://codereview.chromium.org/11474045/diff/1/ui/gl/gl_context_glx.cc#oldco...
ui/gl/gl_context_glx.cc:94: // Get the visuals for the X drawable.
On 2012/12/08 20:10:57, kbr wrote:
> If the last patch didn't work on all clients then presumably they were going
> down this code path for on-screen surfaces. Are you sure it isn't needed any
> more?

For the failures that we looked into, they were all because the extension
GLX_ARB_create_context wasn't supported (so they just didn't have the function
glXCreateContextAttribsARB), so it looks like all platforms should have
glXCreateNewContext and should be able to use it (I'll have to keep my eyes on
the waterfall though).

[commit-bot: I haz the power, 2012-12-10 18:14:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ccameron@chromium.org/11474045/1

[Ken Russell (switch to Gerrit), 2012-12-10 18:44:51]

https://codereview.chromium.org/11474045/diff/1/ui/gl/gl_context_glx.cc
File ui/gl/gl_context_glx.cc (left):

https://codereview.chromium.org/11474045/diff/1/ui/gl/gl_context_glx.cc#oldco...
ui/gl/gl_context_glx.cc:94: // Get the visuals for the X drawable.
On 2012/12/10 00:28:06, ccameron1 wrote:
> On 2012/12/08 20:10:57, kbr wrote:
> > If the last patch didn't work on all clients then presumably they were going
> > down this code path for on-screen surfaces. Are you sure it isn't needed any
> > more?
> 
> For the failures that we looked into, they were all because the extension
> GLX_ARB_create_context wasn't supported (so they just didn't have the function
> glXCreateContextAttribsARB), so it looks like all platforms should have
> glXCreateNewContext and should be able to use it (I'll have to keep my eyes on
> the waterfall though).

My concern was whether GLSurface::GetConfig worked for on-screen windows, but I
re-checked NativeViewSurfaceGLX::GetConfig and it does.

[commit-bot: I haz the power, 2012-12-10 20:13:38]

Change committed as 172122