Use CompositorFrameSinkSupport in tests.

This CL changes FrameSinkManagerTest to use CompositorFrameSinkSupport
instead of FrameSinkManagerClient. This will allow for
FrameSinkManagerClient to be deleted in a follow up CL and
FrameSinkManagerImpls internal data structures to be cleaned up.

Also rename SurfaceManagerOrderingParamTest to
FrameSinkManagerOrderingParamTest. This wasn't updated previously.

Bug: 792192
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I0ad3f1e8f8530dccf001fe81c6c9191176dbaddf
Reviewed-on: https://chromium-review.googlesource.com/809614Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521912}

components/viz/service/frame_sinks/compositor_frame_sink_support.h

@@ -112,6 +112,8 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
   Surface* GetCurrentSurfaceForTesting();
 
  private:
+  friend class FrameSinkManagerTest;
+
   CompositorFrameSinkSupport(mojom::CompositorFrameSinkClient* client,
                              const FrameSinkId& frame_sink_id,
                              bool is_root,

components/viz/service/frame_sinks/frame_sink_manager_unittest.cc

@@ -5,7 +5,7 @@
 #include <stddef.h>
 
 #include "components/viz/common/frame_sinks/begin_frame_source.h"
-#include "components/viz/service/frame_sinks/frame_sink_manager_client.h"
+#include "components/viz/service/frame_sinks/compositor_frame_sink_support.h"
 #include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
 #include "components/viz/test/begin_frame_source_test.h"
 #include "components/viz/test/fake_external_begin_frame_source.h"
@@ -16,116 +16,76 @@ namespace {
 
 constexpr FrameSinkId kArbitraryFrameSinkId(1, 1);
 
-class FakeFrameSinkManagerClient : public FrameSinkManagerClient {
- public:
-  explicit FakeFrameSinkManagerClient(const FrameSinkId& frame_sink_id)
-      : source_(nullptr), manager_(nullptr), frame_sink_id_(frame_sink_id) {}
-
-  FakeFrameSinkManagerClient(const FrameSinkId& frame_sink_id,
-                             FrameSinkManagerImpl* manager)
-      : source_(nullptr), manager_(nullptr), frame_sink_id_(frame_sink_id) {
-    DCHECK(manager);
-    Register(manager);
-  }
-
-  ~FakeFrameSinkManagerClient() override {
-    if (manager_) {
-      Unregister();
-    }
-    EXPECT_EQ(nullptr, source_);
-  }
-
-  BeginFrameSource* source() { return source_; }
-  const FrameSinkId& frame_sink_id() { return frame_sink_id_; }
-
-  void Register(FrameSinkManagerImpl* manager) {
-    EXPECT_EQ(nullptr, manager_);
-    manager_ = manager;
-    manager_->RegisterFrameSinkManagerClient(frame_sink_id_, this);
-  }
-
-  void Unregister() {
-    EXPECT_NE(manager_, nullptr);
-    manager_->UnregisterFrameSinkManagerClient(frame_sink_id_);
-    manager_ = nullptr;
-  }
-
-  // FrameSinkManagerClient implementation.
-  void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override {
-    DCHECK(!source_ || !begin_frame_source);
-    source_ = begin_frame_source;
-  }
-
- private:
-  BeginFrameSource* source_;
-  FrameSinkManagerImpl* manager_;
-  FrameSinkId frame_sink_id_;
-};
+}  // namespace
 
 class FrameSinkManagerTest : public testing::Test {
  public:
   FrameSinkManagerTest() = default;
   ~FrameSinkManagerTest() override = default;
 
+  std::unique_ptr<CompositorFrameSinkSupport> CreateCompositorFrameSinkSupport(
+      const FrameSinkId& frame_sink_id) {
+    return CompositorFrameSinkSupport::Create(nullptr, &manager_, frame_sink_id,
+                                              false, false);
+  }
+
+  const BeginFrameSource* GetBeginFrameSource(
+      const std::unique_ptr<CompositorFrameSinkSupport>& support) {
+    return support->begin_frame_source_;
+  }
+
  protected:
   FrameSinkManagerImpl manager_;
 };
 
 TEST_F(FrameSinkManagerTest, SingleClients) {
-  FakeFrameSinkManagerClient client(FrameSinkId(1, 1));
-  FakeFrameSinkManagerClient other_client(FrameSinkId(2, 2));
+  auto client = CreateCompositorFrameSinkSupport(FrameSinkId(1, 1));
+  auto other_client = CreateCompositorFrameSinkSupport(FrameSinkId(2, 2));
   StubBeginFrameSource source;
 
-  EXPECT_EQ(nullptr, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
-  client.Register(&manager_);
-  other_client.Register(&manager_);
-  EXPECT_EQ(nullptr, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(other_client));
 
   // Test setting unsetting BFS
-  manager_.RegisterBeginFrameSource(&source, client.frame_sink_id());
-  EXPECT_EQ(&source, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
+  manager_.RegisterBeginFrameSource(&source, client->frame_sink_id());
+  EXPECT_EQ(&source, GetBeginFrameSource(client));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(other_client));
   manager_.UnregisterBeginFrameSource(&source);
-  EXPECT_EQ(nullptr, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(other_client));
 
   // Set BFS for other namespace
-  manager_.RegisterBeginFrameSource(&source, other_client.frame_sink_id());
-  EXPECT_EQ(&source, other_client.source());
-  EXPECT_EQ(nullptr, client.source());
+  manager_.RegisterBeginFrameSource(&source, other_client->frame_sink_id());
+  EXPECT_EQ(&source, GetBeginFrameSource(other_client));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client));
   manager_.UnregisterBeginFrameSource(&source);
-  EXPECT_EQ(nullptr, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(other_client));
 
   // Re-set BFS for original
-  manager_.RegisterBeginFrameSource(&source, client.frame_sink_id());
-  EXPECT_EQ(&source, client.source());
+  manager_.RegisterBeginFrameSource(&source, client->frame_sink_id());
+  EXPECT_EQ(&source, GetBeginFrameSource(client));
   manager_.UnregisterBeginFrameSource(&source);
-  EXPECT_EQ(nullptr, client.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client));
 }
 
 // This test verifies that a client is still connected to the BeginFrameSource
 // after restart.
 TEST_F(FrameSinkManagerTest, ClientRestart) {
-  FakeFrameSinkManagerClient client(kArbitraryFrameSinkId);
+  auto client = CreateCompositorFrameSinkSupport(kArbitraryFrameSinkId);
   StubBeginFrameSource source;
-  client.Register(&manager_);
 
   manager_.RegisterBeginFrameSource(&source, kArbitraryFrameSinkId);
-  EXPECT_EQ(&source, client.source());
+  EXPECT_EQ(&source, GetBeginFrameSource(client));
 
-  // |client| is disconnect from |source| after Unregister.
-  client.Unregister();
-  EXPECT_EQ(nullptr, client.source());
+  client.reset();
 
-  // |client| is reconnected with |source| after re-Register.
-  client.Register(&manager_);
-  EXPECT_EQ(&source, client.source());
+  // |client| is reconnected with |source| after being recreated..
+  client = CreateCompositorFrameSinkSupport(kArbitraryFrameSinkId);
+  EXPECT_EQ(&source, GetBeginFrameSource(client));
 
   manager_.UnregisterBeginFrameSource(&source);
-  EXPECT_EQ(nullptr, client.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client));
 }
 
 // This test verifies that a PrimaryBeginFrameSource will receive BeginFrames
