Fix surface reference assumptions in SurfaceManager.

cc/surfaces/surface_manager_ref_unittest.cc

Index: cc/surfaces/surface_manager_ref_unittest.cc
diff --git a/cc/surfaces/surface_manager_ref_unittest.cc b/cc/surfaces/surface_manager_ref_unittest.cc
index b1dc27a1071e5bb74516421d47700d578128c1b9..348ee8da0454ab227cfcf617402c90220075ece1 100644
--- a/cc/surfaces/surface_manager_ref_unittest.cc
+++ b/cc/surfaces/surface_manager_ref_unittest.cc
@@ -13,11 +13,12 @@
 #include "cc/surfaces/surface_factory_client.h"
 #include "cc/surfaces/surface_id.h"
 #include "cc/surfaces/surface_manager.h"
-#include "cc/surfaces/surface_sequence_generator.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using testing::ElementsAre;
+using testing::IsEmpty;
+using testing::SizeIs;
 using testing::UnorderedElementsAre;
 
 namespace cc {
@@ -27,8 +28,6 @@ namespace {
 constexpr FrameSinkId kFrameSink1(1, 0);
 constexpr FrameSinkId kFrameSink2(2, 0);
 constexpr FrameSinkId kFrameSink3(3, 0);
-const LocalFrameId kLocalFrame1(1, base::UnguessableToken::Create());
-const LocalFrameId kLocalFrame2(2, base::UnguessableToken::Create());
 
 class StubSurfaceFactoryClient : public SurfaceFactoryClient {
  public:
@@ -43,24 +42,17 @@ class SurfaceManagerRefTest : public testing::Test {
  public:
   SurfaceManager& manager() { return *manager_; }
 
-  // Creates a new Surface with the provided SurfaceId. Will first create the
-  // SurfaceFactory for |frame_sink_id| if necessary.
-  SurfaceId CreateSurface(const FrameSinkId& frame_sink_id,
-                          const LocalFrameId& local_frame_id) {
+  // Creates a new Surface with the provided |frame_sink_id| and |local_id|.
+  // Will first create a SurfaceFactory for |frame_sink_id| if necessary.
+  SurfaceId CreateSurface(const FrameSinkId& frame_sink_id, uint32_t local_id) {
+    LocalFrameId local_frame_id(local_id,
+                                base::UnguessableToken::Deserialize(0, 1u));
     GetFactory(frame_sink_id)
         .SubmitCompositorFrame(local_frame_id, CompositorFrame(),
                                SurfaceFactory::DrawCallback());
     return SurfaceId(frame_sink_id, local_frame_id);
   }
 
-  SurfaceId CreateSurface(uint32_t client_id,
-                          uint32_t sink_id,
-                          uint32_t local_id) {
-    return CreateSurface(
-        FrameSinkId(client_id, sink_id),
-        LocalFrameId(local_id, base::UnguessableToken::Deserialize(0, 1u)));
-  }
-
   // Destroy Surface with |surface_id|.
   void DestroySurface(const SurfaceId& surface_id) {
     GetFactory(surface_id.frame_sink_id()).EvictSurface();
@@ -88,12 +80,19 @@ class SurfaceManagerRefTest : public testing::Test {
     manager_.reset();
   }
 
-  // Returns all the references from the given surface id.
-  SurfaceManager::SurfaceIdSet GetReferencesFrom(const SurfaceId& surface_id) {
+  // Returns all the references where |surface_id| is the parent.
+  const SurfaceManager::SurfaceIdSet& GetReferencesFrom(
+      const SurfaceId& surface_id) {
     return manager().parent_to_child_refs_[surface_id];
   }
 
-  SurfaceManager::SurfaceIdSet GetReferencesFromRoot() {
+  // Returns all the references where |surface_id| is the child.
+  const SurfaceManager::SurfaceIdSet& GetReferencesFor(
+      const SurfaceId& surface_id) {
+    return manager().child_to_parent_refs_[surface_id];
+  }
+
+  const SurfaceManager::SurfaceIdSet& GetReferencesFromRoot() {
     return GetReferencesFrom(manager().GetRootSurfaceId());
   }
 
@@ -123,61 +122,37 @@ class SurfaceManagerRefTest : public testing::Test {
 };
 
 TEST_F(SurfaceManagerRefTest, AddReference) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
   manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
 
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id1), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 0u);
+  EXPECT_THAT(GetReferencesFor(id1),
+              UnorderedElementsAre(manager().GetRootSurfaceId()));
+  EXPECT_THAT(GetReferencesFrom(id1), IsEmpty());
 }
 
 TEST_F(SurfaceManagerRefTest, AddRemoveReference) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
-  SurfaceId id2 = CreateSurface(kFrameSink2, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
+  SurfaceId id2 = CreateSurface(kFrameSink2, 1);
   manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
   manager().AddSurfaceReference(id1, id2);
 
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id1), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id2), 0u);
-
-  manager().RemoveSurfaceReference(id1, id2);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id1), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 0u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 0u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id2), 0u);
-}
-
-TEST_F(SurfaceManagerRefTest, AddRemoveReferenceRecursive) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
-  SurfaceId id2 = CreateSurface(kFrameSink2, kLocalFrame1);
-  SurfaceId id3 = CreateSurface(kFrameSink3, kLocalFrame1);
+  EXPECT_THAT(GetReferencesFor(id1),
+              UnorderedElementsAre(manager().GetRootSurfaceId()));
+  EXPECT_THAT(GetReferencesFor(id2), UnorderedElementsAre(id1));
+  EXPECT_THAT(GetReferencesFrom(id1), UnorderedElementsAre(id2));
+  EXPECT_THAT(GetReferencesFrom(id2), IsEmpty());
 
