StreamTextureImages can now override a Texture's service id

gpu/command_buffer/service/texture_manager.cc

Index: gpu/command_buffer/service/texture_manager.cc
diff --git a/gpu/command_buffer/service/texture_manager.cc b/gpu/command_buffer/service/texture_manager.cc
index 7b7e36ed74ecf509c3526872d29ea7db3e89e7b1..ba75e2e0ecfe21edb24c0bb40efef93582f3e6df 100644
--- a/gpu/command_buffer/service/texture_manager.cc
+++ b/gpu/command_buffer/service/texture_manager.cc
@@ -1477,6 +1477,7 @@ void Texture::SetLevelImageInternal(GLenum target,
                                     gl::GLImage* image,
                                     GLStreamTextureImage* stream_texture_image,
                                     ImageState state) {
+  DCHECK(!stream_texture_image || stream_texture_image == image);
   DCHECK_GE(level, 0);
   size_t face_index = GLES2Util::GLTargetToFaceIndex(target);
   DCHECK_LT(static_cast<size_t>(face_index),
@@ -1487,8 +1488,15 @@ void Texture::SetLevelImageInternal(GLenum target,
       face_infos_[face_index].level_infos[level];
   DCHECK_EQ(info.target, target);
   DCHECK_EQ(info.level, level);
-  info.image = image;
-  info.stream_texture_image = stream_texture_image;

[liberato (no reviews please), 2016/06/02 14:37:02]

wow -- this was fragile before, and i just realized how bad it was.  this kept
setting and resetting the stream texture during normal drawing. DoCopyTexImage
-> SetLevelImage -> here with |stream_texture_image| == nullptr, then
avda_codec_image (hopefully) gets far enough in CopyTexImage to re-set it
properly.  that would leave the stream texture unset if CopyTexImage was called
on an early-out case, such as CopyTextureCHROMIUM.

i think that it would get back to normal on the next browser compositor frame,
but the texture matrix wouldn't be set properly until it was.  that happens
before the associated CopyTexImage, so one frame would be wrong at least, i
think.

yay for if() :)

+
+  if (info.image != image) {

[liberato (no reviews please), 2016/06/02 14:37:02]

please update the tests (texture_manager_unittest.cc) for the new names, and
include a to check for this case, and the new parameter in
setLeveStreamTextureImage.  there are already tests for the unowned service id
to base it on.

[watk, 2016/06/03 01:09:46]

Done.

+    info.image = image;
+    info.stream_texture_image = stream_texture_image;
+    // If |image| is different, then |stream_texture_image| must also be
+    // different, so clear the stream texture service id because it's no longer

[liberato (no reviews please), 2016/06/02 14:37:02]

nit: the comment is a little misleading.  we don't know that it's no longer
valid.  it's because the stream texture manages the lifetime of the service id,
and we can't guarantee that it's still valid.  the extra context might be
helpful just so folks realize that it's a security thing.

[watk, 2016/06/03 01:09:46]

Done.

+    // valid.
+    SetStreamTextureServiceId(0);
+  }
   info.image_state = state;
 
   UpdateCanRenderCondition();
@@ -1506,8 +1514,10 @@ void Texture::SetLevelImage(GLenum target,
 void Texture::SetLevelStreamTextureImage(GLenum target,
                                          GLint level,
                                          GLStreamTextureImage* image,
-                                         ImageState state) {
+                                         ImageState state,
+                                         GLuint service_id) {
   SetLevelImageInternal(target, level, image, image, state);
+  SetStreamTextureServiceId(service_id);
 }
 
 const Texture::LevelInfo* Texture::GetLevelInfo(GLint target,
@@ -1619,7 +1629,7 @@ bool Texture::CanRenderTo(const FeatureInfo* feature_info, GLint level) const {
   return (color_renderable || depth_renderable || stencil_renderable);
 }
 
-void Texture::SetUnownedServiceId(GLuint service_id) {
+void Texture::SetStreamTextureServiceId(GLuint service_id) {
   GLuint new_service_id = service_id;
 
   // Take no action if this isn't an OES_EXTERNAL texture.
@@ -2100,9 +2110,11 @@ void TextureManager::SetLevelStreamTextureImage(TextureRef* ref,
                                                 GLenum target,
                                                 GLint level,
                                                 GLStreamTextureImage* image,
-                                                Texture::ImageState state) {
+                                                Texture::ImageState state,
+                                                GLuint service_id) {
   DCHECK(ref);
-  ref->texture()->SetLevelStreamTextureImage(target, level, image, state);
+  ref->texture()->SetLevelStreamTextureImage(target, level, image, state,
+                                             service_id);
 }
 
 size_t TextureManager::GetSignatureSize() const {