Commit 9947cd2d authored by Chris Blume's avatar Chris Blume Committed by Commit Bot

Unit test to mirror actual behavior

The ParentLocalSurfaceIdAllocator unit tests have a function to get a
fake child-allocated LocalSurfaceId. It hard-codes values.

The hard-coded parent sequence number is bad but not terrible. It forces
knowledge of the calling functions. This isn't good. It can be improved.

However, the embed token part is pretty bad. It forces the unit tests to
assert the wrong behavior. A child allocation should use the parent's
embed token. Since the hard-coded value is different, the calling tests
assert that the embed tokens don't match when in reality they should.

To fix these bad behaviors, the fake child allocation should take a
parent allocator (or parent-allocated LocalSurfaceId) so the values can
be based on what the test provides.

BUG=837030

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Iaa8e7b6226c0a6c6d67fa420183f24a36e7499ce
Reviewed-on: https://chromium-review.googlesource.com/1036530Reviewed-by: default avatarFady Samuel <fsamuel@chromium.org>
Commit-Queue: Chris Blume <cblume@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554928}
parent 89c60972
...@@ -24,7 +24,8 @@ namespace { ...@@ -24,7 +24,8 @@ namespace {
const LocalSurfaceId& local_surface_id); const LocalSurfaceId& local_surface_id);
::testing::AssertionResult NonceIsEmpty(const LocalSurfaceId& local_surface_id); ::testing::AssertionResult NonceIsEmpty(const LocalSurfaceId& local_surface_id);
LocalSurfaceId GetFakeChildAllocatedLocalSurfaceId(); LocalSurfaceId GetFakeChildAllocatedLocalSurfaceId(
const ParentLocalSurfaceIdAllocator& parent_allocator);
ParentLocalSurfaceIdAllocator GetChildUpdatedAllocator(); ParentLocalSurfaceIdAllocator GetChildUpdatedAllocator();
} // namespace } // namespace
...@@ -86,14 +87,14 @@ TEST(ParentLocalSurfaceIdAllocatorTest, ...@@ -86,14 +87,14 @@ TEST(ParentLocalSurfaceIdAllocatorTest,
UpdateFromChildOnlyUpdatesExpectedLocalSurfaceIdComponents) { UpdateFromChildOnlyUpdatesExpectedLocalSurfaceIdComponents) {
ParentLocalSurfaceIdAllocator child_updated_parent_allocator; ParentLocalSurfaceIdAllocator child_updated_parent_allocator;
LocalSurfaceId preupdate_local_surface_id = LocalSurfaceId preupdate_local_surface_id =
child_updated_parent_allocator.GetCurrentLocalSurfaceId(); child_updated_parent_allocator.GenerateId();
LocalSurfaceId child_allocated_local_surface_id = LocalSurfaceId child_allocated_local_surface_id =
GetFakeChildAllocatedLocalSurfaceId(); GetFakeChildAllocatedLocalSurfaceId(child_updated_parent_allocator);
EXPECT_NE(preupdate_local_surface_id.parent_sequence_number(), EXPECT_EQ(preupdate_local_surface_id.parent_sequence_number(),
child_allocated_local_surface_id.parent_sequence_number()); child_allocated_local_surface_id.parent_sequence_number());
EXPECT_NE(preupdate_local_surface_id.child_sequence_number(), EXPECT_NE(preupdate_local_surface_id.child_sequence_number(),
child_allocated_local_surface_id.child_sequence_number()); child_allocated_local_surface_id.child_sequence_number());
EXPECT_NE(preupdate_local_surface_id.embed_token(), EXPECT_EQ(preupdate_local_surface_id.embed_token(),
child_allocated_local_surface_id.embed_token()); child_allocated_local_surface_id.embed_token());
const LocalSurfaceId& returned_local_surface_id = const LocalSurfaceId& returned_local_surface_id =
...@@ -102,11 +103,11 @@ TEST(ParentLocalSurfaceIdAllocatorTest, ...@@ -102,11 +103,11 @@ TEST(ParentLocalSurfaceIdAllocatorTest,
const LocalSurfaceId& postupdate_local_surface_id = const LocalSurfaceId& postupdate_local_surface_id =
child_updated_parent_allocator.GetCurrentLocalSurfaceId(); child_updated_parent_allocator.GetCurrentLocalSurfaceId();
EXPECT_NE(postupdate_local_surface_id.parent_sequence_number(), EXPECT_EQ(postupdate_local_surface_id.parent_sequence_number(),
child_allocated_local_surface_id.parent_sequence_number()); child_allocated_local_surface_id.parent_sequence_number());
EXPECT_EQ(postupdate_local_surface_id.child_sequence_number(), EXPECT_EQ(postupdate_local_surface_id.child_sequence_number(),
child_allocated_local_surface_id.child_sequence_number()); child_allocated_local_surface_id.child_sequence_number());
EXPECT_NE(postupdate_local_surface_id.embed_token(), EXPECT_EQ(postupdate_local_surface_id.embed_token(),
child_allocated_local_surface_id.embed_token()); child_allocated_local_surface_id.embed_token());
EXPECT_EQ(returned_local_surface_id, EXPECT_EQ(returned_local_surface_id,
child_updated_parent_allocator.GetCurrentLocalSurfaceId()); child_updated_parent_allocator.GetCurrentLocalSurfaceId());
...@@ -193,19 +194,20 @@ namespace { ...@@ -193,19 +194,20 @@ namespace {
return ::testing::AssertionFailure() << "embed_token() is not empty"; return ::testing::AssertionFailure() << "embed_token() is not empty";
} }
LocalSurfaceId GetFakeChildAllocatedLocalSurfaceId() { LocalSurfaceId GetFakeChildAllocatedLocalSurfaceId(
constexpr uint32_t kParentSequenceNumber = 1; const ParentLocalSurfaceIdAllocator& parent_allocator) {
constexpr uint32_t kChildSequenceNumber = 3; const LocalSurfaceId& current_local_surface_id =
const base::UnguessableToken embed_token = base::UnguessableToken::Create(); parent_allocator.GetCurrentLocalSurfaceId();
return LocalSurfaceId(kParentSequenceNumber, kChildSequenceNumber, return LocalSurfaceId(current_local_surface_id.parent_sequence_number(),
embed_token); current_local_surface_id.child_sequence_number() + 1,
current_local_surface_id.embed_token());
} }
ParentLocalSurfaceIdAllocator GetChildUpdatedAllocator() { ParentLocalSurfaceIdAllocator GetChildUpdatedAllocator() {
ParentLocalSurfaceIdAllocator child_updated_parent_allocator; ParentLocalSurfaceIdAllocator child_updated_parent_allocator;
LocalSurfaceId child_allocated_local_surface_id = LocalSurfaceId child_allocated_local_surface_id =
GetFakeChildAllocatedLocalSurfaceId(); GetFakeChildAllocatedLocalSurfaceId(child_updated_parent_allocator);
child_updated_parent_allocator.UpdateFromChild( child_updated_parent_allocator.UpdateFromChild(
child_allocated_local_surface_id); child_allocated_local_surface_id);
return child_updated_parent_allocator; return child_updated_parent_allocator;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment