Retain references to surfaces from both active AND pending CompositorFrames

cc/surfaces/compositor_frame_sink_support.cc

Index: cc/surfaces/compositor_frame_sink_support.cc
diff --git a/cc/surfaces/compositor_frame_sink_support.cc b/cc/surfaces/compositor_frame_sink_support.cc
index e7848f8021f16e3d3cadc78723a0076d505d2e9f..74fd6a073994a5e3d681713df8f9de2678bb6dee 100644
--- a/cc/surfaces/compositor_frame_sink_support.cc
+++ b/cc/surfaces/compositor_frame_sink_support.cc
@@ -76,25 +76,28 @@ void CompositorFrameSinkSupport::SubmitCompositorFrame(
   ++ack_pending_count_;
   float device_scale_factor = frame.metadata.device_scale_factor;
 
+  surface_factory_.SubmitCompositorFrame(
+      local_surface_id, std::move(frame),
+      base::Bind(&CompositorFrameSinkSupport::DidReceiveCompositorFrameAck,
+                 weak_factory_.GetWeakPtr()));
+
   if (surface_manager_->using_surface_references()) {
     SurfaceId last_surface_id = reference_tracker_.current_surface_id();
 
     // Populate list of surface references to add and remove based on reference
-    // surfaces in current frame compared with the last frame.
-    reference_tracker_.UpdateReferences(local_surface_id,
-                                        frame.metadata.referenced_surfaces);
-
-    surface_factory_.SubmitCompositorFrame(
-        local_surface_id, std::move(frame),
-        base::Bind(&CompositorFrameSinkSupport::DidReceiveCompositorFrameAck,
-                   weak_factory_.GetWeakPtr()));
+    // surfaces in current frame compared with the last frame. The list of
+    // surface references includes references from both the pending and active
+    // frame if any.
+    SurfaceId current_surface_id(frame_sink_id_, local_surface_id);
+    Surface* surface = surface_manager_->GetSurfaceForId(current_surface_id);
+    std::vector<SurfaceId> referenced_surfaces = surface->referenced_surfaces();
+
+    referenced_surfaces.insert(referenced_surfaces.end(),

[vmpstr, 2017/02/10 00:24:40]

This is, somewhat poor from an API perspective.

if pending_referenced_surfaces are indeed referenced surfaces of the surface,
then ::referenced_surfaces() should include it in its vector. If you want to
retain this distinction, then I think referenced_surfaces need to be renamed to
active_referenced_surfaces.

Line 93 also incurs a copy of a vector, which ideally is something we should try
and avoid. Is it reasonable for UpdateReferences to take two vectors?

[Fady Samuel, 2017/02/10 00:55:30]

I have renamed referenced_surfaces => active_referenced_surfaces and added a
TODO to avoid this copy.

+                               surface->pending_referenced_surfaces().begin(),
+                               surface->pending_referenced_surfaces().end());
+    reference_tracker_.UpdateReferences(local_surface_id, referenced_surfaces);
 
     UpdateSurfaceReferences(last_surface_id, local_surface_id);
-  } else {
-    surface_factory_.SubmitCompositorFrame(
-        local_surface_id, std::move(frame),
-        base::Bind(&CompositorFrameSinkSupport::DidReceiveCompositorFrameAck,
-                   weak_factory_.GetWeakPtr()));
   }
 
   if (display_)