Fix GL extension filtering so that extension bits are set correctly.

ui/gl/gl_gl_api_implementation.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/gl_gl_api_implementation.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/command_line.h"
@@ skipping 316 matching lines @@
 
 void SetGLApiToNoContext() {
   SetGLApi(g_no_context_gl);
 }
 
 const GLVersionInfo* GetGLVersionInfo() {
   return g_version_info;
 }
 
 void InitializeDynamicGLBindingsGL(GLContext* context) {
+  g_real_gl->InitializeFilteredExtensions();
   g_driver_gl.InitializeCustomDynamicBindings(context);
   DCHECK(context && context->IsCurrent(NULL) && !g_version_info);
   g_real_gl->InitializeWithContext();
   g_version_info = new GLVersionInfo(
       context->GetGLVersion().c_str(),
       context->GetGLRenderer().c_str(),
       context->GetExtensions().c_str());
 }
 
 void InitializeDebugGLBindingsGL() {
@@ skipping 50 matching lines @@
 }
 
 GLApiBase::~GLApiBase() {
 }
 
 void GLApiBase::InitializeBase(DriverGL* driver) {
   driver_ = driver;
 }
 
 RealGLApi::RealGLApi() {
+#if DCHECK_IS_ON()
+  filtered_exts_initialized_ = false;
+#endif
 }
 
 RealGLApi::~RealGLApi() {
 }
 
 void RealGLApi::Initialize(DriverGL* driver) {
   InitializeWithCommandLine(driver, base::CommandLine::ForCurrentProcess());
 }
 
 void RealGLApi::InitializeWithCommandLine(DriverGL* driver,
                                           base::CommandLine* command_line) {
   DCHECK(command_line);
   InitializeBase(driver);
 
   const std::string disabled_extensions = command_line->GetSwitchValueASCII(
       switches::kDisableGLExtensions);
   if (!disabled_extensions.empty()) {
     disabled_exts_ = base::SplitString(
         disabled_extensions, ", ;",
         base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
   }
 }
 
 void RealGLApi::InitializeWithContext() {

[no sievers, 2015/07/23 23:45:57]

So this method can go?

[Tobias Sargeant, 2015/07/24 09:29:14]

Yes.

-  InitializeFilteredExtensions();
 }
 
 void RealGLApi::glGetIntegervFn(GLenum pname, GLint* params) {
-  if (!filtered_exts_.empty() && pname == GL_NUM_EXTENSIONS) {
+  if (pname == GL_NUM_EXTENSIONS && disabled_exts_.size()) {
+#if DCHECK_IS_ON()

[no sievers, 2015/07/23 23:45:57]

nit: #if DCHECK_IS_ON is redundant here and below

[Tobias Sargeant, 2015/07/24 09:29:15]

I thought so too, but it turns out that because it DCHECKS a bool that's not
compiled in when DCHECK_IS_ON() == 0, it fails to compile on windows. See:

http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...

+    DCHECK(filtered_exts_initialized_);
+#endif
     *params = static_cast<GLint>(filtered_exts_.size());
   } else {
     GLApiBase::glGetIntegervFn(pname, params);
   }
 }
 
 const GLubyte* RealGLApi::glGetStringFn(GLenum name) {
-  if (!filtered_exts_.empty() && name == GL_EXTENSIONS) {
+  if (name == GL_EXTENSIONS && disabled_exts_.size()) {
+#if DCHECK_IS_ON()
+    DCHECK(filtered_exts_initialized_);
+#endif
     return reinterpret_cast<const GLubyte*>(filtered_exts_str_.c_str());
   }
   return GLApiBase::glGetStringFn(name);
 }
 
 const GLubyte* RealGLApi::glGetStringiFn(GLenum name, GLuint index) {
-  if (!filtered_exts_str_.empty() && name == GL_EXTENSIONS) {
+  if (name == GL_EXTENSIONS && disabled_exts_.size()) {
+#if DCHECK_IS_ON()
+    DCHECK(filtered_exts_initialized_);
+#endif
     if (index >= filtered_exts_.size()) {
       return NULL;
     }
     return reinterpret_cast<const GLubyte*>(filtered_exts_[index].c_str());
   }
   return GLApiBase::glGetStringiFn(name, index);
 }
 
 void RealGLApi::glFlushFn() {
   GLApiBase::glFlushFn();
 }
 
 void RealGLApi::glFinishFn() {
   GLApiBase::glFinishFn();
 }
 
 void RealGLApi::InitializeFilteredExtensions() {
-  if (!disabled_exts_.empty() && filtered_exts_.empty()) {
-    DCHECK(filtered_exts_.empty() && filtered_exts_str_.empty());
-    // Fill in filtered_exts_ vector first.
+  if (disabled_exts_.size()) {
+    filtered_exts_.clear();
     if (gfx::GetGLImplementation() !=
         gfx::kGLImplementationDesktopGLCoreProfile) {
-      const char* gl_extensions = reinterpret_cast<const char*>(
-          GLApiBase::glGetStringFn(GL_EXTENSIONS));
-      if (gl_extensions) {
-        filtered_exts_ = base::SplitString(
-            gl_extensions, " ", base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
-      }
+      filtered_exts_str_ =
+          FilterGLExtensionList(reinterpret_cast<const char*>(

[no sievers, 2015/07/23 23:45:57]

Hmm a bit back and forth going on in here (vector to string, then back to vector
below).

[Tobias Sargeant, 2015/07/24 09:29:14]

I agree, but I felt that the smaller code offset the inefficiency - especially
given that this is meant to happen only at startup.

+                                    GLApiBase::glGetStringFn(GL_EXTENSIONS)),
+                                disabled_exts_);
+      base::SplitString(filtered_exts_str_, ' ', &filtered_exts_);
     } else {
       GLint num_extensions = 0;
       GLApiBase::glGetIntegervFn(GL_NUM_EXTENSIONS, &num_extensions);
       for (GLint i = 0; i < num_extensions; ++i) {
         const char* gl_extension = reinterpret_cast<const char*>(
             GLApiBase::glGetStringiFn(GL_EXTENSIONS, i));
         DCHECK(gl_extension != NULL);
-        filtered_exts_.push_back(gl_extension);
+        if (std::find(disabled_exts_.begin(), disabled_exts_.end(),
+                      gl_extension) == disabled_exts_.end()) {
+          filtered_exts_.push_back(gl_extension);
+        }
       }
+      filtered_exts_str_ = base::JoinString(filtered_exts_, " ");
     }
-
-    // Filter out extensions from the command line.
-    for (const std::string& disabled_ext : disabled_exts_) {
-      filtered_exts_.erase(std::remove(filtered_exts_.begin(),
-                                       filtered_exts_.end(),
-                                       disabled_ext),
-                           filtered_exts_.end());
-    }
-
-    // Construct filtered extensions string for GL_EXTENSIONS string lookups.
-    filtered_exts_str_ = base::JoinString(filtered_exts_, " ");
+#if DCHECK_IS_ON()
+    filtered_exts_initialized_ = true;
+#endif
   }
 }
 
 TraceGLApi::~TraceGLApi() {
 }
 
 NoContextGLApi::NoContextGLApi() {
 }
 
 NoContextGLApi::~NoContextGLApi() {
@@ skipping 107 matching lines @@
 ScopedSetGLToRealGLApi::ScopedSetGLToRealGLApi()
     : old_gl_api_(GetCurrentGLApi()) {
   SetGLToRealGLApi();
 }
 
 ScopedSetGLToRealGLApi::~ScopedSetGLToRealGLApi() {
   SetGLApi(old_gl_api_);
 }
 
 }  // namespace gfx