Report the SurfaceTexture tranform matrix to CC timely

content/common/gpu/stream_texture_android.cc

Index: content/common/gpu/stream_texture_android.cc
diff --git a/content/common/gpu/stream_texture_android.cc b/content/common/gpu/stream_texture_android.cc
index 17d841d8eb1b43a3382be5a5fc20f976af5055ba..082f33250bc5d0f0d95a2cc160055241f848bf65 100644
--- a/content/common/gpu/stream_texture_android.cc
+++ b/content/common/gpu/stream_texture_android.cc
@@ -63,8 +63,6 @@ StreamTexture::StreamTexture(GpuCommandBufferStub* owner_stub,
                              uint32_t texture_id)
     : surface_texture_(gfx::SurfaceTexture::Create(texture_id)),
       size_(0, 0),
-      has_valid_frame_(false),
-      has_pending_frame_(false),
       owner_stub_(owner_stub),
       route_id_(route_id),
       has_listener_(false),
@@ -140,49 +138,6 @@ scoped_ptr<ui::ScopedMakeCurrent> StreamTexture::MakeStubCurrent() {
   return scoped_make_current;
 }
 
-void StreamTexture::UpdateTexImage() {
-  DCHECK(surface_texture_.get());
-  DCHECK(owner_stub_);
-
-  if (!has_pending_frame_) return;
-
-  scoped_ptr<ui::ScopedMakeCurrent> scoped_make_current(MakeStubCurrent());
-
-  surface_texture_->UpdateTexImage();
-
-  has_valid_frame_ = true;
-  has_pending_frame_ = false;
-
-  float mtx[16];
-  surface_texture_->GetTransformMatrix(mtx);
-
-  if (memcmp(current_matrix_, mtx, sizeof(mtx)) != 0) {
-    memcpy(current_matrix_, mtx, sizeof(mtx));
-
-    if (has_listener_) {
-      GpuStreamTextureMsg_MatrixChanged_Params params;
-      memcpy(&params.m00, mtx, sizeof(mtx));
-      owner_stub_->channel()->Send(
-          new GpuStreamTextureMsg_MatrixChanged(route_id_, params));
-    }
-  }
-
-  if (scoped_make_current.get()) {
-    // UpdateTexImage() implies glBindTexture().
-    // The cmd decoder takes care of restoring the binding for this GLImage as
-    // far as the current context is concerned, but if we temporarily change
-    // it, we have to keep the state intact in *that* context also.
-    const gpu::gles2::ContextState* state =
-        owner_stub_->decoder()->GetContextState();
-    const gpu::gles2::TextureUnit& active_unit =
-        state->texture_units[state->active_texture_unit];
-    glBindTexture(GL_TEXTURE_EXTERNAL_OES,
-                  active_unit.bound_texture_external_oes.get()
-                      ? active_unit.bound_texture_external_oes->service_id()
-                      : 0);
-  }
-}
-
 bool StreamTexture::CopyTexImage(unsigned target) {
   if (target == GL_TEXTURE_2D) {
     glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, size_.width(), size_.height(), 0,
@@ -205,8 +160,6 @@ bool StreamTexture::CopyTexImage(unsigned target) {
   if (static_cast<unsigned>(texture_id) != texture_id_)
     return false;
 
-  UpdateTexImage();
-
   TextureManager* texture_manager =
       owner_stub_->decoder()->GetContextGroup()->texture_manager();
   gpu::gles2::Texture* texture =
@@ -223,11 +176,44 @@ bool StreamTexture::CopyTexImage(unsigned target) {
 }
 
 void StreamTexture::OnFrameAvailable() {
-  has_pending_frame_ = true;
-  if (has_listener_ && owner_stub_) {
+  if (!owner_stub_)
+    return;
+
+  if (has_listener_) {
     owner_stub_->channel()->Send(
         new GpuStreamTextureMsg_FrameAvailable(route_id_));

[liberato (no reviews please), 2016/02/02 15:49:44]

if one moves this later (after matrixchanged), does it remove the race?

these are both handled by a listener bound to the CC thread.  how can they race
then?

   }
+  scoped_ptr<ui::ScopedMakeCurrent> scoped_make_current(MakeStubCurrent());
+
+  surface_texture_->UpdateTexImage();
+
+  float mtx[16];
+  surface_texture_->GetTransformMatrix(mtx);
+
+  if (memcmp(current_matrix_, mtx, sizeof(mtx)) != 0) {
+    memcpy(current_matrix_, mtx, sizeof(mtx));
+
+    if (has_listener_) {
+      GpuStreamTextureMsg_MatrixChanged_Params params;
+      memcpy(&params.m00, mtx, sizeof(mtx));
+      owner_stub_->channel()->Send(

[no sievers, 2016/02/01 18:30:47]

I meant: Send this message before GpuStreamTextureMsg_FrameAvailable above to
make sure it uses the correct matrix.

[jchen10, 2016/02/02 03:19:24]

So nice! :)

Done.

+          new GpuStreamTextureMsg_MatrixChanged(route_id_, params));
+    }
+  }
+  if (scoped_make_current.get()) {
+    // UpdateTexImage() implies glBindTexture().
+    // The cmd decoder takes care of restoring the binding for this GLImage as
+    // far as the current context is concerned, but if we temporarily change
+    // it, we have to keep the state intact in *that* context also.
+    const gpu::gles2::ContextState* state =
+        owner_stub_->decoder()->GetContextState();
+    const gpu::gles2::TextureUnit& active_unit =
+        state->texture_units[state->active_texture_unit];
+    glBindTexture(GL_TEXTURE_EXTERNAL_OES,
+                  active_unit.bound_texture_external_oes.get()
+                      ? active_unit.bound_texture_external_oes->service_id()
+                      : 0);
+  }
 }
 
 gfx::Size StreamTexture::GetSize() {
@@ -298,8 +284,6 @@ bool StreamTexture::CopyTexSubImage(unsigned target,
   glGetIntegerv(GL_TEXTURE_BINDING_2D, &target_texture);
   DCHECK(target_texture);
 
-  UpdateTexImage();
-
   if (!framebuffer_) {
     glGenFramebuffersEXT(1, &framebuffer_);