Retain references to surfaces from both active AND pending CompositorFrames

cc/surfaces/compositor_frame_sink_support.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/surfaces/compositor_frame_sink_support.h"
 
 #include <algorithm>
 #include <utility>
 #include <vector>
 
@@ skipping 58 matching lines @@
   needs_begin_frame_ = needs_begin_frame;
   UpdateNeedsBeginFramesInternal();
 }
 
 void CompositorFrameSinkSupport::SubmitCompositorFrame(
     const LocalSurfaceId& local_surface_id,
     CompositorFrame frame) {
   ++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);
+    // 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();
 
-    surface_factory_.SubmitCompositorFrame(
-        local_surface_id, std::move(frame),
-        base::Bind(&CompositorFrameSinkSupport::DidReceiveCompositorFrameAck,
-                   weak_factory_.GetWeakPtr()));
+    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_)
     display_->SetLocalSurfaceId(local_surface_id, device_scale_factor);
 }
 
 void CompositorFrameSinkSupport::Require(const LocalSurfaceId& local_surface_id,
                                          const SurfaceSequence& sequence) {
   surface_manager_->RequireSequence(SurfaceId(frame_sink_id_, local_surface_id),
                                     sequence);
@@ skipping 139 matching lines @@
     return;
 
   added_frame_observer_ = needs_begin_frame_;
   if (needs_begin_frame_)
     begin_frame_source_->AddObserver(this);
   else
     begin_frame_source_->RemoveObserver(this);
 }
 
 }  // namespace cc