cc: Reject CompositorFrames to old child surfaces

cc/surfaces/surface.cc

Index: cc/surfaces/surface.cc
diff --git a/cc/surfaces/surface.cc b/cc/surfaces/surface.cc
index 2ceae59329081c2295c5aadac3d04fb171d0742e..68c820d8b054989ee55dd287e943a277e94e43df 100644
--- a/cc/surfaces/surface.cc
+++ b/cc/surfaces/surface.cc
@@ -15,7 +15,9 @@
 #include "cc/surfaces/local_surface_id_allocator.h"
 #include "cc/surfaces/pending_frame_observer.h"
 #include "cc/surfaces/surface_factory.h"
+#include "cc/surfaces/surface_factory_client.h"
 #include "cc/surfaces/surface_manager.h"
+#include "cc/surfaces/surface_resource_holder_client.h"
 
 namespace cc {
 
@@ -50,9 +52,23 @@ void Surface::SetPreviousFrameSurface(Surface* surface) {
   surface->TakeLatencyInfoFromPendingFrame(&frame.metadata.latency_info);
 }
 
+void Surface::Close() {
+  closed_ = true;
+}
+
 void Surface::QueueFrame(CompositorFrame frame,
                          const DrawCallback& callback,
                          const WillDrawCallback& will_draw_callback) {
+  if (closed_) {
+    if (factory_ && factory_->resource_holder_client()) {
+      ReturnedResourceArray resources;
+      TransferableResource::ReturnResources(frame.resource_list, &resources);
+      factory_->resource_holder_client()->ReturnResources(resources);
+    }
+    callback.Run();
+    return;
+  }
+
   TakeLatencyInfoFromPendingFrame(&frame.metadata.latency_info);
 
   base::Optional<FrameData> previous_pending_frame_data =
@@ -68,6 +84,23 @@ void Surface::QueueFrame(CompositorFrame frame,
   bool is_pending_frame = !blocking_surfaces_.empty();
 
   if (is_pending_frame) {
+    // Reject CompositorFrames to surfaces referenced from CompositorFrame while

[danakj, 2017/05/04 21:39:13]

CompositorFrame is a type, you're using it in this sentence like it's an object,
and I can't tell if there is two of them or what. I suggest giving the things ur
talking about in here names and refer to them that way, like algebra.

[Fady Samuel, 2017/05/04 22:44:04]

I'm not sure what you mean. I do agree this is confusing, because the comment is
a bit stale. I've updated the comment. Hopefully it's clearer now.

+    // it is blocked. This saves some CPU cycles to allow children to catch up
+    // to this surface's client.
+    base::flat_set<FrameSinkId> embedded_frame_sink_ids;
+    for (const SurfaceId& surface_id : frame.metadata.embedded_surfaces)
+      embedded_frame_sink_ids.insert(surface_id.frame_sink_id());
+    for (const SurfaceId& surface_id : frame.metadata.referenced_surfaces) {
+      // If this surface is a fallback surface, then we close the fallback to
+      // accepting new frames.
+      bool is_fallback_surface =
+          embedded_frame_sink_ids.count(surface_id.frame_sink_id()) > 0;

[danakj, 2017/05/04 21:39:13]

Didnt ur other CL just make it not possible to be in both lists?

[Fady Samuel, 2017/05/04 22:44:04]

I'm comparing FrameSinkIds not SurfaceIds.

Basically, the primary and fallback SurfaceInfo should have the same
FrameSinkId. If we find a surface ID with a FrameSinkId from
referenced_surfaces, then we know the referenced_surfaces SurfaceId is a
fallback.

+      if (is_fallback_surface) {
+        Surface* surface = factory_->manager()->GetSurfaceForId(surface_id);
+        DCHECK(surface);
+        surface->Close();
+      }
+    }
     pending_frame_data_ =
         FrameData(std::move(frame), callback, will_draw_callback);
     // Ask the surface manager to inform |this| when its dependencies are