Referencing SurfaceManager::GetLatestInFlightSurface in surface activation...

Referencing SurfaceManager::GetLatestInFlightSurface in surface activation instead of fallback surface.

Use SurfaceManager::LatestInFlightSurface in Surface::ActivateFrame to use the latest in-flight
surface as the active reference instead of fallback. This allows for better garbage collection
of surfaces and optimize the notion of fallback surface.

Bug: 857575
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I4f597e431866b3a9886c04b982101199fd4011d7
Reviewed-on: https://chromium-review.googlesource.com/1149092
Commit-Queue: Andre Kaba <akaba@google.com>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578429}

components/viz/service/frame_sinks/compositor_frame_sink_support.cc

@@ -121,7 +121,6 @@ void CompositorFrameSinkSupport::OnSurfaceActivated(Surface* surface) {
   }
 
   DCHECK(surface->HasActiveFrame());
-  surface->UpdateSurfaceReferences();
 
   // Check if this is a display root surface and the SurfaceId is changing.
   if (is_root_ && (!referenced_local_surface_id_ ||

components/viz/service/frame_sinks/surface_synchronization_unittest.cc

@@ -2525,5 +2525,79 @@ TEST_F(SurfaceSynchronizationTest,
   EXPECT_EQ(parent_id, parent_support().last_activated_surface_id());
 }
 
+// This test verifies that once a Surface is activated, the latest in flight
+// surface for each SurfaceRange in the submitted CompositorFrame will be added
+// to the referenced surfaces.
+TEST_F(SurfaceSynchronizationTest, ReferencesAfterActivationSameFrameSink) {
+  SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+
+  SurfaceId child_id1 = MakeSurfaceId(kChildFrameSink1, 1);
+  SurfaceId child_id2 = MakeSurfaceId(kChildFrameSink1, 2);
+  SurfaceId child_id3 = MakeSurfaceId(kChildFrameSink1, 3);
+
+  DisableAssignTemporaryReferences();
+
+  child_support1().SubmitCompositorFrame(child_id1.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+
+  child_support1().SubmitCompositorFrame(child_id2.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+
+  parent_support().SubmitCompositorFrame(
+      parent_id.local_surface_id(),
+      MakeCompositorFrame(empty_surface_ids(),
+                          {SurfaceRange(child_id1, child_id3)},
+                          std::vector<TransferableResource>()));
+
+  const base::flat_set<SurfaceId>& child_references =
+      GetChildReferences(parent_id);
+
+  // |child_id2| is the latest active surface so we return it.
+  EXPECT_EQ(child_references.size(), 1u);
+  EXPECT_THAT(child_references, UnorderedElementsAre(child_id2));
+}
+
+// This test verifies that once a Surface is activated by a CompositorFrame
+// referencing a SurfaceRange with differing frame sinks, fallback surface will
+// be returned by GetLatestInFlightSurface.
+TEST_F(SurfaceSynchronizationTest,
+       ReferencesAfterActivationDifferentFrameSink) {
+  SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+
+  SurfaceId child_id1 = MakeSurfaceId(kChildFrameSink1, 1);
+  SurfaceId child_id2 = MakeSurfaceId(kChildFrameSink1, 2);
+  SurfaceId child_id3 = MakeSurfaceId(kChildFrameSink2, 3);
+
+  DisableAssignTemporaryReferences();
+
+  child_support1().SubmitCompositorFrame(child_id1.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+
+  child_support1().SubmitCompositorFrame(child_id2.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+
+  parent_support().SubmitCompositorFrame(
+      parent_id.local_surface_id(),
+      MakeCompositorFrame(empty_surface_ids(),
+                          {SurfaceRange(child_id1, child_id3)},
+                          std::vector<TransferableResource>()));
+
+  const base::flat_set<SurfaceId>& child_references =
+      GetChildReferences(parent_id);
+
+  // Since frame sink ids are different parent will reference fallback surface.
+  // However, after (crbug.com/857575) parent should reference |child_id2|
+  // instead.
+  EXPECT_THAT(child_references, UnorderedElementsAre(child_id1));
+
+  parent_support().SubmitCompositorFrame(
+      parent_id.local_surface_id(),
+      MakeCompositorFrame(empty_surface_ids(),
+                          {SurfaceRange(child_id2, child_id3)},
+                          std::vector<TransferableResource>()));
+
+  EXPECT_THAT(child_references, UnorderedElementsAre(child_id2));
+}
+
 }  // namespace test
 }  // namespace viz

components/viz/service/surfaces/surface.cc

@@ -329,12 +329,28 @@ void Surface::ActivateFrame(FrameData frame_data,
 
   active_frame_data_ = std::move(frame_data);
 
+  // Extract the latest in flight surface from the ranges in the frame then
+  // notify SurfaceManager of the new references.
   active_referenced_surfaces_.clear();
-  for (SurfaceRange surface_range :
+  for (const SurfaceRange& surface_range :
        active_frame_data_->frame.metadata.referenced_surfaces) {
-    if (surface_range.start())
-      active_referenced_surfaces_.emplace_back(*surface_range.start());
+    // TODO(akaba): remove this case when GetLatestInFlightSurface is able to
+    // return primary.
+    Surface* primary_surface =
+        surface_manager_->GetSurfaceForId(surface_range.end());
+    if (primary_surface && primary_surface->HasActiveFrame()) {
+      active_referenced_surfaces_.emplace_back(surface_range.end());
+      continue;
+    }
+
+    if (surface_range.start()) {
+      Surface* surface = surface_manager_->GetLatestInFlightSurface(
+          surface_range.end(), *surface_range.start());
+      if (surface)
+        active_referenced_surfaces_.emplace_back(surface->surface_id());
+    }
   }
+  UpdateSurfaceReferences();
 
   for (auto& copy_request : old_copy_requests)
     RequestCopyOfOutput(std::move(copy_request));