Prevent crash when using getExtension while a PBO is bound.

gpu/command_buffer/service/test_helper.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/test_helper.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 16 matching lines @@
 
 using ::testing::_;
 using ::testing::DoAll;
 using ::testing::InSequence;
 using ::testing::MatcherCast;
 using ::testing::Pointee;
 using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SetArrayArgument;
 using ::testing::SetArgumentPointee;
+using ::testing::SetArgPointee;
 using ::testing::StrEq;
 using ::testing::StrictMock;
 
 namespace gpu {
 namespace gles2 {
 
 namespace {
 
 template<typename T>
 T ConstructShaderVariable(
@@ skipping 277 matching lines @@
   EXPECT_CALL(*gl, DeleteTextures(TextureManager::kNumDefaultTextures, _))
       .Times(1)
       .RetiresOnSaturation();
 }
 
 void TestHelper::SetupContextGroupInitExpectations(
     ::gl::MockGLInterface* gl,
     const DisallowedFeatures& disallowed_features,
     const char* extensions,
     const char* gl_version,
+    ContextType context_type,
     bool bind_generates_resource) {
   InSequence sequence;
 
-  SetupFeatureInfoInitExpectationsWithGLVersion(gl, extensions, "", gl_version);
+  gl::GLVersionInfo gl_info(gl_version, "", extensions);
 
-  gl::GLVersionInfo gl_info(gl_version, "", extensions);
+  SetupFeatureInfoInitExpectationsWithGLVersion(gl, extensions, "", gl_version,
+      context_type, gl_info.is_es3);

[Zhenyao Mo, 2016/08/31 00:15:19]

gl_info.is_es3 looks incorrect.  It indicates whether the underlying driver is
es3, not whether es3 is enabled in command buffer. I think you need to add an
extra parameter to SetupContextGroupInitExpectations() to indicate that.

[Kai Ninomiya, 2016/08/31 00:47:59]

Hm... okay. I can pass in that extra parameter easily for the 2 tests which call
SetupContextGroupInitExpectations. But it's also called from
GLES2DecoderTestBase::InitDecoderFromCommandLine
(https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_dec...).

Despite the `GLES2*` name, it seems that I can't assume `enable_es3` should
always be false here. Should I propagate this change all the way up to the
individual tests? Or can I compute `enable_es3` from, like, `(init.context_type
== CONTEXT_TYPE_WEBGL2 || init.context_type == CONTEXT_TYPE_OPENGLES3)`?

[Zhenyao Mo, 2016/08/31 00:51:40]

Yeah that should be fine. In testing, if we specify the context type, it is
always obeyed.

[Kai Ninomiya, 2016/08/31 01:11:17]

Okay. I also quickly simulated removing `enable_es3` entirely (i.e., replacing
it with context_type). But that does not work: `context_type` is GLES2 and
`enable_es3` is true at one point in TextureManagerTest::SetupFeatureInfo
(https://cs.chromium.org/chromium/src/gpu/command_buffer/service/texture_manag...).
Is that okay?

[Zhenyao Mo, 2016/08/31 01:18:30]

I am sorry you stepped on these issues.  Such inconsistencies are definitely
poor designs - originally the command buffer was only designed for ES2.  So to
force it to support ES3 without a major re-design, this is what we ended up
with.

 
   EXPECT_CALL(*gl, GetIntegerv(GL_MAX_RENDERBUFFER_SIZE, _))
       .WillOnce(SetArgumentPointee<1>(kMaxRenderbufferSize))
       .RetiresOnSaturation();
   if (strstr(extensions, "GL_EXT_framebuffer_multisample") ||
       strstr(extensions, "GL_EXT_multisampled_render_to_texture") ||
       gl_info.is_es3 || gl_info.is_desktop_core_profile) {
     EXPECT_CALL(*gl, GetIntegerv(GL_MAX_SAMPLES, _))
         .WillOnce(SetArgumentPointee<1>(kMaxSamples))
         .RetiresOnSaturation();
@@ skipping 111 matching lines @@
       .RetiresOnSaturation();
 
   bool use_default_textures = bind_generates_resource;
   SetupTextureManagerInitExpectations(
       gl, false, gl_info.is_desktop_core_profile, extensions,
       use_default_textures);
 }
 
 void TestHelper::SetupFeatureInfoInitExpectations(::gl::MockGLInterface* gl,
                                                   const char* extensions) {
-  SetupFeatureInfoInitExpectationsWithGLVersion(gl, extensions, "", "");
+  SetupFeatureInfoInitExpectationsWithGLVersion(gl, extensions, "", "",
+      CONTEXT_TYPE_OPENGLES2);
 }
 
 void TestHelper::SetupFeatureInfoInitExpectationsWithGLVersion(
     ::gl::MockGLInterface* gl,
     const char* extensions,
     const char* gl_renderer,
     const char* gl_version,
+    ContextType context_type,
     bool enable_es3) {
   InSequence sequence;
 
   EXPECT_CALL(*gl, GetString(GL_VERSION))
       .WillOnce(Return(reinterpret_cast<const uint8_t*>(gl_version)))
       .RetiresOnSaturation();
 
   // Persistent storage is needed for the split extension string.
   split_extensions_.clear();
   if (extensions) {
@@ skipping 18 matching lines @@
         .RetiresOnSaturation();
   }
 
   EXPECT_CALL(*gl, GetString(GL_VERSION))
       .WillOnce(Return(reinterpret_cast<const uint8_t*>(gl_version)))
       .RetiresOnSaturation();
   EXPECT_CALL(*gl, GetString(GL_RENDERER))
       .WillOnce(Return(reinterpret_cast<const uint8_t*>(gl_renderer)))
       .RetiresOnSaturation();
 
+  if (!(context_type == CONTEXT_TYPE_WEBGL1 ||
+        context_type == CONTEXT_TYPE_OPENGLES2) &&
+      gl_info.is_es3_capable && enable_es3) {
+    EXPECT_CALL(*gl, GetIntegerv(GL_PIXEL_UNPACK_BUFFER_BINDING, _))
+      .WillOnce(SetArgPointee<1>(0))
+      .RetiresOnSaturation();
+  }
+
   if ((strstr(extensions, "GL_ARB_texture_float") ||
        gl_info.is_desktop_core_profile) ||
       (gl_info.is_es3 && strstr(extensions, "GL_EXT_color_buffer_float"))) {
     static const GLuint tx_ids[] = {101, 102};
     static const GLuint fb_ids[] = {103, 104};
     const GLsizei width = 16;
     EXPECT_CALL(*gl, GetIntegerv(GL_FRAMEBUFFER_BINDING, _))
         .WillOnce(SetArgumentPointee<1>(fb_ids[0]))
         .RetiresOnSaturation();
     EXPECT_CALL(*gl, GetIntegerv(GL_TEXTURE_BINDING_2D, _))
@@ skipping 653 matching lines @@
     GLenum precision,
     bool static_use,
     const std::string& name) {
   return ConstructShaderVariable<sh::OutputVariable>(
       type, array_size, precision, static_use, name);
 }
 
 }  // namespace gles2
 }  // namespace gpu