cc/surfaces/surface_manager_unittest.cc - Issue 2795683003: [cc]Replace use of SurfaceFactory with CompositorFrameSinkSupport in tests

Unified Diff: cc/surfaces/surface_manager_unittest.cc

Issue 2795683003: [cc]Replace use of SurfaceFactory with CompositorFrameSinkSupport in tests (Closed)

Patch Set: Address comments Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: cc/surfaces/surface_manager_unittest.cc

diff --git a/cc/surfaces/surface_manager_unittest.cc b/cc/surfaces/surface_manager_unittest.cc

index 6142560acb4315e2bcfd8eddd156ce2929ff5905..c65ad8d768ef90bd8aca0d258ca47b65ea0ee657 100644

--- a/cc/surfaces/surface_manager_unittest.cc

+++ b/cc/surfaces/surface_manager_unittest.cc

@@ -5,58 +5,19 @@

#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) {}

- 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_; }

+constexpr bool is_root = true;

+constexpr bool is_root_child = false;

+constexpr bool handles_frame_sink_id_invalidation = false;

+constexpr bool needs_sync_points = true;

- 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

class SurfaceManagerTest : public testing::Test {

public:

@@ -80,38 +41,38 @@ class SurfaceManagerTest : public testing::Test {

};

TEST_F(SurfaceManagerTest, SingleClients) {

- FakeSurfaceFactoryClient client(FrameSinkId(1, 1));

- FakeSurfaceFactoryClient other_client(FrameSinkId(2, 2));

+ CompositorFrameSinkSupport client(

+ nullptr, &manager_, FrameSinkId(1, 1), is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

+ CompositorFrameSinkSupport other_client(

+ nullptr, &manager_, FrameSinkId(2, 2), is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

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.begin_frame_source());

+ EXPECT_EQ(nullptr, other_client.begin_frame_source());

// 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.begin_frame_source());

+ EXPECT_EQ(nullptr, other_client.begin_frame_source());

manager_.UnregisterBeginFrameSource(&source);

- EXPECT_EQ(nullptr, client.source());

- EXPECT_EQ(nullptr, other_client.source());

+ EXPECT_EQ(nullptr, client.begin_frame_source());

+ EXPECT_EQ(nullptr, other_client.begin_frame_source());

// 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.begin_frame_source());

+ EXPECT_EQ(nullptr, client.begin_frame_source());

manager_.UnregisterBeginFrameSource(&source);

- EXPECT_EQ(nullptr, client.source());

- EXPECT_EQ(nullptr, other_client.source());

+ EXPECT_EQ(nullptr, client.begin_frame_source());

+ EXPECT_EQ(nullptr, other_client.begin_frame_source());

// Re-set BFS for original

manager_.RegisterBeginFrameSource(&source, client.frame_sink_id());

- EXPECT_EQ(&source, client.source());

+ EXPECT_EQ(&source, client.begin_frame_source());

manager_.UnregisterBeginFrameSource(&source);

- EXPECT_EQ(nullptr, client.source());

+ EXPECT_EQ(nullptr, client.begin_frame_source());

}

TEST_F(SurfaceManagerTest, MultipleDisplays) {

@@ -120,27 +81,37 @@ TEST_F(SurfaceManagerTest, MultipleDisplays) {

// 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),

+ is_root, handles_frame_sink_id_invalidation,

+ needs_sync_points);

+ CompositorFrameSinkSupport root2(nullptr, &manager_, FrameSinkId(2, 2),

+ is_root, handles_frame_sink_id_invalidation,

+ needs_sync_points);

+ CompositorFrameSinkSupport client_a(

+ nullptr, &manager_, FrameSinkId(3, 3), is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

+ CompositorFrameSinkSupport client_b(

+ nullptr, &manager_, FrameSinkId(4, 4), is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

+ CompositorFrameSinkSupport client_c(

+ nullptr, &manager_, FrameSinkId(5, 5), is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

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.begin_frame_source(), &root1_source);

+ EXPECT_EQ(root2.begin_frame_source(), &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.begin_frame_source(), root1.begin_frame_source());

manager_.RegisterFrameSinkHierarchy(client_a.frame_sink_id(),

client_b.frame_sink_id());

- EXPECT_EQ(client_b.source(), root1.source());

+ EXPECT_EQ(client_b.begin_frame_source(), root1.begin_frame_source());

manager_.RegisterFrameSinkHierarchy(root2.frame_sink_id(),

client_c.frame_sink_id());

- EXPECT_EQ(client_c.source(), root2.source());

+ EXPECT_EQ(client_c.begin_frame_source(), root2.begin_frame_source());

// Attach A into root2's subtree, like a window moving across displays.

// root1 -> A -> B

@@ -149,28 +120,28 @@ TEST_F(SurfaceManagerTest, MultipleDisplays) {

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.begin_frame_source(), root1.begin_frame_source());

+ EXPECT_EQ(client_b.begin_frame_source(), root1.begin_frame_source());

+ EXPECT_EQ(client_c.begin_frame_source(), root2.begin_frame_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(client_a.source(), root2.source());

- EXPECT_EQ(client_b.source(), root2.source());

- EXPECT_EQ(client_c.source(), root2.source());

+ EXPECT_EQ(client_a.begin_frame_source(), root2.begin_frame_source());

+ EXPECT_EQ(client_b.begin_frame_source(), root2.begin_frame_source());

+ EXPECT_EQ(client_c.begin_frame_source(), root2.begin_frame_source());

// 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.begin_frame_source());

+ EXPECT_NE(nullptr, root2.begin_frame_source());

// 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.begin_frame_source());

+ EXPECT_EQ(nullptr, client_b.begin_frame_source());

+ EXPECT_EQ(nullptr, client_c.begin_frame_source());

+ EXPECT_EQ(nullptr, root2.begin_frame_source());

// Cleanup hierarchy.

manager_.UnregisterFrameSinkHierarchy(root2.frame_sink_id(),

@@ -192,30 +163,36 @@ TEST_F(SurfaceManagerTest, ParentWithoutClientRetained) {

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, is_root,

+ handles_frame_sink_id_invalidation,

+ needs_sync_points);

+ CompositorFrameSinkSupport client_b(

+ nullptr, &manager_, kFrameSinkIdB, is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

+ CompositorFrameSinkSupport client_c(

+ nullptr, &manager_, kFrameSinkIdC, is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

manager_.RegisterBeginFrameSource(&root_source, root.frame_sink_id());

- EXPECT_EQ(&root_source, root.source());

+ EXPECT_EQ(&root_source, root.begin_frame_source());

// 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.begin_frame_source(), client_b.begin_frame_source());

// 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.begin_frame_source());

manager_.RegisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdC);

// The root's BeginFrameSource should propagate to C.

- EXPECT_EQ(root.source(), client_c.source());

+ EXPECT_EQ(root.begin_frame_source(), client_c.begin_frame_source());

manager_.UnregisterBeginFrameSource(&root_source);

- EXPECT_EQ(nullptr, root.source());

- EXPECT_EQ(nullptr, client_c.source());

+ EXPECT_EQ(nullptr, root.begin_frame_source());

+ EXPECT_EQ(nullptr, client_c.begin_frame_source());

}

// This test sets up the same hierarchy as ParentWithoutClientRetained.

@@ -231,9 +208,15 @@ TEST_F(SurfaceManagerTest,

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, is_root,

+ handles_frame_sink_id_invalidation,

+ needs_sync_points);

+ CompositorFrameSinkSupport client_b(

+ nullptr, &manager_, kFrameSinkIdB, is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

+ CompositorFrameSinkSupport client_c(

+ nullptr, &manager_, kFrameSinkIdC, is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

// Set up initial hierarchy: root -> A -> B.

// Note that A does not have a SurfaceFactoryClient.

@@ -241,7 +224,7 @@ TEST_F(SurfaceManagerTest,

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.begin_frame_source());

// Unregister B, and attach C to A: root -> A -> C

manager_.UnregisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdB);

@@ -250,12 +233,12 @@ TEST_F(SurfaceManagerTest,

// 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.begin_frame_source());

+ EXPECT_EQ(root.begin_frame_source(), client_c.begin_frame_source());

manager_.UnregisterBeginFrameSource(&root_source);

- EXPECT_EQ(nullptr, root.source());

- EXPECT_EQ(nullptr, client_c.source());

+ EXPECT_EQ(nullptr, root.begin_frame_source());

+ EXPECT_EQ(nullptr, client_c.begin_frame_source());

}

// In practice, registering and unregistering both parent/child relationships

@@ -267,10 +250,7 @@ TEST_F(SurfaceManagerTest,

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();

@@ -286,44 +266,46 @@ class SurfaceManagerOrderingTest : public SurfaceManagerTest {

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(frame_sink_id_a_, frame_sink_id_b_);

+ manager_.RegisterFrameSinkHierarchy(frame_sink_id_b_, frame_sink_id_c_);

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(frame_sink_id_a_, frame_sink_id_b_);

+ manager_.UnregisterFrameSinkHierarchy(frame_sink_id_b_, frame_sink_id_c_);

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_, frame_sink_id_a_, is_root,

+ handles_frame_sink_id_invalidation, needs_sync_points);

+ client_b_ = base::MakeUnique<CompositorFrameSinkSupport>(

+ nullptr, &manager_, frame_sink_id_b_, is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

+ client_c_ = base::MakeUnique<CompositorFrameSinkSupport>(

+ nullptr, &manager_, frame_sink_id_c_, is_root_child,

+ handles_frame_sink_id_invalidation, needs_sync_points);

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_, frame_sink_id_a_);

AssertCorrectBFSState();

}

void UnregisterBFS() {

@@ -334,15 +316,18 @@ class SurfaceManagerOrderingTest : public SurfaceManagerTest {

}

void AssertEmptyBFS() {

- EXPECT_EQ(nullptr, client_a_.source());

- EXPECT_EQ(nullptr, client_b_.source());

- EXPECT_EQ(nullptr, client_c_.source());

+ EXPECT_TRUE(client_a_.get() == nullptr ||

+ client_a_->begin_frame_source() == nullptr);

+ EXPECT_TRUE(client_b_.get() == nullptr ||

+ client_b_->begin_frame_source() == nullptr);

+ EXPECT_TRUE(client_c_.get() == nullptr ||

+ client_c_->begin_frame_source() == nullptr);

}

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_->begin_frame_source());

+ EXPECT_EQ(&source_, client_b_->begin_frame_source());

+ EXPECT_EQ(&source_, client_c_->begin_frame_source());

}

protected:

@@ -353,9 +338,11 @@ class SurfaceManagerOrderingTest : public SurfaceManagerTest {

}

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_->begin_frame_source());

+ EXPECT_TRUE(client_b_.get() == nullptr ||

Fady Samuel 2017/04/04 22:48:16 Can this happen?

Alex Z. 2017/04/05 14:38:55 Yes. It happens in SurfaceManagerOrderingTest(),

+ client_b_->begin_frame_source() == nullptr);

+ EXPECT_TRUE(client_c_.get() == nullptr ||

Fady Samuel 2017/04/04 22:48:17 Can this happen?

Alex Z. 2017/04/05 14:38:55 Yes. It happens in SurfaceManagerOrderingTest(),

+ client_c_->begin_frame_source() == nullptr);

return;

}

@@ -364,9 +351,13 @@ class SurfaceManagerOrderingTest : public SurfaceManagerTest {

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_;

+ const FrameSinkId frame_sink_id_a_ = FrameSinkId(1, 1);

+ const FrameSinkId frame_sink_id_b_ = FrameSinkId(2, 2);

+ const FrameSinkId frame_sink_id_c_ = FrameSinkId(3, 3);

Fady Samuel 2017/04/04 22:48:17 Move these to the anonymous namespace.

Alex Z. 2017/04/05 14:38:55 Done.

bool hierarchy_registered_;

bool clients_registered_;

« cc/surfaces/surface_hittest_unittest.cc ('K') | « cc/surfaces/surface_hittest_unittest.cc ('k') | cc/surfaces/surface_unittest.cc » ('j') | no next file with comments »