-  manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
-  manager().AddSurfaceReference(id1, id2);
-  manager().AddSurfaceReference(id2, id3);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id1), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id3), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id2), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id3), 0u);
-
-  // Should remove reference from id1 -> id2 and then since id2 has zero
-  // references all references it holds should be removed.
   manager().RemoveSurfaceReference(id1, id2);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id1), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 0u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id3), 0u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 0u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id2), 0u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id3), 0u);
+  EXPECT_THAT(GetReferencesFor(id1), SizeIs(1));
+  EXPECT_THAT(GetReferencesFor(id2), IsEmpty());
+  EXPECT_THAT(GetReferencesFrom(id1), IsEmpty());
+  EXPECT_THAT(GetReferencesFrom(id2), IsEmpty());
 }
 
 TEST_F(SurfaceManagerRefTest, NewSurfaceFromFrameSink) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
-  SurfaceId id2 = CreateSurface(kFrameSink2, kLocalFrame1);
-  SurfaceId id3 = CreateSurface(kFrameSink3, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
+  SurfaceId id2 = CreateSurface(kFrameSink2, 1);
+  SurfaceId id3 = CreateSurface(kFrameSink3, 1);
 
   manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
   manager().AddSurfaceReference(id1, id2);
@@ -185,59 +160,83 @@ TEST_F(SurfaceManagerRefTest, NewSurfaceFromFrameSink) {
 
   // |kFramesink2| received a CompositorFrame with a new size, so it destroys
   // |id2| and creates |id2_next|. No reference have been removed yet.
-  DestroySurface(id2);
-  SurfaceId id2_next = CreateSurface(kFrameSink2, kLocalFrame2);
-  EXPECT_NE(manager().GetSurfaceForId(id2), nullptr);
-  EXPECT_NE(manager().GetSurfaceForId(id2_next), nullptr);
+  SurfaceId id2_next = CreateSurface(kFrameSink2, 2);
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id2));
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id2_next));
 
   // Add references to and from |id2_next|.
   manager().AddSurfaceReference(id1, id2_next);
   manager().AddSurfaceReference(id2_next, id3);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2_next), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id3), 2u);
+  EXPECT_THAT(GetReferencesFor(id2), UnorderedElementsAre(id1));
+  EXPECT_THAT(GetReferencesFor(id2_next), UnorderedElementsAre(id1));
+  EXPECT_THAT(GetReferencesFor(id3), UnorderedElementsAre(id2, id2_next));
 
   manager().RemoveSurfaceReference(id1, id2);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 0u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2_next), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id3), 1u);
