Skip to content

Commit 05f38d1

Browse files
ddiproiettoddiproietto
committed
FrameTraceTest: Account for extra perfetto trace packet
Due to a bug in perfetto, the last trace packet was always lost. The test worked around that by making sure that there always was a meaningless packet at the end, often created with traceTimestamp(... UNSPECIFIED). Now the bug in perfetto is being fixed. We cannot simply remove traceTimestamp(... UNSPECIFIED) everywhere because often that also generated useful packets. This CL fixes the problem by updating the expectations to consider this last packet as well. Also, some EXPECT are converted to ASSERT, to prevent accessing invalid memory if the tests fail. Bug: 162206162 Change-Id: Ibf7b83bd3953ab575ea6ebc350123cea830fecd2
1 parent c3ad2e3 commit 05f38d1

1 file changed

Lines changed: 127 additions & 37 deletions

File tree

‎services/surfaceflinger/tests/unittests/FrameTracerTest.cpp‎

Lines changed: 127 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -179,23 +179,35 @@ TEST_F(FrameTracerTest, canTraceAfterAddingLayer) {
179179
tracingSession->StopBlocking();
180180

181181
auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
182-
EXPECT_EQ(packets.size(), 1);
183-
184-
const auto& packet = packets[0];
185-
ASSERT_TRUE(packet.has_timestamp());
186-
EXPECT_EQ(packet.timestamp(), timestamp);
187-
ASSERT_TRUE(packet.has_graphics_frame_event());
188-
const auto& frame_event = packet.graphics_frame_event();
189-
ASSERT_TRUE(frame_event.has_buffer_event());
190-
const auto& buffer_event = frame_event.buffer_event();
191-
ASSERT_TRUE(buffer_event.has_buffer_id());
192-
EXPECT_EQ(buffer_event.buffer_id(), bufferID);
193-
ASSERT_TRUE(buffer_event.has_frame_number());
194-
EXPECT_EQ(buffer_event.frame_number(), frameNumber);
195-
ASSERT_TRUE(buffer_event.has_type());
196-
EXPECT_EQ(buffer_event.type(), perfetto::protos::GraphicsFrameEvent_BufferEventType(type));
197-
ASSERT_TRUE(buffer_event.has_duration_ns());
198-
EXPECT_EQ(buffer_event.duration_ns(), duration);
182+
ASSERT_EQ(packets.size(), 2);
183+
184+
const auto& packet1 = packets[0];
185+
ASSERT_TRUE(packet1.has_timestamp());
186+
EXPECT_EQ(packet1.timestamp(), timestamp);
187+
ASSERT_TRUE(packet1.has_graphics_frame_event());
188+
const auto& frame_event1 = packet1.graphics_frame_event();
189+
ASSERT_TRUE(frame_event1.has_buffer_event());
190+
const auto& buffer_event1 = frame_event1.buffer_event();
191+
ASSERT_TRUE(buffer_event1.has_buffer_id());
192+
EXPECT_EQ(buffer_event1.buffer_id(), bufferID);
193+
ASSERT_TRUE(buffer_event1.has_frame_number());
194+
EXPECT_EQ(buffer_event1.frame_number(), frameNumber);
195+
ASSERT_TRUE(buffer_event1.has_type());
196+
EXPECT_EQ(buffer_event1.type(), perfetto::protos::GraphicsFrameEvent_BufferEventType(type));
197+
ASSERT_TRUE(buffer_event1.has_duration_ns());
198+
EXPECT_EQ(buffer_event1.duration_ns(), duration);
199+
200+
const auto& packet2 = packets[1];
201+
ASSERT_TRUE(packet2.has_timestamp());
202+
EXPECT_EQ(packet2.timestamp(), 0);
203+
ASSERT_TRUE(packet2.has_graphics_frame_event());
204+
const auto& frame_event2 = packet2.graphics_frame_event();
205+
ASSERT_TRUE(frame_event2.has_buffer_event());
206+
const auto& buffer_event2 = frame_event2.buffer_event();
207+
ASSERT_TRUE(buffer_event2.has_type());
208+
EXPECT_EQ(buffer_event2.type(),
209+
perfetto::protos::GraphicsFrameEvent_BufferEventType(
210+
FrameTracer::FrameEvent::UNSPECIFIED));
199211
}
200212
}
201213

