Prevent crash when using getExtension while a PBO is bound.

gpu/command_buffer/service/test_helper.cc

Index: gpu/command_buffer/service/test_helper.cc
diff --git a/gpu/command_buffer/service/test_helper.cc b/gpu/command_buffer/service/test_helper.cc
index b0714d9454784d63e1f88ae56bf0ca4d2ef87655..617a0ef7d952fe726484429fdbae6322b98f5376 100644
--- a/gpu/command_buffer/service/test_helper.cc
+++ b/gpu/command_buffer/service/test_helper.cc
@@ -34,6 +34,7 @@ using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::SetArrayArgument;
 using ::testing::SetArgumentPointee;
+using ::testing::SetArgPointee;
 using ::testing::StrEq;
 using ::testing::StrictMock;
 
@@ -331,13 +332,15 @@ void TestHelper::SetupContextGroupInitExpectations(
     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);
 
+  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();
@@ -468,7 +471,8 @@ void TestHelper::SetupContextGroupInitExpectations(
 
 void TestHelper::SetupFeatureInfoInitExpectations(::gl::MockGLInterface* gl,
                                                   const char* extensions) {
-  SetupFeatureInfoInitExpectationsWithGLVersion(gl, extensions, "", "");
+  SetupFeatureInfoInitExpectationsWithGLVersion(gl, extensions, "", "",
+      CONTEXT_TYPE_OPENGLES2);
 }
 
 void TestHelper::SetupFeatureInfoInitExpectationsWithGLVersion(
@@ -476,6 +480,7 @@ void TestHelper::SetupFeatureInfoInitExpectationsWithGLVersion(
     const char* extensions,
     const char* gl_renderer,
     const char* gl_version,
+    ContextType context_type,
     bool enable_es3) {
   InSequence sequence;
 
@@ -514,6 +519,14 @@ void TestHelper::SetupFeatureInfoInitExpectationsWithGLVersion(
       .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"))) {