@@ -138,17 +98,17 @@ TEST_F(FrameSinkManagerTest, PrimaryBeginFrameSource) {
   BeginFrameSource* begin_frame_source = manager_.GetPrimaryBeginFrameSource();
   begin_frame_source->AddObserver(&obs);
 
-  FakeFrameSinkManagerClient root1(FrameSinkId(1, 1), &manager_);
+  auto root1 = CreateCompositorFrameSinkSupport(FrameSinkId(1, 1));
   std::unique_ptr<FakeExternalBeginFrameSource> external_source1 =
       base::MakeUnique<FakeExternalBeginFrameSource>(60.f, false);
   manager_.RegisterBeginFrameSource(external_source1.get(),
-                                    root1.frame_sink_id());
+                                    root1->frame_sink_id());
 
-  FakeFrameSinkManagerClient root2(FrameSinkId(2, 2), &manager_);
+  auto root2 = CreateCompositorFrameSinkSupport(FrameSinkId(2, 2));
   std::unique_ptr<FakeExternalBeginFrameSource> external_source2 =
       base::MakeUnique<FakeExternalBeginFrameSource>(60.f, false);
   manager_.RegisterBeginFrameSource(external_source2.get(),
-                                    root2.frame_sink_id());
+                                    root2->frame_sink_id());
 
   // Ticking |external_source2| does not propagate to |begin_frame_source|.
   {
@@ -200,70 +160,70 @@ TEST_F(FrameSinkManagerTest, MultipleDisplays) {
 
   // root1 -> A -> B
   // root2 -> C
-  FakeFrameSinkManagerClient root1(FrameSinkId(1, 1), &manager_);
-  FakeFrameSinkManagerClient root2(FrameSinkId(2, 2), &manager_);
-  FakeFrameSinkManagerClient client_a(FrameSinkId(3, 3), &manager_);
-  FakeFrameSinkManagerClient client_b(FrameSinkId(4, 4), &manager_);
-  FakeFrameSinkManagerClient client_c(FrameSinkId(5, 5), &manager_);
+  auto root1 = CreateCompositorFrameSinkSupport(FrameSinkId(1, 1));
+  auto root2 = CreateCompositorFrameSinkSupport(FrameSinkId(2, 2));
+  auto client_a = CreateCompositorFrameSinkSupport(FrameSinkId(3, 3));
+  auto client_b = CreateCompositorFrameSinkSupport(FrameSinkId(4, 4));
+  auto client_c = CreateCompositorFrameSinkSupport(FrameSinkId(5, 5));
 
-  manager_.RegisterBeginFrameSource(&root1_source, root1.frame_sink_id());
-  manager_.RegisterBeginFrameSource(&root2_source, root2.frame_sink_id());
-  EXPECT_EQ(root1.source(), &root1_source);
-  EXPECT_EQ(root2.source(), &root2_source);
+  manager_.RegisterBeginFrameSource(&root1_source, root1->frame_sink_id());
+  manager_.RegisterBeginFrameSource(&root2_source, root2->frame_sink_id());
+  EXPECT_EQ(GetBeginFrameSource(root1), &root1_source);
+  EXPECT_EQ(GetBeginFrameSource(root2), &root2_source);
 
   // Set up initial hierarchy.
-  manager_.RegisterFrameSinkHierarchy(root1.frame_sink_id(),
-                                      client_a.frame_sink_id());
-  EXPECT_EQ(client_a.source(), root1.source());
-  manager_.RegisterFrameSinkHierarchy(client_a.frame_sink_id(),
-                                      client_b.frame_sink_id());
-  EXPECT_EQ(client_b.source(), root1.source());
-  manager_.RegisterFrameSinkHierarchy(root2.frame_sink_id(),
-                                      client_c.frame_sink_id());
-  EXPECT_EQ(client_c.source(), root2.source());
+  manager_.RegisterFrameSinkHierarchy(root1->frame_sink_id(),
+                                      client_a->frame_sink_id());
+  EXPECT_EQ(GetBeginFrameSource(client_a), GetBeginFrameSource(root1));
+  manager_.RegisterFrameSinkHierarchy(client_a->frame_sink_id(),
+                                      client_b->frame_sink_id());
+  EXPECT_EQ(GetBeginFrameSource(client_b), GetBeginFrameSource(root1));
+  manager_.RegisterFrameSinkHierarchy(root2->frame_sink_id(),
+                                      client_c->frame_sink_id());
+  EXPECT_EQ(GetBeginFrameSource(client_c), GetBeginFrameSource(root2));
 
   // Attach A into root2's subtree, like a window moving across displays.
   // root1 -> A -> B
   // root2 -> C -> A -> B
-  manager_.RegisterFrameSinkHierarchy(client_c.frame_sink_id(),
-                                      client_a.frame_sink_id());
+  manager_.RegisterFrameSinkHierarchy(client_c->frame_sink_id(),
+                                      client_a->frame_sink_id());
   // With the heuristic of just keeping existing BFS in the face of multiple,
   // no client sources should change.
-  EXPECT_EQ(client_a.source(), root1.source());
-  EXPECT_EQ(client_b.source(), root1.source());
-  EXPECT_EQ(client_c.source(), root2.source());
+  EXPECT_EQ(GetBeginFrameSource(client_a), GetBeginFrameSource(root1));
+  EXPECT_EQ(GetBeginFrameSource(client_b), GetBeginFrameSource(root1));
+  EXPECT_EQ(GetBeginFrameSource(client_c), GetBeginFrameSource(root2));
 
-  // Detach A from root1.  A and B should now be updated to root2.
-  manager_.UnregisterFrameSinkHierarchy(root1.frame_sink_id(),
-                                        client_a.frame_sink_id());
-  EXPECT_EQ(client_a.source(), root2.source());
-  EXPECT_EQ(client_b.source(), root2.source());
-  EXPECT_EQ(client_c.source(), root2.source());
+  // Detach A from root1->  A and B should now be updated to root2->
+  manager_.UnregisterFrameSinkHierarchy(root1->frame_sink_id(),
+                                        client_a->frame_sink_id());
+  EXPECT_EQ(GetBeginFrameSource(client_a), GetBeginFrameSource(root2));
+  EXPECT_EQ(GetBeginFrameSource(client_b), GetBeginFrameSource(root2));
+  EXPECT_EQ(GetBeginFrameSource(client_c), GetBeginFrameSource(root2));
 
   // Detach root1 from BFS.  root1 should now have no source.
   manager_.UnregisterBeginFrameSource(&root1_source);
-  EXPECT_EQ(nullptr, root1.source());
-  EXPECT_NE(nullptr, root2.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(root1));
+  EXPECT_NE(nullptr, GetBeginFrameSource(root2));
 
   // Detatch root2 from BFS.
   manager_.UnregisterBeginFrameSource(&root2_source);
-  EXPECT_EQ(nullptr, client_a.source());
-  EXPECT_EQ(nullptr, client_b.source());
-  EXPECT_EQ(nullptr, client_c.source());
-  EXPECT_EQ(nullptr, root2.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client_a));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client_b));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client_c));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(root2));
 
   // Cleanup hierarchy.
