Reset unpack buffer in FeatureInfo if PBOs are supported by the driver

We need to reset GL_PIXEL_UNPACK_BUFFER in FeatureInfo initialization if it is
non-0. We should always do that regardless of whether or not the current decoder
exposes them, because with shared contexts it is possible that another decoder
uses ES3 whereas the current one doesn't.

Bug: 905519
Change-Id: I2c2f457fcdd76ca4812ac8d71c8f77d694506a57
Reviewed-on: https://chromium-review.googlesource.com/c/1340973
Commit-Queue: Antoine Labour <piman@chromium.org>
Commit-Queue: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609050}

gpu/command_buffer/service/feature_info.cc

@@ -428,16 +428,18 @@ void FeatureInfo::InitializeFeatures() {
 
   bool enable_es3 = IsWebGL2OrES3OrHigherContext();
 
-  // Pixel buffer bindings can be manipulated by the client if ES3 is enabled or
-  // the GL_NV_pixel_buffer_object extension is exposed by ANGLE when using the
-  // passthrough command decoder
-  bool pixel_buffers_exposed =
-      enable_es3 || gfx::HasExtension(extensions, "GL_NV_pixel_buffer_object");
+  // Really it's part of core OpenGL 2.1 and up, but let's assume the
+  // extension is still advertised.
+  bool has_pixel_buffers =
+      gl_version_info_->is_es3 || gl_version_info_->is_desktop_core_profile ||
+      gfx::HasExtension(extensions, "GL_ARB_pixel_buffer_object") ||
+      gfx::HasExtension(extensions, "GL_NV_pixel_buffer_object");
 
-  // Both decoders may bind pixel buffers if exposing an ES3 or WebGL 2 context
-  // and the passthrough command decoder may also bind PBOs if
-  // NV_pixel_buffer_object is exposed.
-  ScopedPixelUnpackBufferOverride scoped_pbo_override(pixel_buffers_exposed, 0);
+  // If ES3 or pixel buffer objects are enabled by the driver, we have to assume
+  // the unpack buffer binding may be changed on the underlying context. This is
+  // true whether or not this particular decoder exposes PBOs, as it could be
+  // changed on another decoder that does expose them.
+  ScopedPixelUnpackBufferOverride scoped_pbo_override(has_pixel_buffers, 0);
 
   AddExtensionString("GL_ANGLE_translated_shader_source");
   AddExtensionString("GL_CHROMIUM_async_pixel_transfers");
@@ -1266,13 +1268,6 @@ void FeatureInfo::InitializeFeatures() {
       gfx::HasExtension(extensions, "GL_ARB_map_buffer_range") ||
       gfx::HasExtension(extensions, "GL_EXT_map_buffer_range");
 
-  // Really it's part of core OpenGL 2.1 and up, but let's assume the
-  // extension is still advertised.
-  bool has_pixel_buffers =
-      gl_version_info_->is_es3 || gl_version_info_->is_desktop_core_profile ||
-      gfx::HasExtension(extensions, "GL_ARB_pixel_buffer_object") ||
-      gfx::HasExtension(extensions, "GL_NV_pixel_buffer_object");
-
   // We will use either glMapBuffer() or glMapBufferRange() for async readbacks.
   if (has_pixel_buffers && ui_gl_fence_works &&
       !workarounds_.disable_async_readpixels) {

gpu/command_buffer/service/test_helper.cc

@@ -517,7 +517,8 @@ void TestHelper::SetupFeatureInfoInitExpectationsWithGLVersion(
       .WillOnce(Return(reinterpret_cast<const uint8_t*>(gl_renderer)))
       .RetiresOnSaturation();
 
-  if (enable_es3 ||
+  if (gl_info.is_es3 || gl_info.is_desktop_core_profile ||
+      gfx::HasExtension(extension_set, "GL_ARB_pixel_buffer_object") ||
       gfx::HasExtension(extension_set, "GL_NV_pixel_buffer_object")) {
     EXPECT_CALL(*gl, GetIntegerv(GL_PIXEL_UNPACK_BUFFER_BINDING, _))
       .WillOnce(SetArgPointee<1>(0))