Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1292)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: cc/surfaces/surface_manager_unittest.cc

Issue 2795683003: [cc]Replace use of SurfaceFactory with CompositorFrameSinkSupport in tests (Closed)
Patch Set: Remove SurfaceFacotry Usages in SurfaceAggregatorTest And 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
Download patch
« cc/surfaces/compositor_frame_sink_support.cc ('K') | « cc/surfaces/surface_hittest_unittest.cc ('k') | cc/surfaces/surface_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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..018aa089de2072b252ab2711e7812b278bf60783 100644
--- a/cc/surfaces/surface_manager_unittest.cc
+++ b/cc/surfaces/surface_manager_unittest.cc
@@ -5,58 +5,22 @@
#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;
+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);
- 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 +44,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.BeginFrameSourceForTesting());
+ 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) {
@@ -120,27 +84,40 @@ 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.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
@@ -149,28 +126,34 @@ 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.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(),
@@ -192,30 +175,38 @@ 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.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.
@@ -231,9 +222,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 +238,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.BeginFrameSourceForTesting());
// Unregister B, and attach C to A: root -> A -> C
manager_.UnregisterFrameSinkHierarchy(kFrameSinkIdA, kFrameSinkIdB);
@@ -250,12 +247,13 @@ 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.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
@@ -267,10 +265,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 +281,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 +331,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_->BeginFrameSourceForTesting() == nullptr);
+ EXPECT_TRUE(client_b_.get() == nullptr ||
+ client_b_->BeginFrameSourceForTesting() == nullptr);
+ EXPECT_TRUE(client_c_.get() == nullptr ||
+ client_c_->BeginFrameSourceForTesting() == 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_->BeginFrameSourceForTesting());
+ EXPECT_EQ(&source_, client_b_->BeginFrameSourceForTesting());
+ EXPECT_EQ(&source_, client_c_->BeginFrameSourceForTesting());
}
protected:
@@ -353,9 +353,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_->BeginFrameSourceForTesting());
+ EXPECT_TRUE(client_b_.get() == nullptr ||
+ client_b_->BeginFrameSourceForTesting() == nullptr);
+ EXPECT_TRUE(client_c_.get() == nullptr ||
+ client_c_->BeginFrameSourceForTesting() == nullptr);
return;
}
@@ -364,9 +366,9 @@ 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_;
bool hierarchy_registered_;
bool clients_registered_;
« cc/surfaces/compositor_frame_sink_support.cc ('K') | « cc/surfaces/surface_hittest_unittest.cc ('k') | cc/surfaces/surface_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698