Crash fix for when running under the Android emulator

gpu/config/gpu_info_collector_android.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 "gpu/config/gpu_info_collector.h"
 
 #include "base/android/build_info.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
@@ skipping 198 matching lines @@
       gpu_info->gl_extensions.find("GL_EXT_robustness") != std::string::npos ||
       gpu_info->gl_extensions.find("GL_KHR_robustness") != std::string::npos ||
       gpu_info->gl_extensions.find("GL_ARB_robustness") != std::string::npos;
 
   if (supports_robustness) {
     glGetIntegervFn(
         GL_RESET_NOTIFICATION_STRATEGY_ARB,
         reinterpret_cast<GLint*>(&gpu_info->gl_reset_notification_strategy));
   }
 
-  std::string glsl_version_string =
-      reinterpret_cast<const char*>(glGetStringFn(GL_SHADING_LANGUAGE_VERSION));
+  std::string glsl_version_string;
+  if (const char* glsl_version_cstring = reinterpret_cast<const char*>(

[Tobias Sargeant, 2015/09/22 12:12:03]

Defining glsl_version_cstring in the if seems a little odd to me, but grepping
through the chromium source I can see other instances of this pattern, and I
can't see any comment in the style guide. Probably ok to do this.

In the case where glGetString returns NULL, I would expect a GL error to be set.
You probably want to call glGetError() to clear that error (whether you log it,
or just drop it is probably up to you) so that it doesn't hang around and
potentially cause later code to fail.

[davve, 2015/09/23 10:24:51]

I'm open to other suggestions. It felt unnecessary to keep the const char*
variable in scope.
 
While I agree with your logic, glGetError() returns GL_NO_ERROR right after
glGetStringFn returned NULL in this case.

Fwiw, Dart seemed to have encountered the same bug/issue as I have:

https://github.com/MarkBennett/dart/blob/9246c00961f137c2c0cfdfc292eb207f870a...

Note that this isn't from the official dart repo and is quite old. The entire
module it belongs to seems to have been removed on trunk.

What do you think about hard-coding the value in this case?

And sure I could add a glGetError() call, but it would address a very
theoretical problem.

+          glGetStringFn(GL_SHADING_LANGUAGE_VERSION)))
+    glsl_version_string = glsl_version_cstring;
 
   std::string glsl_version = GetVersionFromString(glsl_version_string).first;
   gpu_info->pixel_shader_version = glsl_version;
   gpu_info->vertex_shader_version = glsl_version;
 
   if (curr_display != EGL_NO_DISPLAY &&
       curr_context != EGL_NO_CONTEXT) {
     eglMakeCurrentFn(curr_display, curr_draw_surface, curr_read_surface,
                      curr_context);
   } else {
@@ skipping 49 matching lines @@
   gpu_info->gpu.device_string = gpu_info->gl_renderer;
   return kCollectInfoSuccess;
 }
 
 void MergeGPUInfo(GPUInfo* basic_gpu_info,
                   const GPUInfo& context_gpu_info) {
   MergeGPUInfoGL(basic_gpu_info, context_gpu_info);
 }
 
 }  // namespace gpu