Fix surface sync crash on CrOs

If a client embeds two different SurfaceIds with the same embed token,
we will end up in a bad state. Ignore duplicate embed tokens in
activation dependencies. I will investigate why the clients are
behaving this way but regardless of the reason viz should be resilient
and not crash.

Bug: 1001143,962367
Change-Id: I19dddbe60c2d87ce6b94a4fee4b584efafa70876
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1791797
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/master@{#694920}

components/viz/service/frame_sinks/surface_synchronization_unittest.cc

@@ -3073,4 +3073,42 @@ TEST_F(SurfaceSynchronizationTest,
   EXPECT_EQ(parent_id2, parent_support().last_activated_surface_id());
 }
 
+// Check that if two different SurfaceIds with the same embed token are
+// embedded, viz doesn't crash. https://crbug.com/1001143
+TEST_F(SurfaceSynchronizationTest,
+       DuplicateAllocationGroupInActivationDependencies) {
+  const SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+  const SurfaceId child1_id1 = MakeSurfaceId(kChildFrameSink1, 1);
+  const SurfaceId child1_id2 = MakeSurfaceId(kChildFrameSink1, 2);
+  const SurfaceId child2_id1 = MakeSurfaceId(kChildFrameSink2, 1);
+
+  // Submit a CompositorFrame to |child1_id1| embedding |child2_id1|.
+  CompositorFrame child1_frame =
+      CompositorFrameBuilder()
+          .AddDefaultRenderPass()
+          .SetActivationDependencies({child2_id1})
+          .SetReferencedSurfaces({SurfaceRange(base::nullopt, child2_id1)})
+          .Build();
+  child_support1().SubmitCompositorFrame(child1_id1.local_surface_id(),
+                                         std::move(child1_frame));
+
+  // Submit a CompositorFrame to |parent_id| embedding both |child1_id1| and
+  // |child1_id2|.
+  CompositorFrame parent_frame =
+      CompositorFrameBuilder()
+          .AddDefaultRenderPass()
+          .SetActivationDependencies({child1_id1, child1_id2})
+          .SetReferencedSurfaces({SurfaceRange(base::nullopt, child1_id1),
+                                  SurfaceRange(base::nullopt, child1_id2)})
+          .Build();
+  // This shouldn't crash.
+  parent_support().SubmitCompositorFrame(parent_id.local_surface_id(),
+                                         std::move(parent_frame));
+
+  // When multiple dependencies have the same embed token, only the first one
+  // should be taken into account.
+  EXPECT_EQ(1u, parent_surface()->activation_dependencies().size());
+  EXPECT_EQ(child1_id1, *parent_surface()->activation_dependencies().begin());
+}
+
 }  // namespace viz

components/viz/service/surfaces/surface.cc

@@ -493,12 +493,14 @@ void Surface::UpdateActivationDependencies(
   if (current_frame.metadata.deadline.IsZero())
     return;
 
-  std::vector<SurfaceAllocationGroup*> new_blocking_allocation_groups;
+  base::flat_set<SurfaceAllocationGroup*> new_blocking_allocation_groups;
   std::vector<SurfaceId> new_activation_dependencies;
   for (const SurfaceId& surface_id :
        current_frame.metadata.activation_dependencies) {
     SurfaceAllocationGroup* group =
         surface_manager_->GetOrCreateAllocationGroupForSurfaceId(surface_id);
+    if (base::Contains(new_blocking_allocation_groups, group))
+      continue;
     if (group)
       group->UpdateLastPendingReferenceAndMaybeActivate(surface_id);
     Surface* dependency = surface_manager_->GetSurfaceForId(surface_id);
@@ -511,7 +513,7 @@ void Surface::UpdateActivationDependencies(
     }
     if (group) {
       group->RegisterBlockedEmbedder(this, surface_id);
-      new_blocking_allocation_groups.push_back(group);
+      new_blocking_allocation_groups.insert(group);
     }
     TRACE_EVENT_WITH_FLOW2(
         TRACE_DISABLED_BY_DEFAULT("viz.surface_id_flow"),