-  manager_.UnregisterFrameSinkHierarchy(root2.frame_sink_id(),
-                                        client_c.frame_sink_id());
-  manager_.UnregisterFrameSinkHierarchy(client_c.frame_sink_id(),
-                                        client_a.frame_sink_id());
-  manager_.UnregisterFrameSinkHierarchy(client_a.frame_sink_id(),
-                                        client_b.frame_sink_id());
+  manager_.UnregisterFrameSinkHierarchy(root2->frame_sink_id(),
+                                        client_c->frame_sink_id());
+  manager_.UnregisterFrameSinkHierarchy(client_c->frame_sink_id(),
+                                        client_a->frame_sink_id());
+  manager_.UnregisterFrameSinkHierarchy(client_a->frame_sink_id(),
+                                        client_b->frame_sink_id());
 }
 
 // This test verifies that a BeginFrameSource path to the root from a
 // FrameSinkId is preserved even if that FrameSinkId has no children
-// and does not have a corresponding FrameSinkManagerClient.
+// and does not have a corresponding CompositorFrameSinkSupport.
 TEST_F(FrameSinkManagerTest, ParentWithoutClientRetained) {
   StubBeginFrameSource root_source;
 
@@ -272,30 +232,30 @@ TEST_F(FrameSinkManagerTest, ParentWithoutClientRetained) {
   constexpr FrameSinkId kFrameSinkIdB(3, 3);
   constexpr FrameSinkId kFrameSinkIdC(4, 4);
 
-  FakeFrameSinkManagerClient root(kFrameSinkIdRoot, &manager_);
-  FakeFrameSinkManagerClient client_b(kFrameSinkIdB, &manager_);
-  FakeFrameSinkManagerClient client_c(kFrameSinkIdC, &manager_);
+  auto root = CreateCompositorFrameSinkSupport(kFrameSinkIdRoot);
+  auto client_b = CreateCompositorFrameSinkSupport(kFrameSinkIdB);
+  auto client_c = CreateCompositorFrameSinkSupport(kFrameSinkIdC);
 
-  manager_.RegisterBeginFrameSource(&root_source, root.frame_sink_id());
-  EXPECT_EQ(&root_source, root.source());
+  manager_.RegisterBeginFrameSource(&root_source, root->frame_sink_id());
+  EXPECT_EQ(&root_source, GetBeginFrameSource(root));
 
   // Set up initial hierarchy: root -> A -> B.
-  // Note that A does not have a FrameSinkManagerClient.
+  // Note that A does not have a CompositorFrameSinkSupport.
   manager_.RegisterFrameSinkHierarchy(kFrameSinkIdRoot, kFrameSinkIdA);
   manager_.RegisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdB);
   // The root's BeginFrameSource should propagate to B.
-  EXPECT_EQ(root.source(), client_b.source());
+  EXPECT_EQ(GetBeginFrameSource(root), GetBeginFrameSource(client_b));
 
   // Unregister B, and attach C to A: root -> A -> C
   manager_.UnregisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdB);
