[cc]Replace use of SurfaceFactory with CompositorFrameSinkSupport in tests

cc/surfaces/surface_manager_unittest.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 <stddef.h>
 
 #include "cc/scheduler/begin_frame_source.h"
-#include "cc/surfaces/surface_factory_client.h"
+#include "cc/surfaces/compositor_frame_sink_support.h"
 #include "cc/surfaces/surface_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace cc {
+namespace {
 
-class FakeSurfaceFactoryClient : public SurfaceFactoryClient {
- public:
-  explicit FakeSurfaceFactoryClient(const FrameSinkId& frame_sink_id)
-      : source_(nullptr), manager_(nullptr), frame_sink_id_(frame_sink_id) {}
+constexpr bool kIsRoot = true;
+constexpr bool kIsChildRoot = false;
+constexpr bool kHandlesFrameSinkIdInvalidation = false;
+constexpr bool kNeedsSyncPoints = true;
+constexpr FrameSinkId kFrameSinkIdA = FrameSinkId(1, 1);
+constexpr FrameSinkId kFrameSinkIdB = FrameSinkId(2, 2);
+constexpr FrameSinkId kFrameSinkIdC = FrameSinkId(3, 3);
 
-  FakeSurfaceFactoryClient(const FrameSinkId& frame_sink_id,
-                           SurfaceManager* manager)
-      : source_(nullptr), manager_(nullptr), frame_sink_id_(frame_sink_id) {
-    DCHECK(manager);
-    Register(manager);
-  }
-
-  ~FakeSurfaceFactoryClient() override {
-    if (manager_) {
-      Unregister();
-    }
-    EXPECT_EQ(nullptr, source_);
-  }
-
-  BeginFrameSource* source() { return source_; }
-  const FrameSinkId& frame_sink_id() { return frame_sink_id_; }
-
-  void Register(SurfaceManager* manager) {
-    EXPECT_EQ(nullptr, manager_);
-    manager_ = manager;
-    manager_->RegisterSurfaceFactoryClient(frame_sink_id_, this);
-  }
-
-  void Unregister() {
-    EXPECT_NE(manager_, nullptr);
-    manager_->UnregisterSurfaceFactoryClient(frame_sink_id_);
-    manager_ = nullptr;
-  }
-
-  // SurfaceFactoryClient implementation.
-  void ReturnResources(const ReturnedResourceArray& resources) override {}
-  void SetBeginFrameSource(BeginFrameSource* begin_frame_source) override {
-    DCHECK(!source_ || !begin_frame_source);
-    source_ = begin_frame_source;
-  };
-
- private:
-  BeginFrameSource* source_;
-  SurfaceManager* manager_;
-  FrameSinkId frame_sink_id_;
-};
+}  // namespace

[danakj, 2017/04/07 15:52:45]

we usually wrap the whole test file in anon namespace fwiw, this is the easiest
way to ensure no symbols collide

[Alex Z., 2017/04/07 20:43:58]

