cc/surfaces/surface_manager_unittest.cc - Issue 2144733005: [WIP] cc: Plumb SurfaceId from clients

Unified Diff: cc/surfaces/surface_manager_unittest.cc

Issue 2144733005: [WIP] cc: Plumb SurfaceId from clients Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Ensure only SurfaceFactoy and tests can update hierarchy Created 4 years, 5 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

Download patch

Index: cc/surfaces/surface_manager_unittest.cc

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

index 9c290944d30b49ae7fe07f9371577dce6db4d91d..ebbf878205787dc473fdd8d948e32c9c151f7d35 100644

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

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

@@ -13,10 +13,10 @@ namespace cc {

class FakeSurfaceFactoryClient : public SurfaceFactoryClient {

public:

- explicit FakeSurfaceFactoryClient(int id_namespace)

- : source_(nullptr), manager_(nullptr), id_namespace_(id_namespace) {}

- FakeSurfaceFactoryClient(int id_namespace, SurfaceManager* manager)

- : source_(nullptr), manager_(nullptr), id_namespace_(id_namespace) {

+ explicit FakeSurfaceFactoryClient(int client_id)

+ : source_(nullptr), manager_(nullptr), client_id_(client_id) {}

+ FakeSurfaceFactoryClient(int client_id, SurfaceManager* manager)

+ : source_(nullptr), manager_(nullptr), client_id_(client_id) {

DCHECK(manager);

}

@@ -29,17 +29,17 @@ class FakeSurfaceFactoryClient : public SurfaceFactoryClient {

}

BeginFrameSource* source() { return source_; }

- uint32_t id_namespace() { return id_namespace_; }

+ uint32_t client_id() { return client_id_; }

void Register(SurfaceManager* manager) {

EXPECT_EQ(manager_, nullptr);

manager_ = manager;

- manager_->RegisterSurfaceFactoryClient(id_namespace_, this);

+ manager_->RegisterClient(client_id_, this);

}

void Unregister() {

EXPECT_NE(manager_, nullptr);

- manager_->UnregisterSurfaceFactoryClient(id_namespace_);

+ manager_->UnregisterClient(client_id_);

manager_ = nullptr;

}

@@ -53,7 +53,7 @@ class FakeSurfaceFactoryClient : public SurfaceFactoryClient {

private:

BeginFrameSource* source_;

SurfaceManager* manager_;

- uint32_t id_namespace_;

+ uint32_t client_id_;

};

class EmptyBeginFrameSource : public BeginFrameSource {

@@ -66,19 +66,10 @@ class EmptyBeginFrameSource : public BeginFrameSource {

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_NAMESPACE = 10 };

SurfaceManagerTest() {

- for (size_t i = 0; i < MAX_NAMESPACE; ++i)

- manager_.RegisterSurfaceClientId(i);

}

~SurfaceManagerTest() override {

- for (size_t i = 0; i < MAX_NAMESPACE; ++i)

- manager_.InvalidateSurfaceClientId(i);

}

protected:

@@ -98,7 +89,7 @@ TEST_F(SurfaceManagerTest, SingleClients) {

EXPECT_EQ(other_client.source(), nullptr);

// Test setting unsetting BFS

- manager_.RegisterBeginFrameSource(&source, client.id_namespace());

+ manager_.RegisterBeginFrameSource(&source, client.client_id());

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

EXPECT_EQ(other_client.source(), nullptr);

manager_.UnregisterBeginFrameSource(&source);

@@ -106,7 +97,7 @@ TEST_F(SurfaceManagerTest, SingleClients) {

EXPECT_EQ(other_client.source(), nullptr);

// Set BFS for other namespace

- manager_.RegisterBeginFrameSource(&source, other_client.id_namespace());

+ manager_.RegisterBeginFrameSource(&source, other_client.client_id());

EXPECT_EQ(other_client.source(), &source);

EXPECT_EQ(client.source(), nullptr);

manager_.UnregisterBeginFrameSource(&source);

@@ -114,7 +105,7 @@ TEST_F(SurfaceManagerTest, SingleClients) {

EXPECT_EQ(other_client.source(), nullptr);

// Re-set BFS for original

- manager_.RegisterBeginFrameSource(&source, client.id_namespace());

+ manager_.RegisterBeginFrameSource(&source, client.client_id());

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

manager_.UnregisterBeginFrameSource(&source);

EXPECT_EQ(client.source(), nullptr);

@@ -132,27 +123,27 @@ TEST_F(SurfaceManagerTest, MultipleDisplays) {

FakeSurfaceFactoryClient client_b(4, &manager_);

FakeSurfaceFactoryClient client_c(5, &manager_);

- manager_.RegisterBeginFrameSource(&root1_source, root1.id_namespace());

- manager_.RegisterBeginFrameSource(&root2_source, root2.id_namespace());

+ manager_.RegisterBeginFrameSource(&root1_source, root1.client_id());

+ manager_.RegisterBeginFrameSource(&root2_source, root2.client_id());

EXPECT_EQ(root1.source(), &root1_source);

EXPECT_EQ(root2.source(), &root2_source);

// Set up initial hierarchy.

- manager_.RegisterSurfaceNamespaceHierarchy(root1.id_namespace(),

- client_a.id_namespace());

+ manager_.RegisterSurfaceNamespaceHierarchy(root1.client_id(),

+ client_a.client_id());

EXPECT_EQ(client_a.source(), root1.source());

- manager_.RegisterSurfaceNamespaceHierarchy(client_a.id_namespace(),

- client_b.id_namespace());

+ manager_.RegisterSurfaceNamespaceHierarchy(client_a.client_id(),

+ client_b.client_id());

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

- manager_.RegisterSurfaceNamespaceHierarchy(root2.id_namespace(),

- client_c.id_namespace());

+ manager_.RegisterSurfaceNamespaceHierarchy(root2.client_id(),

+ client_c.client_id());

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

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

// root1 -> A -> B

// root2 -> C -> A -> B

- manager_.RegisterSurfaceNamespaceHierarchy(client_c.id_namespace(),

- client_a.id_namespace());

+ manager_.RegisterSurfaceNamespaceHierarchy(client_c.client_id(),

+ client_a.client_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());

@@ -160,8 +151,8 @@ TEST_F(SurfaceManagerTest, MultipleDisplays) {

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

// Detach A from root1. A and B should now be updated to root2.

- manager_.UnregisterSurfaceNamespaceHierarchy(root1.id_namespace(),

- client_a.id_namespace());

+ manager_.UnregisterSurfaceNamespaceHierarchy(root1.client_id(),

+ client_a.client_id());

EXPECT_EQ(client_a.source(), root2.source());

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

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

@@ -179,12 +170,12 @@ TEST_F(SurfaceManagerTest, MultipleDisplays) {

EXPECT_EQ(root2.source(), nullptr);

// Cleanup hierarchy.

- manager_.UnregisterSurfaceNamespaceHierarchy(root2.id_namespace(),

- client_c.id_namespace());

- manager_.UnregisterSurfaceNamespaceHierarchy(client_c.id_namespace(),

- client_a.id_namespace());

- manager_.UnregisterSurfaceNamespaceHierarchy(client_a.id_namespace(),

- client_b.id_namespace());

+ manager_.UnregisterSurfaceNamespaceHierarchy(root2.client_id(),

+ client_c.client_id());

+ manager_.UnregisterSurfaceNamespaceHierarchy(client_c.client_id(),

+ client_a.client_id());

+ manager_.UnregisterSurfaceNamespaceHierarchy(client_a.client_id(),

+ client_b.client_id());

}

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

@@ -215,19 +206,19 @@ class SurfaceManagerOrderingTest : public SurfaceManagerTest {

void RegisterHierarchy() {

DCHECK(!hierarchy_registered_);

hierarchy_registered_ = true;

- manager_.RegisterSurfaceNamespaceHierarchy(client_a_.id_namespace(),

- client_b_.id_namespace());

- manager_.RegisterSurfaceNamespaceHierarchy(client_b_.id_namespace(),

- client_c_.id_namespace());

+ manager_.RegisterSurfaceNamespaceHierarchy(client_a_.client_id(),

+ client_b_.client_id());

+ manager_.RegisterSurfaceNamespaceHierarchy(client_b_.client_id(),

+ client_c_.client_id());

AssertCorrectBFSState();

}

void UnregisterHierarchy() {

DCHECK(hierarchy_registered_);

hierarchy_registered_ = false;

- manager_.UnregisterSurfaceNamespaceHierarchy(client_a_.id_namespace(),

- client_b_.id_namespace());

- manager_.UnregisterSurfaceNamespaceHierarchy(client_b_.id_namespace(),

- client_c_.id_namespace());

+ manager_.UnregisterSurfaceNamespaceHierarchy(client_a_.client_id(),

+ client_b_.client_id());

+ manager_.UnregisterSurfaceNamespaceHierarchy(client_b_.client_id(),

+ client_c_.client_id());

AssertCorrectBFSState();

}

@@ -252,7 +243,7 @@ class SurfaceManagerOrderingTest : public SurfaceManagerTest {

void RegisterBFS() {

DCHECK(!bfs_registered_);

bfs_registered_ = true;

- manager_.RegisterBeginFrameSource(&source_, client_a_.id_namespace());

+ manager_.RegisterBeginFrameSource(&source_, client_a_.client_id());

AssertCorrectBFSState();

}

void UnregisterBFS() {

@@ -302,80 +293,51 @@ class SurfaceManagerOrderingTest : public SurfaceManagerTest {

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

+enum BFSOrder { BFS_FIRST, BFS_SECOND };

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

+static const BFSOrder kBFSOrderList[] = {BFS_FIRST, BFS_SECOND};

class SurfaceManagerOrderingParamTest

: public SurfaceManagerOrderingTest,

- public ::testing::WithParamInterface<

- std::tr1::tuple<RegisterOrder, UnregisterOrder, BFSOrder>> {};

+ public ::testing::WithParamInterface<BFSOrder> {};

TEST_P(SurfaceManagerOrderingParamTest, 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

// of every permutation of BFS setting and unsetting.

- // The register/unregister functions themselves test most of the state.

- RegisterOrder register_order = std::tr1::get<0>(GetParam());

- UnregisterOrder unregister_order = std::tr1::get<1>(GetParam());

- BFSOrder bfs_order = std::tr1::get<2>(GetParam());

+ // The register/unregister functions themselves test most of the stat;

+ BFSOrder bfs_order = GetParam();

+ RegisterClients();

// Attach everything up in the specified order.

if (bfs_order == BFS_FIRST)

RegisterBFS();

- if (register_order == REGISTER_HIERARCHY_FIRST)

- RegisterHierarchy();

- else

- RegisterClients();

+ RegisterHierarchy();

if (bfs_order == BFS_SECOND)

RegisterBFS();

- if (register_order == REGISTER_HIERARCHY_FIRST)

- RegisterClients();

- else

- RegisterHierarchy();

- if (bfs_order == BFS_THIRD)

- RegisterBFS();

// Everything hooked up, so should be valid.

AssertAllValidBFS();

// Detach everything in the specified order.

- if (bfs_order == BFS_THIRD)

+ if (bfs_order == BFS_SECOND)

UnregisterBFS();

- if (unregister_order == UNREGISTER_HIERARCHY_FIRST)

UnregisterHierarchy();

- else

- UnregisterClients();

- if (bfs_order == BFS_SECOND)

- UnregisterBFS();

+ if (bfs_order == BFS_FIRST)

+ UnregisterBFS();

- if (unregister_order == UNREGISTER_HIERARCHY_FIRST)

UnregisterClients();

- else

- UnregisterHierarchy();

- if (bfs_order == BFS_FIRST)

- UnregisterBFS();

}

-INSTANTIATE_TEST_CASE_P(

- SurfaceManagerOrderingParamTestInstantiation,

- SurfaceManagerOrderingParamTest,

- ::testing::Combine(::testing::ValuesIn(kRegisterOrderList),

- ::testing::ValuesIn(kUnregisterOrderList),

- ::testing::ValuesIn(kBFSOrderList)));

+INSTANTIATE_TEST_CASE_P(SurfaceManagerOrderingParamTestInstantiation,

+ SurfaceManagerOrderingParamTest,

+ ::testing::ValuesIn(kBFSOrderList));

} // namespace cc

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