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 6dc261bde664d9549a95c85dfc905caffe12ec65..d79b90610e00c83cd443df95eb38e15b0eb2af14 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,36 @@ void Surface::QueueFrame(CompositorFrame frame,
   bool is_pending_frame = !blocking_surfaces_.empty();
 
   if (is_pending_frame) {
+    // Reject CompositorFrames submitted to surfaces referenced from this
+    // CompositorFrame as fallbacks. This saves some CPU cycles to allow
+    // children to catch up to the parent.
+    // Note: |activation_dependencies| and |referenced_surfaces| are disjointed

[danakj, 2017/05/05 19:51:35]

This part of the comment seems like it belongs in CompositorFrameMetadata, which
is already there so not sure it is needed, unless you wanna buff up the comments
there.

[Fady Samuel, 2017/05/05 20:25:02]

Done. Moved this to CompositorFrameMetadata.

+    //       sets of surface IDs. If a surface ID is known to exist and can be
+    //       used without additional synchronization, then it is placed in
+    //       |referenced_surfaces|. |activation_dependencies| are the set of
+    //       surface IDs that this |frame| would like to block on until they
+    //       become available or a deadline hits.
+    //
+    //       A surface ID in |referenced_surfaces| that has a corresponding

[danakj, 2017/05/05 19:51:35]

Thanks, this is what I was looking for. Can you move this to L106 and replace
the comment there with this? The comment there right now can go away, it only
says what the code is doing, and repeats what is said on L88.

[Fady Samuel, 2017/05/05 20:25:02]

Done.

+    //       surface ID in |activation_dependencies| with the same frame sink ID
+    //       is said to be a fallback surface that can be used in place of the
+    //       primary surface if the deadline passes before the dependency
+    //       becomes available.
+    base::flat_set<FrameSinkId> frame_sink_ids_for_dependencies;
+    for (const SurfaceId& surface_id : frame.metadata.activation_dependencies)
+      frame_sink_ids_for_dependencies.insert(surface_id.frame_sink_id());
+    for (const SurfaceId& surface_id : frame.metadata.referenced_surfaces) {
+      // If this |surface_id| corresponds to a fallback surface referenced by
+      // this |frame|, then we close the corresponding surface to avoid
+      // accepting new frames.
+      bool is_fallback_surface =
+          frame_sink_ids_for_dependencies.count(surface_id.frame_sink_id()) > 0;
+      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