+  EXPECT_THAT(GetReferencesFor(id2), IsEmpty());
+  EXPECT_THAT(GetReferencesFor(id2_next), UnorderedElementsAre(id1));
+  EXPECT_THAT(GetReferencesFor(id3), UnorderedElementsAre(id2_next));
 
   // |id2| should be deleted during GC but other surfaces shouldn't.
-  EXPECT_EQ(manager().GetSurfaceForId(id2), nullptr);
-  EXPECT_NE(manager().GetSurfaceForId(id2_next), nullptr);
-  EXPECT_NE(manager().GetSurfaceForId(id3), nullptr);
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id2));
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id2_next));
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id3));
+}
+
+TEST_F(SurfaceManagerRefTest, ReferenceCycleGetsDeleted) {
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
+  SurfaceId id2 = CreateSurface(kFrameSink2, 1);
+  SurfaceId id3 = CreateSurface(kFrameSink3, 1);
+
+  manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
+  manager().AddSurfaceReference(id1, id2);
+  manager().AddSurfaceReference(id2, id3);
+
+  // This reference forms a cycle between id2 and id3.
+  manager().AddSurfaceReference(id3, id2);
+
+  DestroySurface(id3);
+  DestroySurface(id2);
+  DestroySurface(id1);
+
+  manager().RemoveSurfaceReference(manager().GetRootSurfaceId(), id1);
+
+  // Removing the reference from the root to id1 should allow all three surfaces
+  // to be deleted during GC even with a cycle between 2 and 3.
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id1));
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id2));
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id3));
 }
 
 TEST_F(SurfaceManagerRefTest, CheckGC) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
-  SurfaceId id2 = CreateSurface(kFrameSink2, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
+  SurfaceId id2 = CreateSurface(kFrameSink2, 1);
 
   manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
   manager().AddSurfaceReference(id1, id2);
 
-  EXPECT_NE(manager().GetSurfaceForId(id1), nullptr);
-  EXPECT_NE(manager().GetSurfaceForId(id2), nullptr);
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id1));
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id2));
 
   // Destroying the surfaces shouldn't delete them yet, since there is still an
   // active reference on all surfaces.
   DestroySurface(id1);
   DestroySurface(id2);
-  EXPECT_NE(manager().GetSurfaceForId(id1), nullptr);
-  EXPECT_NE(manager().GetSurfaceForId(id2), nullptr);
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id1));
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id2));
 
   // Should delete |id2| when the only reference to it is removed.
   manager().RemoveSurfaceReference(id1, id2);
-  EXPECT_EQ(manager().GetSurfaceForId(id2), nullptr);
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id2));
 
   // Should delete |id1| when the only reference to it is removed.
   manager().RemoveSurfaceReference(manager().GetRootSurfaceId(), id1);
-  EXPECT_EQ(manager().GetSurfaceForId(id1), nullptr);
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id1));
 }
 
 TEST_F(SurfaceManagerRefTest, CheckGCRecusiveFull) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
-  SurfaceId id2 = CreateSurface(kFrameSink2, kLocalFrame1);
-  SurfaceId id3 = CreateSurface(kFrameSink3, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
+  SurfaceId id2 = CreateSurface(kFrameSink2, 1);
+  SurfaceId id3 = CreateSurface(kFrameSink3, 1);
 
   manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
   manager().AddSurfaceReference(id1, id2);
@@ -249,17 +248,17 @@ TEST_F(SurfaceManagerRefTest, CheckGCRecusiveFull) {
 
   // Destroying the surfaces shouldn't delete them yet, since there is still an
   // active reference on all surfaces.
-  EXPECT_NE(manager().GetSurfaceForId(id3), nullptr);
-  EXPECT_NE(manager().GetSurfaceForId(id2), nullptr);
-  EXPECT_NE(manager().GetSurfaceForId(id1), nullptr);
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id3));
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id2));
+  EXPECT_NE(nullptr, manager().GetSurfaceForId(id1));
 
   manager().RemoveSurfaceReference(manager().GetRootSurfaceId(), id1);
 
   // Removing the reference from the root to id1 should allow all three surfaces
   // to be deleted during GC.