-  EXPECT_EQ(nullptr, client_b.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client_b));
   manager_.RegisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdC);
   // The root's BeginFrameSource should propagate to C.
-  EXPECT_EQ(root.source(), client_c.source());
+  EXPECT_EQ(GetBeginFrameSource(root), GetBeginFrameSource(client_c));
 
   manager_.UnregisterBeginFrameSource(&root_source);
-  EXPECT_EQ(nullptr, root.source());
-  EXPECT_EQ(nullptr, client_c.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(root));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client_c));
 }
 
 // This test sets up the same hierarchy as ParentWithoutClientRetained.
@@ -311,52 +271,66 @@ TEST_F(FrameSinkManagerTest,
   constexpr FrameSinkId kFrameSinkIdB(3, 3);
   constexpr FrameSinkId kFrameSinkIdC(4, 4);
 
-  FakeFrameSinkManagerClient root(kFrameSinkIdRoot, &manager_);
-  FakeFrameSinkManagerClient client_b(kFrameSinkIdB, &manager_);
-  FakeFrameSinkManagerClient client_c(kFrameSinkIdC, &manager_);
+  auto root = CreateCompositorFrameSinkSupport(kFrameSinkIdRoot);
+  auto client_b = CreateCompositorFrameSinkSupport(kFrameSinkIdB);
+  auto client_c = CreateCompositorFrameSinkSupport(kFrameSinkIdC);
 
   // Set up initial hierarchy: root -> A -> B.
-  // Note that A does not have a FrameSinkManagerClient.
+  // Note that A does not have a CompositorFrameSinkSupport.
   manager_.RegisterFrameSinkHierarchy(kFrameSinkIdRoot, kFrameSinkIdA);
   manager_.RegisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdB);
   // The root does not yet have a BeginFrameSource so client B should not have
   // one either.
