Prevent crash when using getExtension while a PBO is bound.

gpu/command_buffer/service/feature_info.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/command_buffer/service/feature_info.h"
 
 #include <stddef.h>
 
 #include <set>
 #include <vector>
@@ skipping 62 matching lines @@
   }
 
   const std::set<std::string>& GetImpl() {
     return string_set_;
   }
 
  private:
   std::set<std::string> string_set_;
 };
 
+class ScopedPixelUnpackBufferOverride {
+ public:
+  explicit ScopedPixelUnpackBufferOverride(
+      ContextType context_type,
+      GLuint binding_override)
+      : original_binding_(0) {
+    if (!(context_type == CONTEXT_TYPE_WEBGL1 ||
+          context_type == CONTEXT_TYPE_OPENGLES2)) {
+      glGetIntegerv(GL_PIXEL_UNPACK_BUFFER_BINDING, &original_binding_);
+      glBindBuffer(GL_PIXEL_UNPACK_BUFFER, binding_override);

[Zhenyao Mo, 2016/08/29 19:18:48]

A minor optimization: if original_bindings_ == binding_override, then no
BindBuffer() calls.

This should also simplify the test expectations change.

[Ken Russell (switch to Gerrit), 2016/08/29 19:36:17]

Good point Mo. See also below.

[Kai Ninomiya, 2016/08/30 23:51:02]

Done.

+    }
+  }
+
+  ~ScopedPixelUnpackBufferOverride() {
+    if (original_binding_) {

[Ken Russell (switch to Gerrit), 2016/08/29 19:36:17]

Similar test could be done here to avoid restoring the binding if it was
unchanged.

[Kai Ninomiya, 2016/08/30 23:51:02]

Done.

+      glBindBuffer(GL_PIXEL_UNPACK_BUFFER,
+                   static_cast<GLuint>(original_binding_));
+    }
+  }
+
+ private:
+    GLint original_binding_;
+};
+
 }  // anonymous namespace.
 
 FeatureInfo::FeatureFlags::FeatureFlags()
     : chromium_framebuffer_multisample(false),
       chromium_sync_query(false),
       use_core_framebuffer_multisample(false),
       multisampled_render_to_texture(false),
       use_img_for_multisampled_render_to_texture(false),
       chromium_screen_space_antialiasing(false),
       use_chromium_screen_space_antialiasing_via_shaders(false),
@@ skipping 652 matching lines @@
     validators_.pixel_type.AddValue(GL_HALF_FLOAT_OES);
     validators_.read_pixel_type.AddValue(GL_HALF_FLOAT_OES);
     AddExtensionString("GL_OES_texture_half_float");
     if (enable_texture_half_float_linear) {
       oes_texture_half_float_linear_available_ = true;
       if (!disallowed_features_.oes_texture_half_float_linear)
         EnableOESTextureHalfFloatLinear();
     }
   }
 
+  ScopedPixelUnpackBufferOverride scoped_pbo_override(context_type_, 0);

[Ken Russell (switch to Gerrit), 2016/08/29 19:36:17]

I think you should move this up to the top of this method,
FeatureInfo::InitializeFeatures. Otherwise, the same bug is likely to crop up
again if someone inserts any extension checking code which calls TexImage2D.

[Kai Ninomiya, 2016/08/30 23:51:02]

Done.

+
   if (may_enable_chromium_color_buffer_float) {
     static_assert(GL_RGBA32F_ARB == GL_RGBA32F &&
                   GL_RGBA32F_EXT == GL_RGBA32F &&
                   GL_RGB32F_ARB == GL_RGB32F &&
                   GL_RGB32F_EXT == GL_RGB32F,
                   "sized float internal format variations must match");
     // We don't check extension support beyond ARB_texture_float on desktop GL,
     // and format support varies between GL configurations. For example, spec
     // prior to OpenGL 3.0 mandates framebuffer support only for one
     // implementation-chosen format, and ES3.0 EXT_color_buffer_float does not
@@ skipping 699 matching lines @@
   if (pos == std::string::npos) {
     extensions_ += (extensions_.empty() ? "" : " ") + str;
   }
 }
 
 FeatureInfo::~FeatureInfo() {
 }
 
 }  // namespace gles2
 }  // namespace gpu