Aura: Resize locks with --ui-enable-threaded-compositing

content/common/gpu/texture_image_transport_surface.cc

Index: content/common/gpu/texture_image_transport_surface.cc
diff --git a/content/common/gpu/texture_image_transport_surface.cc b/content/common/gpu/texture_image_transport_surface.cc
index 7078a3e972b41d247b976f96347942b6f2becab5..eb9aac33c6bd55f33bb9e4040f5d81616f5d5aa8 100644
--- a/content/common/gpu/texture_image_transport_surface.cc
+++ b/content/common/gpu/texture_image_transport_surface.cc
@@ -44,7 +44,8 @@ TextureImageTransportSurface::TextureImageTransportSurface(
         handle_(handle),
         parent_stub_(NULL),
         is_swap_buffers_pending_(false),
-        did_unschedule_(false) {
+        did_unschedule_(false),
+        did_flip_(false) {
   helper_.reset(new ImageTransportHelper(this,
                                          manager,
                                          stub,
@@ -321,7 +322,7 @@ bool TextureImageTransportSurface::PostSubBuffer(
             region_to_copy.width(), region_to_copy.height());
       }
     }
-  } else {
+  } else if (!surfaces_same_size && did_flip_) {
     DCHECK(new_damage_rect == gfx::Rect(expected_size));
   }
 
@@ -389,21 +390,35 @@ void TextureImageTransportSurface::OnSetFrontSurfaceIsProtected(
   }
 }
 
-void TextureImageTransportSurface::OnBufferPresented(uint32 sync_point) {
+void TextureImageTransportSurface::OnBufferPresented(bool presented,
+                                                     uint32 sync_point) {
   if (sync_point == 0) {
-    BufferPresentedImpl();
+    BufferPresentedImpl(presented);
   } else {
     helper_->manager()->sync_point_manager()->AddSyncPointCallback(
         sync_point,
         base::Bind(&TextureImageTransportSurface::BufferPresentedImpl,
-                   this->AsWeakPtr()));
+                   this->AsWeakPtr(),
+                   presented));
   }
 }
 
-void TextureImageTransportSurface::BufferPresentedImpl() {
+void TextureImageTransportSurface::BufferPresentedImpl(bool presented) {
   DCHECK(is_swap_buffers_pending_);
   is_swap_buffers_pending_ = false;
 
+  if (presented) {
+    // If we had not flipped, the two frame damage tracking is inconsistent.
+    // So conservatively take the whole frame.
+    if (!did_flip_)
+      previous_damage_rect_ = gfx::Rect(textures_[front()].size);
+  } else {
+    front_ = back();
+    previous_damage_rect_ = gfx::Rect(0, 0, 0, 0);
+  }

[piman, 2012/10/17 21:26:32]

Just a suggestion: would it help to move the 2 lines from PostSubBuffer:

front_ = back();
previous_damage_rect_ = new_damage_rect;

(and corresponding from SwapBuffers) onto here?

It kinda feels we're trying to "undo" whatever was done there in some cases.

[jonathan.backer, 2012/10/18 20:20:35]

I was motivated by a comment you made on an earlier version of this CL where I
did the flip here: "Mmh, is it ok to move this [the flip] here wrt e.g. dropping
front surface? If AdjustFrontBufferAllocation gets called between the swap and
the swap ack, then it'll change the allocation of the old-front-now-back buffer
instead of the old-back-now-front buffer."

I'm not sure how big a concern this is. I don't think that the front buffer
allocation can change like this (mid swap), but I really don't know.

I don't think that deferring the flip until we know that it goes through would
help substantially with the previous_damage_rect_ logic here. When we fast ACK,
we don't need to copy on the next post sub buffer. When we go from fast ACKing
to flipping, our two frame damage tracking is screwed up. We could do better by
accumulating damage with a union, but this is such an uncommon case that I'd
rather go suboptimal and obviously correct.

+
+  did_flip_ = presented;
+
   // We're relying on the fact that the parent context is
   // finished with it's context when it inserts the sync point that
   // triggers this callback.