Always use either glXCreateNewContext or glXCreateContextAttribsARB for GLX

ui/gl/gl_context_glx.cc

Index: ui/gl/gl_context_glx.cc
diff --git a/ui/gl/gl_context_glx.cc b/ui/gl/gl_context_glx.cc
index c6752634a19a640c253bf7dd1e81eb720b8753fa..a6b97872eb1c5e4b13d92ae2f7aa9ff67d1615ac 100644
--- a/ui/gl/gl_context_glx.cc
+++ b/ui/gl/gl_context_glx.cc
@@ -49,12 +49,14 @@ bool GLContextGLX::Initialize(
   GLXContext share_handle = static_cast<GLXContext>(
       share_group() ? share_group()->GetHandle() : NULL);
 
-  if (GLSurfaceGLX::IsCreateContextRobustnessSupported()) {
-    DVLOG(1) << "GLX_ARB_create_context_robustness supported.";
-
+  if (GLSurfaceGLX::IsCreateContextSupported()) {
+    DVLOG(1) << "GLX_ARB_create_context supported.";
     std::vector<int> attribs;
-    attribs.push_back(GLX_CONTEXT_RESET_NOTIFICATION_STRATEGY_ARB);
-    attribs.push_back(GLX_LOSE_CONTEXT_ON_RESET_ARB);
+    if (GLSurfaceGLX::IsCreateContextRobustnessSupported()) {
+      DVLOG(1) << "GLX_ARB_create_context_robustness supported.";
+      attribs.push_back(GLX_CONTEXT_RESET_NOTIFICATION_STRATEGY_ARB);
+      attribs.push_back(GLX_LOSE_CONTEXT_ON_RESET_ARB);
+    }
     attribs.push_back(0);
     context_ = glXCreateContextAttribsARB(
         display_,
@@ -62,75 +64,29 @@ bool GLContextGLX::Initialize(
         share_handle,
         True,
         &attribs.front());
-    if (context_) {
-      DVLOG(1) << "  Successfully allocated "
-               << (compatible_surface->IsOffscreen() ?
-                   "offscreen" : "onscreen")
-               << " GL context with LOSE_CONTEXT_ON_RESET_ARB";
-    } else {
-      // TODO(kbr): it is not expected that things will work properly
-      // in this case, since we will likely allocate our offscreen
-      // contexts with this bit set and the onscreen contexts without,
-      // and won't be able to put them in the same share group.
-      // Consider what to do here; force loss of all contexts and
-      // reallocation without ARB_robustness?
-      LOG(ERROR) <<
-          "  FAILED to allocate GL context with LOSE_CONTEXT_ON_RESET_ARB";
+    if (!context_) {
+      LOG(ERROR) << "Failed to create GL context with "
+                 << "glXCreateContextAttribsARB.";
+      return false;
     }
-  }
-
-  if (!context_) {
-    // The means by which the context is created depends on whether
-    // the drawable type works reliably with GLX 1.3. If it does not
-    // then fall back to GLX 1.2.
-    if (compatible_surface->IsOffscreen()) {
-      context_ = glXCreateNewContext(
-          display_,
-          static_cast<GLXFBConfig>(compatible_surface->GetConfig()),
-          GLX_RGBA_TYPE,
-          share_handle,
-          True);
-    } else {
-      // Get the visuals for the X drawable.

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

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]

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).

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

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

-      XWindowAttributes attributes;
-      if (!XGetWindowAttributes(
-          display_,
-          reinterpret_cast<GLXDrawable>(compatible_surface->GetHandle()),
-          &attributes)) {
-        LOG(ERROR) << "XGetWindowAttributes failed for window " <<
-            reinterpret_cast<GLXDrawable>(
-                compatible_surface->GetHandle()) << ".";
-        return false;
-      }
-
-      XVisualInfo visual_info_template;
-      visual_info_template.visualid = XVisualIDFromVisual(attributes.visual);
-
-      int visual_info_count = 0;
-      scoped_ptr_malloc<XVisualInfo, ScopedPtrXFree> visual_info_list(
-          XGetVisualInfo(display_, VisualIDMask,
-                         &visual_info_template,
-                         &visual_info_count));
-
-      DCHECK(visual_info_list.get());
-      if (visual_info_count == 0) {
-        LOG(ERROR) << "No visual info for visual ID.";
-        return false;
-      }
-
-      // Attempt to create a context with each visual in turn until one works.
-      context_ = glXCreateContext(
-          display_,
-          visual_info_list.get(),
-          share_handle,
-          True);
+  } else {
+    DVLOG(1) << "GLX_ARB_create_context not supported.";
+    context_ = glXCreateNewContext(
+       display_,
+       static_cast<GLXFBConfig>(compatible_surface->GetConfig()),
+       GLX_RGBA_TYPE,
+       share_handle,
+       True);
+    if (!context_) {
+      LOG(ERROR) << "Failed to create GL context with glXCreateNewContext.";
+      return false;
     }
   }
-
-  if (!context_) {
-    LOG(ERROR) << "Couldn't create GL context.";
-    return false;
-  }
+  DCHECK(context_);
+  DVLOG(1) << "  Successfully allocated "
+           << (compatible_surface->IsOffscreen() ?
+               "offscreen" : "onscreen")
+           << " GL context with LOSE_CONTEXT_ON_RESET_ARB";
 
   DVLOG(1) << (compatible_surface->IsOffscreen() ? "Offscreen" : "Onscreen")
            << " context was "
@@ -156,9 +112,10 @@ bool GLContextGLX::MakeCurrent(GLSurface* surface) {
     return true;
 
   TRACE_EVENT0("gpu", "GLContextGLX::MakeCurrent");
-  if (!glXMakeCurrent(
+  if (!glXMakeContextCurrent(
       display_,
       reinterpret_cast<GLXDrawable>(surface->GetHandle()),
+      reinterpret_cast<GLXDrawable>(surface->GetHandle()),
       static_cast<GLXContext>(context_))) {
     LOG(ERROR) << "Couldn't make context current with X drawable.";
     Destroy();
@@ -188,7 +145,7 @@ void GLContextGLX::ReleaseCurrent(GLSurface* surface) {
     return;
 
   SetCurrent(NULL, NULL);
-  if (!glXMakeCurrent(display_, 0, 0))
+  if (!glXMakeContextCurrent(display_, 0, 0, 0))
     LOG(ERROR) << "glXMakeCurrent failed in ReleaseCurrent";
 }