-  EXPECT_EQ(manager().GetSurfaceForId(id1), nullptr);
-  EXPECT_EQ(manager().GetSurfaceForId(id2), nullptr);
-  EXPECT_EQ(manager().GetSurfaceForId(id3), nullptr);
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id1));
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id2));
+  EXPECT_EQ(nullptr, manager().GetSurfaceForId(id3));
 }
 
 TEST_F(SurfaceManagerRefTest, TryAddReferenceToBadSurface) {
@@ -270,61 +269,61 @@ TEST_F(SurfaceManagerRefTest, TryAddReferenceToBadSurface) {
   // Adding reference from root to the Surface should do nothing because
   // SurfaceManager doesn't know Surface for |id| exists.
   manager().AddSurfaceReference(manager().GetRootSurfaceId(), id);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id), 0u);
+  EXPECT_THAT(GetReferencesFor(id), IsEmpty());
 }
 
 TEST_F(SurfaceManagerRefTest, TryDoubleAddReference) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
-  SurfaceId id2 = CreateSurface(kFrameSink2, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
+  SurfaceId id2 = CreateSurface(kFrameSink2, 1);
 
   manager().AddSurfaceReference(manager().GetRootSurfaceId(), id1);
   manager().AddSurfaceReference(id1, id2);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 1u);
+  EXPECT_THAT(GetReferencesFor(id2), SizeIs(1));
+  EXPECT_THAT(GetReferencesFrom(id1), SizeIs(1));
 
   // The second request should be ignored without crashing.
   manager().AddSurfaceReference(id1, id2);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 1u);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 1u);