-  EXPECT_EQ(nullptr, client_b.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client_b));
 
   // Unregister B, and attach C to A: root -> A -> C
   manager_.UnregisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdB);
   manager_.RegisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdC);
 
   // Registering a BeginFrameSource at the root should propagate it to C.
-  manager_.RegisterBeginFrameSource(&root_source, root.frame_sink_id());
+  manager_.RegisterBeginFrameSource(&root_source, root->frame_sink_id());
   // The root's BeginFrameSource should propagate to C.
-  EXPECT_EQ(&root_source, root.source());
-  EXPECT_EQ(root.source(), client_c.source());
+  EXPECT_EQ(&root_source, GetBeginFrameSource(root));
+  EXPECT_EQ(GetBeginFrameSource(root), GetBeginFrameSource(client_c));
 
   manager_.UnregisterBeginFrameSource(&root_source);
-  EXPECT_EQ(nullptr, root.source());
-  EXPECT_EQ(nullptr, client_c.source());
+  EXPECT_EQ(nullptr, GetBeginFrameSource(root));
+  EXPECT_EQ(nullptr, GetBeginFrameSource(client_c));
 }
 
+namespace {
+
+constexpr FrameSinkId kClientIdA(1, 1);
+constexpr FrameSinkId kClientIdB(2, 2);
+constexpr FrameSinkId kClientIdC(3, 3);
+
+enum RegisterOrder { REGISTER_HIERARCHY_FIRST, REGISTER_CLIENTS_FIRST };
+enum UnregisterOrder { UNREGISTER_HIERARCHY_FIRST, UNREGISTER_CLIENTS_FIRST };
+enum BFSOrder { BFS_FIRST, BFS_SECOND, BFS_THIRD };
+
+static const RegisterOrder kRegisterOrderList[] = {REGISTER_HIERARCHY_FIRST,
+                                                   REGISTER_CLIENTS_FIRST};
+static const UnregisterOrder kUnregisterOrderList[] = {
+    UNREGISTER_HIERARCHY_FIRST, UNREGISTER_CLIENTS_FIRST};
+static const BFSOrder kBFSOrderList[] = {BFS_FIRST, BFS_SECOND, BFS_THIRD};
+
+}  // namespace
+
 // In practice, registering and unregistering both parent/child relationships