@@ -219,7 +231,18 @@ TEST_F(FrameTracerTest, traceFenceTriggersOnNextTraceAfterFenceFired) {
219231
tracingSession->StopBlocking();
220232

221233
auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
222-
EXPECT_EQ(packets.size(), 0);
234+
ASSERT_EQ(packets.size(), 1);
235+
const auto& packet = packets[0];
236+
ASSERT_TRUE(packet.has_timestamp());
237+
EXPECT_EQ(packet.timestamp(), 0);
238+
ASSERT_TRUE(packet.has_graphics_frame_event());
239+
const auto& frame_event = packet.graphics_frame_event();
240+
ASSERT_TRUE(frame_event.has_buffer_event());
241+
const auto& buffer_event = frame_event.buffer_event();
242+
ASSERT_TRUE(buffer_event.has_type());
243+
EXPECT_EQ(buffer_event.type(),
244+
perfetto::protos::GraphicsFrameEvent_BufferEventType(
245+
FrameTracer::FrameEvent::UNSPECIFIED));
223246
}
224247

225248
{
@@ -235,22 +258,56 @@ TEST_F(FrameTracerTest, traceFenceTriggersOnNextTraceAfterFenceFired) {
235258
tracingSession->StopBlocking();
236259

237260
auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
238-
EXPECT_EQ(packets.size(), 2); // Two packets because of the extra trace made above.
239-
240-
const auto& packet = packets[1];
241-
ASSERT_TRUE(packet.has_timestamp());
242-
EXPECT_EQ(packet.timestamp(), timestamp);
243-
ASSERT_TRUE(packet.has_graphics_frame_event());
244-
const auto& frame_event = packet.graphics_frame_event();
245-
ASSERT_TRUE(frame_event.has_buffer_event());
246-
const auto& buffer_event = frame_event.buffer_event();
247-
ASSERT_TRUE(buffer_event.has_buffer_id());
248-
EXPECT_EQ(buffer_event.buffer_id(), bufferID);
249-
ASSERT_TRUE(buffer_event.has_frame_number());
250-
EXPECT_EQ(buffer_event.frame_number(), frameNumber);
251-
ASSERT_TRUE(buffer_event.has_type());
252-
EXPECT_EQ(buffer_event.type(), perfetto::protos::GraphicsFrameEvent_BufferEventType(type));
253-
EXPECT_FALSE(buffer_event.has_duration_ns());
261+
ASSERT_EQ(packets.size(), 3);
262+
263+
const auto& packet1 = packets[0];
264+
ASSERT_TRUE(packet1.has_timestamp());
265+
EXPECT_EQ(packet1.timestamp(), timestamp);
266+
ASSERT_TRUE(packet1.has_graphics_frame_event());
267+
const auto& frame_event1 = packet1.graphics_frame_event();
268+
ASSERT_TRUE(frame_event1.has_buffer_event());
269+
const auto& buffer_event1 = frame_event1.buffer_event();
270+
ASSERT_TRUE(buffer_event1.has_buffer_id());
271+
EXPECT_EQ(buffer_event1.buffer_id(), bufferID);
272+
ASSERT_TRUE(buffer_event1.has_frame_number());
273+
EXPECT_EQ(buffer_event1.frame_number(), frameNumber);
274+
ASSERT_TRUE(buffer_event1.has_type());
275+
EXPECT_EQ(buffer_event1.type(),
276+
perfetto::protos::GraphicsFrameEvent::BufferEventType(type));
277+
EXPECT_FALSE(buffer_event1.has_duration_ns());
278+
279+
const auto& packet2 = packets[1];
280+
ASSERT_TRUE(packet2.has_timestamp());
281+
EXPECT_EQ(packet2.timestamp(), timestamp);
282+
ASSERT_TRUE(packet2.has_graphics_frame_event());
283+
const auto& frame_event2 = packet2.graphics_frame_event();
284+
ASSERT_TRUE(frame_event2.has_buffer_event());
285+
const auto& buffer_event2 = frame_event2.buffer_event();
286+
ASSERT_TRUE(buffer_event2.has_buffer_id());
287+
EXPECT_EQ(buffer_event2.buffer_id(), bufferID);
288+
ASSERT_TRUE(buffer_event2.has_frame_number());
289+
EXPECT_EQ(buffer_event2.frame_number(), frameNumber);
290+
ASSERT_TRUE(buffer_event2.has_type());
291+
EXPECT_EQ(buffer_event2.type(),
292+
perfetto::protos::GraphicsFrameEvent::BufferEventType(type));
293+
EXPECT_FALSE(buffer_event2.has_duration_ns());
294+
295+
const auto& packet3 = packets[2];
296+
ASSERT_TRUE(packet3.has_timestamp());
297+
EXPECT_EQ(packet3.timestamp(), 0);
298+
ASSERT_TRUE(packet3.has_graphics_frame_event());
299+
const auto& frame_event3 = packet3.graphics_frame_event();
300+
ASSERT_TRUE(frame_event3.has_buffer_event());
301+
const auto& buffer_event3 = frame_event3.buffer_event();
302+
ASSERT_TRUE(buffer_event3.has_buffer_id());
303+
EXPECT_EQ(buffer_event3.buffer_id(), bufferID);
304+
ASSERT_TRUE(buffer_event3.has_frame_number());
305+
EXPECT_EQ(buffer_event3.frame_number(), 0);
306+
ASSERT_TRUE(buffer_event3.has_type());
307+
EXPECT_EQ(buffer_event3.type(),
308+
perfetto::protos::GraphicsFrameEvent::BufferEventType(
309+
FrameTracer::FrameEvent::UNSPECIFIED));
310+
EXPECT_FALSE(buffer_event3.has_duration_ns());
254311
}
255312
}
256313

@@ -285,21 +342,34 @@ TEST_F(FrameTracerTest, traceFenceWithStartTimeAfterSignalTime_ShouldHaveNoDurat
285342
tracingSession->StopBlocking();
286343

287344
auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
288-
EXPECT_EQ(packets.size(), 2);
345+
ASSERT_EQ(packets.size(), 3);
289346

290347
const auto& packet1 = packets[0];
291348
ASSERT_TRUE(packet1.has_timestamp());
292349
EXPECT_EQ(packet1.timestamp(), signalTime1);
293350
ASSERT_TRUE(packet1.has_graphics_frame_event());
294351
ASSERT_TRUE(packet1.graphics_frame_event().has_buffer_event());
295352
ASSERT_FALSE(packet1.graphics_frame_event().buffer_event().has_duration_ns());
353+
EXPECT_EQ(packet1.graphics_frame_event().buffer_event().type(),
354+
perfetto::protos::GraphicsFrameEvent::BufferEventType(type));
296355

297356
const auto& packet2 = packets[1];
298357
ASSERT_TRUE(packet2.has_timestamp());
299358
EXPECT_EQ(packet2.timestamp(), signalTime2);
300359
ASSERT_TRUE(packet2.has_graphics_frame_event());
301360
ASSERT_TRUE(packet2.graphics_frame_event().has_buffer_event());
302361
ASSERT_FALSE(packet2.graphics_frame_event().buffer_event().has_duration_ns());
362+
EXPECT_EQ(packet2.graphics_frame_event().buffer_event().type(),
363+
perfetto::protos::GraphicsFrameEvent::BufferEventType(type));
364+
365+
const auto& packet3 = packets[2];
366+
ASSERT_TRUE(packet3.has_timestamp());
367+
EXPECT_EQ(packet3.timestamp(), 0);
368+
ASSERT_TRUE(packet3.has_graphics_frame_event());
369+
ASSERT_TRUE(packet3.graphics_frame_event().has_buffer_event());
370+
EXPECT_EQ(packet3.graphics_frame_event().buffer_event().type(),
371+
perfetto::protos::GraphicsFrameEvent::BufferEventType(
372+
FrameTracer::FrameEvent::UNSPECIFIED));
303373
}
304374