Done.

 
 class SurfaceManagerTest : public testing::Test {
  public:
   // These tests don't care about namespace registration, so just preregister
   // a set of namespaces that tests can use freely without worrying if they're
   // valid or not.
   enum { MAX_FRAME_SINK = 10 };
 
   SurfaceManagerTest() {
     for (size_t i = 0; i < MAX_FRAME_SINK; ++i)
       manager_.RegisterFrameSinkId(FrameSinkId(i, i));
   }
 
   ~SurfaceManagerTest() override {
     for (size_t i = 0; i < MAX_FRAME_SINK; ++i)
       manager_.InvalidateFrameSinkId(FrameSinkId(i, i));
   }
 
  protected:
   SurfaceManager manager_;
 };
 
 TEST_F(SurfaceManagerTest, SingleClients) {
-  FakeSurfaceFactoryClient client(FrameSinkId(1, 1));
-  FakeSurfaceFactoryClient other_client(FrameSinkId(2, 2));
+  CompositorFrameSinkSupport client(
+      nullptr, &manager_, FrameSinkId(1, 1), kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
+  CompositorFrameSinkSupport other_client(
+      nullptr, &manager_, FrameSinkId(2, 2), kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
   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, client.BeginFrameSourceForTesting());

[danakj, 2017/04/07 15:52:45]

I don't think you need to add a ForTesting() method to do this. That's the last
kind of resort to take.

In this case you're testing the use of the SurfaceFactoryClient interface on
CompositorFrameSinkSupport. So you could make a mock subclass of
CompositorFrameSink support that has a mock SetBeginFrameSource method, and you
can EXPECT_CALL on it, instead of peeking at internal state of
CompositorFrameSupport?

[Alex Z., 2017/04/07 20:43:58]

I added a mock class (see the next patchset) but it causes the 
Ordering test to fail. I think it's because 
CompositorFrameSinkSupport() calls RegisterSurfaceFactoryClient() 
before the mock gets set up. Is there any work around?

[danakj, 2017/04/07 21:16:17]

Does this imply that virtual methods on CompositorFrameSupport are being called
from inside its constructor? If so that's bad C++. Or did I misunderstand?

[Alex Z., 2017/04/07 21:29:01]

MockCompositorFrameSinkSupport is a friend of 
CompositorFrameSinkSupport in the new patchset following fsamuel@'s 
suggestion. This way we can access begin_frame_source_ in the tests 
without exposing it to everywhere else. MockCompositorFrameSinkSupport 
also doesn't need the complexity of the mock macros.

+  EXPECT_EQ(nullptr, other_client.BeginFrameSourceForTesting());
 
   // Test setting unsetting BFS
   manager_.RegisterBeginFrameSource(&source, client.frame_sink_id());
-  EXPECT_EQ(&source, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
+  EXPECT_EQ(&source, client.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, other_client.BeginFrameSourceForTesting());
   manager_.UnregisterBeginFrameSource(&source);
-  EXPECT_EQ(nullptr, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
+  EXPECT_EQ(nullptr, client.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, other_client.BeginFrameSourceForTesting());
 
   // Set BFS for other namespace
   manager_.RegisterBeginFrameSource(&source, other_client.frame_sink_id());
-  EXPECT_EQ(&source, other_client.source());
-  EXPECT_EQ(nullptr, client.source());
+  EXPECT_EQ(&source, other_client.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, client.BeginFrameSourceForTesting());
   manager_.UnregisterBeginFrameSource(&source);
-  EXPECT_EQ(nullptr, client.source());
-  EXPECT_EQ(nullptr, other_client.source());
+  EXPECT_EQ(nullptr, client.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, other_client.BeginFrameSourceForTesting());
 
   // Re-set BFS for original
   manager_.RegisterBeginFrameSource(&source, client.frame_sink_id());
-  EXPECT_EQ(&source, client.source());
+  EXPECT_EQ(&source, client.BeginFrameSourceForTesting());
   manager_.UnregisterBeginFrameSource(&source);
-  EXPECT_EQ(nullptr, client.source());
+  EXPECT_EQ(nullptr, client.BeginFrameSourceForTesting());
 }
 
 TEST_F(SurfaceManagerTest, MultipleDisplays) {
   StubBeginFrameSource root1_source;
   StubBeginFrameSource root2_source;
 
   // root1 -> A -> B
   // root2 -> C
-  FakeSurfaceFactoryClient root1(FrameSinkId(1, 1), &manager_);
-  FakeSurfaceFactoryClient root2(FrameSinkId(2, 2), &manager_);
-  FakeSurfaceFactoryClient client_a(FrameSinkId(3, 3), &manager_);
-  FakeSurfaceFactoryClient client_b(FrameSinkId(4, 4), &manager_);
-  FakeSurfaceFactoryClient client_c(FrameSinkId(5, 5), &manager_);
+  CompositorFrameSinkSupport root1(nullptr, &manager_, FrameSinkId(1, 1),
+                                   kIsRoot, kHandlesFrameSinkIdInvalidation,
+                                   kNeedsSyncPoints);
+  CompositorFrameSinkSupport root2(nullptr, &manager_, FrameSinkId(2, 2),
+                                   kIsRoot, kHandlesFrameSinkIdInvalidation,
+                                   kNeedsSyncPoints);
+  CompositorFrameSinkSupport client_a(
+      nullptr, &manager_, FrameSinkId(3, 3), kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
+  CompositorFrameSinkSupport client_b(
+      nullptr, &manager_, FrameSinkId(4, 4), kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
+  CompositorFrameSinkSupport client_c(
+      nullptr, &manager_, FrameSinkId(5, 5), kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
 
   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);
+  EXPECT_EQ(root1.BeginFrameSourceForTesting(), &root1_source);
+  EXPECT_EQ(root2.BeginFrameSourceForTesting(), &root2_source);
 
   // Set up initial hierarchy.
   manager_.RegisterFrameSinkHierarchy(root1.frame_sink_id(),
                                       client_a.frame_sink_id());
-  EXPECT_EQ(client_a.source(), root1.source());
+  EXPECT_EQ(client_a.BeginFrameSourceForTesting(),
+            root1.BeginFrameSourceForTesting());
   manager_.RegisterFrameSinkHierarchy(client_a.frame_sink_id(),
                                       client_b.frame_sink_id());
-  EXPECT_EQ(client_b.source(), root1.source());
+  EXPECT_EQ(client_b.BeginFrameSourceForTesting(),
+            root1.BeginFrameSourceForTesting());
   manager_.RegisterFrameSinkHierarchy(root2.frame_sink_id(),
                                       client_c.frame_sink_id());
-  EXPECT_EQ(client_c.source(), root2.source());
+  EXPECT_EQ(client_c.BeginFrameSourceForTesting(),
+            root2.BeginFrameSourceForTesting());
 
   // 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());
   // 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(client_a.BeginFrameSourceForTesting(),
+            root1.BeginFrameSourceForTesting());
+  EXPECT_EQ(client_b.BeginFrameSourceForTesting(),
+            root1.BeginFrameSourceForTesting());
+  EXPECT_EQ(client_c.BeginFrameSourceForTesting(),
+            root2.BeginFrameSourceForTesting());
 
   // 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());
+  EXPECT_EQ(client_a.BeginFrameSourceForTesting(),
+            root2.BeginFrameSourceForTesting());
+  EXPECT_EQ(client_b.BeginFrameSourceForTesting(),
+            root2.BeginFrameSourceForTesting());
+  EXPECT_EQ(client_c.BeginFrameSourceForTesting(),
+            root2.BeginFrameSourceForTesting());
 
   // 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, root1.BeginFrameSourceForTesting());
+  EXPECT_NE(nullptr, root2.BeginFrameSourceForTesting());
 
   // 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, client_a.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, client_b.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, client_c.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, root2.BeginFrameSourceForTesting());
 
   // 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());
 }
 
 // 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 SurfaceFactoryClient.
 TEST_F(SurfaceManagerTest, ParentWithoutClientRetained) {
   StubBeginFrameSource root_source;
 
   constexpr FrameSinkId kFrameSinkIdRoot(1, 1);
   constexpr FrameSinkId kFrameSinkIdA(2, 2);
   constexpr FrameSinkId kFrameSinkIdB(3, 3);
   constexpr FrameSinkId kFrameSinkIdC(4, 4);
 
-  FakeSurfaceFactoryClient root(kFrameSinkIdRoot, &manager_);
-  FakeSurfaceFactoryClient client_b(kFrameSinkIdB, &manager_);
-  FakeSurfaceFactoryClient client_c(kFrameSinkIdC, &manager_);
+  CompositorFrameSinkSupport root(nullptr, &manager_, kFrameSinkIdRoot, kIsRoot,
+                                  kHandlesFrameSinkIdInvalidation,
+                                  kNeedsSyncPoints);
+  CompositorFrameSinkSupport client_b(
+      nullptr, &manager_, kFrameSinkIdB, kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
+  CompositorFrameSinkSupport client_c(
+      nullptr, &manager_, kFrameSinkIdC, kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
 
   manager_.RegisterBeginFrameSource(&root_source, root.frame_sink_id());
-  EXPECT_EQ(&root_source, root.source());
+  EXPECT_EQ(&root_source, root.BeginFrameSourceForTesting());
 
   // Set up initial hierarchy: root -> A -> B.
   // Note that A does not have a SurfaceFactoryClient.
   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(root.BeginFrameSourceForTesting(),
+            client_b.BeginFrameSourceForTesting());
 
   // Unregister B, and attach C to A: root -> A -> C
   manager_.UnregisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdB);
-  EXPECT_EQ(nullptr, client_b.source());
+  EXPECT_EQ(nullptr, client_b.BeginFrameSourceForTesting());
   manager_.RegisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdC);
   // The root's BeginFrameSource should propagate to C.
-  EXPECT_EQ(root.source(), client_c.source());
+  EXPECT_EQ(root.BeginFrameSourceForTesting(),
+            client_c.BeginFrameSourceForTesting());
 
   manager_.UnregisterBeginFrameSource(&root_source);
-  EXPECT_EQ(nullptr, root.source());
-  EXPECT_EQ(nullptr, client_c.source());
+  EXPECT_EQ(nullptr, root.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, client_c.BeginFrameSourceForTesting());
 }
 
 // This test sets up the same hierarchy as ParentWithoutClientRetained.
 // However, this unit test registers the BeginFrameSource AFTER C
 // has been attached to A. This test verifies that the BeginFrameSource
 // propagates all the way to C.
 TEST_F(SurfaceManagerTest,
        ParentWithoutClientRetained_LateBeginFrameRegistration) {
   StubBeginFrameSource root_source;
 
   constexpr FrameSinkId kFrameSinkIdRoot(1, 1);
   constexpr FrameSinkId kFrameSinkIdA(2, 2);
   constexpr FrameSinkId kFrameSinkIdB(3, 3);
   constexpr FrameSinkId kFrameSinkIdC(4, 4);
 
-  FakeSurfaceFactoryClient root(kFrameSinkIdRoot, &manager_);
-  FakeSurfaceFactoryClient client_b(kFrameSinkIdB, &manager_);
-  FakeSurfaceFactoryClient client_c(kFrameSinkIdC, &manager_);
+  CompositorFrameSinkSupport root(nullptr, &manager_, kFrameSinkIdRoot, kIsRoot,
+                                  kHandlesFrameSinkIdInvalidation,
+                                  kNeedsSyncPoints);
+  CompositorFrameSinkSupport client_b(
+      nullptr, &manager_, kFrameSinkIdB, kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
+  CompositorFrameSinkSupport client_c(
+      nullptr, &manager_, kFrameSinkIdC, kIsChildRoot,
+      kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
 
   // Set up initial hierarchy: root -> A -> B.
   // Note that A does not have a SurfaceFactoryClient.
   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, client_b.BeginFrameSourceForTesting());
 
   // 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());
   // 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, root.BeginFrameSourceForTesting());
+  EXPECT_EQ(root.BeginFrameSourceForTesting(),
+            client_c.BeginFrameSourceForTesting());
 
   manager_.UnregisterBeginFrameSource(&root_source);
-  EXPECT_EQ(nullptr, root.source());
-  EXPECT_EQ(nullptr, client_c.source());
+  EXPECT_EQ(nullptr, root.BeginFrameSourceForTesting());
+  EXPECT_EQ(nullptr, client_c.BeginFrameSourceForTesting());
 }
 
 // In practice, registering and unregistering both parent/child relationships
 // and SurfaceFactoryClients 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 SurfaceManagerTest {
  public:
   SurfaceManagerOrderingTest()
-      : client_a_(FrameSinkId(1, 1)),
-        client_b_(FrameSinkId(2, 2)),
-        client_c_(FrameSinkId(3, 3)),
-        hierarchy_registered_(false),
+      : hierarchy_registered_(false),
         clients_registered_(false),
         bfs_registered_(false) {
     AssertCorrectBFSState();
   }
 
   ~SurfaceManagerOrderingTest() override {
     EXPECT_FALSE(hierarchy_registered_);
     EXPECT_FALSE(clients_registered_);
     EXPECT_FALSE(bfs_registered_);
     AssertCorrectBFSState();
   }
 
   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(kFrameSinkIdA, kFrameSinkIdB);
+    manager_.RegisterFrameSinkHierarchy(kFrameSinkIdB, kFrameSinkIdC);
     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(kFrameSinkIdA, kFrameSinkIdB);
+    manager_.UnregisterFrameSinkHierarchy(kFrameSinkIdB, kFrameSinkIdC);
     AssertCorrectBFSState();
   }
 
   void RegisterClients() {
     DCHECK(!clients_registered_);
     clients_registered_ = true;
-    client_a_.Register(&manager_);
-    client_b_.Register(&manager_);
-    client_c_.Register(&manager_);
+    client_a_ = base::MakeUnique<CompositorFrameSinkSupport>(
+        nullptr, &manager_, kFrameSinkIdA, kIsRoot,
+        kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
+    client_b_ = base::MakeUnique<CompositorFrameSinkSupport>(
+        nullptr, &manager_, kFrameSinkIdB, kIsChildRoot,
+        kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
+    client_c_ = base::MakeUnique<CompositorFrameSinkSupport>(
+        nullptr, &manager_, kFrameSinkIdC, kIsChildRoot,
+        kHandlesFrameSinkIdInvalidation, kNeedsSyncPoints);
     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_, kFrameSinkIdA);
     AssertCorrectBFSState();
   }
   void UnregisterBFS() {
     DCHECK(bfs_registered_);
     bfs_registered_ = false;
     manager_.UnregisterBeginFrameSource(&source_);
     AssertCorrectBFSState();
   }
 
   void AssertEmptyBFS() {
-    EXPECT_EQ(nullptr, client_a_.source());
-    EXPECT_EQ(nullptr, client_b_.source());
-    EXPECT_EQ(nullptr, client_c_.source());
+    EXPECT_TRUE(!client_a_ || !client_a_->BeginFrameSourceForTesting());
+    EXPECT_TRUE(!client_b_ || !client_b_->BeginFrameSourceForTesting());
+    EXPECT_TRUE(!client_c_ || !client_c_->BeginFrameSourceForTesting());
   }
 
   void AssertAllValidBFS() {
-    EXPECT_EQ(&source_, client_a_.source());
-    EXPECT_EQ(&source_, client_b_.source());
-    EXPECT_EQ(&source_, client_c_.source());
+    EXPECT_EQ(&source_, client_a_->BeginFrameSourceForTesting());
+    EXPECT_EQ(&source_, client_b_->BeginFrameSourceForTesting());
+    EXPECT_EQ(&source_, client_c_->BeginFrameSourceForTesting());
   }
 
  protected:
   void AssertCorrectBFSState() {
     if (!clients_registered_ || !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_, client_a_->BeginFrameSourceForTesting());
+      EXPECT_TRUE(!client_b_ || !client_b_->BeginFrameSourceForTesting());
+      EXPECT_TRUE(!client_c_ || !client_c_->BeginFrameSourceForTesting());
       return;
     }
 
     AssertAllValidBFS();
   }
 
   StubBeginFrameSource source_;
   // A -> B -> C hierarchy, with A always having the BFS.
-  FakeSurfaceFactoryClient client_a_;
-  FakeSurfaceFactoryClient client_b_;
-  FakeSurfaceFactoryClient 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 };
 
@@ skipping 63 matching lines @@
 }
 
 INSTANTIATE_TEST_CASE_P(
     SurfaceManagerOrderingParamTestInstantiation,
     SurfaceManagerOrderingParamTest,
     ::testing::Combine(::testing::ValuesIn(kRegisterOrderList),
                        ::testing::ValuesIn(kUnregisterOrderList),
                        ::testing::ValuesIn(kBFSOrderList)));
 
 }  // namespace cc