-// and FrameSinkManagerClients can happen in any ordering with respect to
+// and CompositorFrameSinkSupports can happen in any ordering with respect to
 // each other.  These following tests verify that all the data structures
 // are properly set up and cleaned up under the four permutations of orderings
 // of this nesting.
-
-class SurfaceManagerOrderingTest : public FrameSinkManagerTest {
+class FrameSinkManagerOrderingTest : public FrameSinkManagerTest {
  public:
-  SurfaceManagerOrderingTest()
-      : client_a_(FrameSinkId(1, 1)),
-        client_b_(FrameSinkId(2, 2)),
-        client_c_(FrameSinkId(3, 3)),
-        hierarchy_registered_(false),
+  FrameSinkManagerOrderingTest()
+      : hierarchy_registered_(false),
         clients_registered_(false),
         bfs_registered_(false) {
     AssertCorrectBFSState();
   }
 
-  ~SurfaceManagerOrderingTest() override {
+  ~FrameSinkManagerOrderingTest() override {
     EXPECT_FALSE(hierarchy_registered_);
     EXPECT_FALSE(clients_registered_);
     EXPECT_FALSE(bfs_registered_);
@@ -366,44 +340,40 @@ class SurfaceManagerOrderingTest : public FrameSinkManagerTest {
   void RegisterHierarchy() {
     DCHECK(!hierarchy_registered_);
     hierarchy_registered_ = true;
-    manager_.RegisterFrameSinkHierarchy(client_a_.frame_sink_id(),
-                                        client_b_.frame_sink_id());
-    manager_.RegisterFrameSinkHierarchy(client_b_.frame_sink_id(),
-                                        client_c_.frame_sink_id());
+    manager_.RegisterFrameSinkHierarchy(kClientIdA, kClientIdB);
+    manager_.RegisterFrameSinkHierarchy(kClientIdB, kClientIdC);
     AssertCorrectBFSState();
   }
   void UnregisterHierarchy() {
     DCHECK(hierarchy_registered_);
     hierarchy_registered_ = false;
-    manager_.UnregisterFrameSinkHierarchy(client_a_.frame_sink_id(),
-                                          client_b_.frame_sink_id());
-    manager_.UnregisterFrameSinkHierarchy(client_b_.frame_sink_id(),
-                                          client_c_.frame_sink_id());
+    manager_.UnregisterFrameSinkHierarchy(kClientIdA, kClientIdB);
+    manager_.UnregisterFrameSinkHierarchy(kClientIdB, kClientIdC);
     AssertCorrectBFSState();
   }
 
   void RegisterClients() {
     DCHECK(!clients_registered_);
     clients_registered_ = true;
-    client_a_.Register(&manager_);
-    client_b_.Register(&manager_);
-    client_c_.Register(&manager_);
+    client_a_ = CreateCompositorFrameSinkSupport(kClientIdA);
+    client_b_ = CreateCompositorFrameSinkSupport(kClientIdB);
+    client_c_ = CreateCompositorFrameSinkSupport(kClientIdC);
     AssertCorrectBFSState();
   }
 
   void UnregisterClients() {
     DCHECK(clients_registered_);
     clients_registered_ = false;
-    client_a_.Unregister();
-    client_b_.Unregister();
-    client_c_.Unregister();
+    client_a_.reset();
+    client_b_.reset();
+    client_c_.reset();
     AssertCorrectBFSState();
   }
 
   void RegisterBFS() {
     DCHECK(!bfs_registered_);
     bfs_registered_ = true;
-    manager_.RegisterBeginFrameSource(&source_, client_a_.frame_sink_id());
+    manager_.RegisterBeginFrameSource(&source_, kClientIdA);
     AssertCorrectBFSState();
   }
   void UnregisterBFS() {
@@ -414,28 +384,32 @@ class SurfaceManagerOrderingTest : public FrameSinkManagerTest {
   }
 
   void AssertEmptyBFS() {
-    EXPECT_EQ(nullptr, client_a_.source());
-    EXPECT_EQ(nullptr, client_b_.source());
-    EXPECT_EQ(nullptr, client_c_.source());
+    EXPECT_EQ(nullptr, GetBeginFrameSource(client_a_));
+    EXPECT_EQ(nullptr, GetBeginFrameSource(client_b_));
+    EXPECT_EQ(nullptr, GetBeginFrameSource(client_c_));
   }
 
   void AssertAllValidBFS() {
-    EXPECT_EQ(&source_, client_a_.source());
-    EXPECT_EQ(&source_, client_b_.source());
-    EXPECT_EQ(&source_, client_c_.source());
+    EXPECT_EQ(&source_, GetBeginFrameSource(client_a_));
+    EXPECT_EQ(&source_, GetBeginFrameSource(client_b_));
+    EXPECT_EQ(&source_, GetBeginFrameSource(client_c_));
   }
 
  protected:
   void AssertCorrectBFSState() {
-    if (!clients_registered_ || !bfs_registered_) {
+    if (!clients_registered_)
+      return;
+
+    if (!bfs_registered_) {
       AssertEmptyBFS();
       return;
     }
+
     if (!hierarchy_registered_) {
       // A valid but not attached to anything.
-      EXPECT_EQ(&source_, client_a_.source());
-      EXPECT_EQ(nullptr, client_b_.source());
-      EXPECT_EQ(nullptr, client_c_.source());
+      EXPECT_EQ(&source_, GetBeginFrameSource(client_a_));
+      EXPECT_EQ(nullptr, GetBeginFrameSource(client_b_));
+      EXPECT_EQ(nullptr, GetBeginFrameSource(client_c_));
       return;
     }
 
@@ -444,31 +418,21 @@ class SurfaceManagerOrderingTest : public FrameSinkManagerTest {
 
   StubBeginFrameSource source_;
   // A -> B -> C hierarchy, with A always having the BFS.
-  FakeFrameSinkManagerClient client_a_;
-  FakeFrameSinkManagerClient client_b_;
-  FakeFrameSinkManagerClient client_c_;
+  std::unique_ptr<CompositorFrameSinkSupport> client_a_;
+  std::unique_ptr<CompositorFrameSinkSupport> client_b_;
+  std::unique_ptr<CompositorFrameSinkSupport> client_c_;
 
   bool hierarchy_registered_;
   bool clients_registered_;
   bool bfs_registered_;
 };
 
-enum RegisterOrder { REGISTER_HIERARCHY_FIRST, REGISTER_CLIENTS_FIRST };
-enum UnregisterOrder { UNREGISTER_HIERARCHY_FIRST, UNREGISTER_CLIENTS_FIRST };
-enum BFSOrder { BFS_FIRST, BFS_SECOND, BFS_THIRD };
-
-static const RegisterOrder kRegisterOrderList[] = {REGISTER_HIERARCHY_FIRST,
-                                                   REGISTER_CLIENTS_FIRST};
-static const UnregisterOrder kUnregisterOrderList[] = {
-    UNREGISTER_HIERARCHY_FIRST, UNREGISTER_CLIENTS_FIRST};
-static const BFSOrder kBFSOrderList[] = {BFS_FIRST, BFS_SECOND, BFS_THIRD};
-
-class SurfaceManagerOrderingParamTest
-    : public SurfaceManagerOrderingTest,
+class FrameSinkManagerOrderingParamTest
+    : public FrameSinkManagerOrderingTest,
       public ::testing::WithParamInterface<
           std::tr1::tuple<RegisterOrder, UnregisterOrder, BFSOrder>> {};
 
-TEST_P(SurfaceManagerOrderingParamTest, Ordering) {
+TEST_P(FrameSinkManagerOrderingParamTest, Ordering) {
   // Test the four permutations of client/hierarchy setting/unsetting and test
   // each place the BFS can be added and removed.  The BFS and the
   // client/hierarchy are less related, so BFS is tested independently instead
@@ -523,11 +487,10 @@ TEST_P(SurfaceManagerOrderingParamTest, Ordering) {
 }
 
 INSTANTIATE_TEST_CASE_P(
-    SurfaceManagerOrderingParamTestInstantiation,
-    SurfaceManagerOrderingParamTest,
+    FrameSinkManagerOrderingParamTestInstantiation,
+    FrameSinkManagerOrderingParamTest,
     ::testing::Combine(::testing::ValuesIn(kRegisterOrderList),
                        ::testing::ValuesIn(kUnregisterOrderList),
                        ::testing::ValuesIn(kBFSOrderList)));
 
-}  // namespace
 }  // namespace viz