305375
TEST_F(FrameTracerTest, traceFenceOlderThanDeadline_ShouldBeIgnored) {
@@ -322,7 +392,15 @@ TEST_F(FrameTracerTest, traceFenceOlderThanDeadline_ShouldBeIgnored) {
322392
tracingSession->StopBlocking();
323393

324394
auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
325-
EXPECT_EQ(packets.size(), 0);
395+
ASSERT_EQ(packets.size(), 1);
396+
const auto& packet = packets[0];
397+
ASSERT_TRUE(packet.has_timestamp());
398+
EXPECT_EQ(packet.timestamp(), 0);
399+
ASSERT_TRUE(packet.has_graphics_frame_event());
400+
ASSERT_TRUE(packet.graphics_frame_event().has_buffer_event());
401+
EXPECT_EQ(packet.graphics_frame_event().buffer_event().type(),
402+
perfetto::protos::GraphicsFrameEvent::BufferEventType(
403+
FrameTracer::FrameEvent::UNSPECIFIED));
326404
}
327405

328406
TEST_F(FrameTracerTest, traceFenceWithValidStartTime_ShouldHaveCorrectDuration) {
@@ -357,7 +435,7 @@ TEST_F(FrameTracerTest, traceFenceWithValidStartTime_ShouldHaveCorrectDuration)
357435
tracingSession->StopBlocking();
358436

359437
auto packets = readGraphicsFramePacketsBlocking(tracingSession.get());
360-
EXPECT_EQ(packets.size(), 2);
438+
ASSERT_EQ(packets.size(), 3);
361439

362440
const auto& packet1 = packets[0];
363441
ASSERT_TRUE(packet1.has_timestamp());
@@ -367,6 +445,7 @@ TEST_F(FrameTracerTest, traceFenceWithValidStartTime_ShouldHaveCorrectDuration)
367445
ASSERT_TRUE(packet1.graphics_frame_event().buffer_event().has_duration_ns());
368446
const auto& buffer_event1 = packet1.graphics_frame_event().buffer_event();
369447
EXPECT_EQ(buffer_event1.duration_ns(), duration);
448+
EXPECT_EQ(buffer_event1.type(), perfetto::protos::GraphicsFrameEvent::BufferEventType(type));
370449

371450
const auto& packet2 = packets[1];
372451
ASSERT_TRUE(packet2.has_timestamp());
@@ -376,6 +455,17 @@ TEST_F(FrameTracerTest, traceFenceWithValidStartTime_ShouldHaveCorrectDuration)
376455
ASSERT_TRUE(packet2.graphics_frame_event().buffer_event().has_duration_ns());
377456
const auto& buffer_event2 = packet2.graphics_frame_event().buffer_event();
378457
EXPECT_EQ(buffer_event2.duration_ns(), duration);
458+
EXPECT_EQ(buffer_event2.type(), perfetto::protos::GraphicsFrameEvent::BufferEventType(type));
459+
460+
const auto& packet3 = packets[2];
461+
ASSERT_TRUE(packet3.has_timestamp());
462+
EXPECT_EQ(packet3.timestamp(), 0);
463+
ASSERT_TRUE(packet3.has_graphics_frame_event());
464+
ASSERT_TRUE(packet3.graphics_frame_event().has_buffer_event());
465+
const auto& buffer_event3 = packet3.graphics_frame_event().buffer_event();
466+
EXPECT_EQ(buffer_event3.type(),
467+
perfetto::protos::GraphicsFrameEvent::BufferEventType(
468+
FrameTracer::FrameEvent::UNSPECIFIED));
379469
}
380470

381471
} // namespace

0 commit comments

Comments
 (0)