Fix gl_unittests on Mac core profile.

ui/gl/scoped_binders.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gl/scoped_binders.h"
 #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 {
 
 ScopedFrameBufferBinder::ScopedFrameBufferBinder(unsigned int fbo)
     : state_restorer_(!GLContext::GetCurrent()
                           ? NULL
                           : GLContext::GetCurrent()->GetGLStateRestorer()),
       old_fbo_(-1) {
   if (!state_restorer_)
     glGetIntegerv(GL_FRAMEBUFFER_BINDING, &old_fbo_);
@@ skipping 74 matching lines @@
                                                  char normalized,
                                                  int stride,
                                                  const void* pointer)
     : buffer_(0),
       enabled_(GL_FALSE),
       index_(index),
       size_(-1),
       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_);
+      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);
-
-  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_);
-
   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_);
+    }
   }
 }
 
 ScopedBufferBinder::ScopedBufferBinder(unsigned int target, unsigned int id)
     : target_(target), old_id_(-1) {
   GLenum target_getter = 0;
   switch (target) {
     case GL_ARRAY_BUFFER:
       target_getter = GL_ARRAY_BUFFER_BINDING;
       break;
@@ skipping 38 matching lines @@
 
 ScopedCapability::~ScopedCapability() {
   if (enabled_ == GL_TRUE) {
     glEnable(capability_);
   } else {
     glDisable(capability_);
   }
 }
 
 }  // namespace gl