Fix gl_unittests on Mac core profile.

ui/gl/scoped_binders.cc

Index: ui/gl/scoped_binders.cc
diff --git a/ui/gl/scoped_binders.cc b/ui/gl/scoped_binders.cc
index f89a6b3b164a3cdbbe0b0ffca001ae2cd33d44b3..6492a5e205355b4f5f39981b2cca280b88642160 100644
--- a/ui/gl/scoped_binders.cc
+++ b/ui/gl/scoped_binders.cc
@@ -6,6 +6,7 @@
 #include "ui/gl/gl_bindings.h"
 #include "ui/gl/gl_context.h"
 #include "ui/gl/gl_state_restorer.h"
+#include "ui/gl/gl_version_info.h"
 
 namespace gl {
 
@@ -100,25 +101,42 @@ ScopedVertexAttribArray::ScopedVertexAttribArray(unsigned int index,
       type_(-1),
       normalized_(GL_FALSE),
       stride_(0),
-      pointer_(0) {
-  glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, &buffer_);
-  glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_ENABLED, &enabled_);
-  glEnableVertexAttribArray(index);
-
-  glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_SIZE, &size_);
-  glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_TYPE, &type_);
-  glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_NORMALIZED, &normalized_);
-  glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_STRIDE, &stride_);
-  glGetVertexAttribPointerv(index, GL_VERTEX_ATTRIB_ARRAY_POINTER, &pointer_);
+      pointer_(0),
+      current_vao_(0u),
+      vao_(0u) {
+  const GLVersionInfo* version_info = GLContext::GetCurrent()->GetVersionInfo();
+  if (version_info->IsAtLeastGL(3, 3) || version_info->IsAtLeastGLES(3, 0)) {
+    GLint value = 0;
+    glGetIntegerv(GL_VERTEX_ARRAY_BINDING, &value);
+    current_vao_ = static_cast<unsigned>(value);
+    glGenVertexArraysOES(1, &vao_);
+    glBindVertexArrayOES(vao_);

[piman, 2016/08/18 00:44:50]

The 2 paths are not really equivalent... the old path only changes the binding
for attribute |index| and leaves the other ones alone, whereas the new path
disables all other attributes (by setting up a new vao).

In particular, it means we can't nest ScopedVertexAttribArray in the new path.

How about only creating a vao if none is bound, and otherwise modify the
currently bound vao, and then resetting it in the destructor? Then we keep
existing semantics.

[Zhenyao Mo, 2016/08/18 00:54:19]

On ES3, there is a default vao, on desktop GL, there is not, but we always
create one on command buffer initialization time.  So the only situation that
the Get* calls here cause problem is the test.

So what about I leave this function totally untouched, but create a vao and bind
it in the test?

[piman, 2016/08/18 00:57:46]

On desktop only then? VAOs are not available on ES2 I believe. Is so, sgtm

[Ken Russell (switch to Gerrit), 2016/08/18 01:32:52]

It sounds good to me to modify the test rather than this class, if this scoped
binder would otherwise work in all cases where it's used in Chromium.

+  } else {
+    glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_BUFFER_BINDING, &buffer_);
+    glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_ENABLED, &enabled_);
+
+    glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_SIZE, &size_);
+    glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_TYPE, &type_);
+    glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_NORMALIZED, &normalized_);
+    glGetVertexAttribiv(index, GL_VERTEX_ATTRIB_ARRAY_STRIDE, &stride_);
+    glGetVertexAttribPointerv(index, GL_VERTEX_ATTRIB_ARRAY_POINTER, &pointer_);
+  }
 
+  glEnableVertexAttribArray(index);
   glVertexAttribPointer(index, size, type, normalized, stride, pointer);
 }
 
 ScopedVertexAttribArray::~ScopedVertexAttribArray() {
-  ScopedBufferBinder buffer_binder(GL_ARRAY_BUFFER, buffer_);
-  glVertexAttribPointer(index_, size_, type_, normalized_, stride_, pointer_);
-  if (enabled_ == GL_FALSE) {
-    glDisableVertexAttribArray(index_);
+  if (vao_ > 0) {
+    glBindVertexArrayOES(current_vao_);
+    glDeleteVertexArraysOES(1, &vao_);
+    vao_ = 0u;
+  } else {
+    ScopedBufferBinder buffer_binder(GL_ARRAY_BUFFER, buffer_);
+    glVertexAttribPointer(index_, size_, type_, normalized_, stride_, pointer_);
+    if (enabled_ == GL_FALSE) {
+      glDisableVertexAttribArray(index_);
+    }
   }
 }