+  EXPECT_THAT(GetReferencesFor(id2), SizeIs(1));
+  EXPECT_THAT(GetReferencesFrom(id1), SizeIs(1));
 }
 
 TEST_F(SurfaceManagerRefTest, TryAddSelfReference) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
 
   // A temporary reference must exist to |id1|.
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id1), 1u);
+  EXPECT_THAT(GetReferencesFor(id1), SizeIs(1));
 
   // Try to add a self reference. This should fail.
   manager().AddSurfaceReference(id1, id1);
 
   // Adding a self reference should be ignored without crashing.
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 0u);
+  EXPECT_THAT(GetReferencesFrom(id1), IsEmpty());
 
   // The temporary reference to |id1| must still exist.
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id1), 1u);
+  EXPECT_THAT(GetReferencesFor(id1), SizeIs(1));
 }
 
 TEST_F(SurfaceManagerRefTest, TryRemoveBadReference) {
-  SurfaceId id1 = CreateSurface(kFrameSink1, kLocalFrame1);
-  SurfaceId id2 = CreateSurface(kFrameSink2, kLocalFrame1);
+  SurfaceId id1 = CreateSurface(kFrameSink1, 1);
+  SurfaceId id2 = CreateSurface(kFrameSink2, 1);
 
   // Removing non-existent reference should be ignored.
   manager().AddSurfaceReference(id1, id2);
   manager().RemoveSurfaceReference(id2, id1);
-  EXPECT_EQ(manager().GetReferencedSurfaceCount(id1), 1u);
-  EXPECT_EQ(manager().GetSurfaceReferenceCount(id2), 1u);
+  EXPECT_THAT(GetReferencesFrom(id1), SizeIs(1));
+  EXPECT_THAT(GetReferencesFor(id2), SizeIs(1));
 }
 
 TEST_F(SurfaceManagerRefTest, AddSurfaceThenReference) {
   // Create a new surface.
-  const SurfaceId surface_id = CreateSurface(2, 1, 1);
+  const SurfaceId surface_id = CreateSurface(kFrameSink2, 1);
 
   // A temporary reference must be added to |surface_id|.
   EXPECT_THAT(GetAllTempReferences(), ElementsAre(surface_id));
   EXPECT_THAT(GetReferencesFromRoot(), ElementsAre(surface_id));
 
   // Create |parent_id| and add a real reference from it to |surface_id|.
-  const SurfaceId parent_id = CreateSurface(1, 1, 1);
+  const SurfaceId parent_id = CreateSurface(kFrameSink1, 1);
   manager().AddSurfaceReference(parent_id, surface_id);
 
   // The temporary reference to |surface_id| should be gone.
@@ -337,7 +336,7 @@ TEST_F(SurfaceManagerRefTest, AddSurfaceThenReference) {
 
 TEST_F(SurfaceManagerRefTest, AddSurfaceThenRootReference) {
   // Create a new surface.
-  const SurfaceId surface_id = CreateSurface(1, 1, 1);
+  const SurfaceId surface_id = CreateSurface(kFrameSink1, 1);
 
   // Temporary reference should be added to |surface_id|.
   EXPECT_THAT(GetAllTempReferences(), ElementsAre(surface_id));
@@ -354,8 +353,8 @@ TEST_F(SurfaceManagerRefTest, AddSurfaceThenRootReference) {
 
 TEST_F(SurfaceManagerRefTest, AddTwoSurfacesThenOneReference) {
   // Create two surfaces with different FrameSinkIds.
-  const SurfaceId surface_id1 = CreateSurface(2, 1, 1);
-  const SurfaceId surface_id2 = CreateSurface(3, 1, 1);
+  const SurfaceId surface_id1 = CreateSurface(kFrameSink2, 1);
+  const SurfaceId surface_id2 = CreateSurface(kFrameSink3, 1);
 
   // Temporary reference should be added for both surfaces.
   EXPECT_THAT(GetAllTempReferences(),
@@ -364,7 +363,7 @@ TEST_F(SurfaceManagerRefTest, AddTwoSurfacesThenOneReference) {
               UnorderedElementsAre(surface_id1, surface_id2));
 
   // Create |parent_id| and add a real reference from it to |surface_id1|.
-  const SurfaceId parent_id = CreateSurface(1, 1, 1);
+  const SurfaceId parent_id = CreateSurface(kFrameSink1, 1);
   manager().AddSurfaceReference(parent_id, surface_id1);
 
   // Real reference must be added to |surface_id1| and the temporary reference
@@ -382,8 +381,8 @@ TEST_F(SurfaceManagerRefTest, AddSurfacesSkipReference) {
   // Add two surfaces that have the same FrameSinkId. This would happen when a
   // client submits two CompositorFrames before parent submits a new
   // CompositorFrame.
-  const SurfaceId surface_id1 = CreateSurface(2, 1, 2);
-  const SurfaceId surface_id2 = CreateSurface(2, 1, 1);
+  const SurfaceId surface_id1 = CreateSurface(kFrameSink2, 2);
+  const SurfaceId surface_id2 = CreateSurface(kFrameSink2, 1);
 
   // Temporary references should be added for both surfaces and they should be
   // stored in the order of creation.
@@ -395,7 +394,7 @@ TEST_F(SurfaceManagerRefTest, AddSurfacesSkipReference) {
 
   // Create |parent_id| and add a reference from it to |surface_id2| which was
   // created later.
-  const SurfaceId parent_id = CreateSurface(1, 1, 1);
+  const SurfaceId parent_id = CreateSurface(kFrameSink1, 1);
   manager().AddSurfaceReference(parent_id, surface_id2);
 
   // The real reference should be added for |surface_id2| and the temporary
@@ -409,8 +408,8 @@ TEST_F(SurfaceManagerRefTest, AddSurfacesSkipReference) {
 TEST_F(SurfaceManagerRefTest, RemoveFirstTempRefOnly) {
   // Add two surfaces that have the same FrameSinkId. This would happen when a
   // client submits two CFs before parent submits a new CF.
-  const SurfaceId surface_id1 = CreateSurface(2, 1, 1);
-  const SurfaceId surface_id2 = CreateSurface(2, 1, 2);
+  const SurfaceId surface_id1 = CreateSurface(kFrameSink2, 1);
+  const SurfaceId surface_id2 = CreateSurface(kFrameSink2, 2);
 
   // Temporary references should be added for both surfaces and they should be
   // stored in the order of creation.
@@ -422,7 +421,7 @@ TEST_F(SurfaceManagerRefTest, RemoveFirstTempRefOnly) {
 
   // Create |parent_id| and add a reference from it to |surface_id1| which was
   // created earlier.
-  const SurfaceId parent_id = CreateSurface(1, 1, 1);
+  const SurfaceId parent_id = CreateSurface(kFrameSink1, 1);
   manager().AddSurfaceReference(parent_id, surface_id1);
 
   // The real reference should be added for |surface_id1| and its temporary