Skip to content

Commit

Permalink
Fix flaky test ShellSurfacePresentationTimeRecorderTest.RecorderDestr…
Browse files Browse the repository at this point in the history
…oyedBeforePresent.

Why is the previous code flaky (with AutoNeedsBeginFrame
feature enabled)?
The root_surface submits two frames in this test case. If
both frames are submitted before the first DrawAndSwap, the
test will pass. Because frame 2 activation at the viz side
will discard frame 1, which will send back presentation
feedback for frame 1. And then DrawAndSwap will cause
presentation feedback for frame 2.
On the other hand, if a DrawAndSwap happens between frame 1
and frame 2, it will cause presentation feedback for frame
1. Later frame 2 arrives. Since it has no damage, the next
DrawAndSwap decides that it doesn't need to draw, which also
means it won't trigger presentation feedback. The feedback
could be triggered by the next frame submission, but in this
test there isn't a third frame.

Why is the previous code *not* flaky, if AutoNeedsBeginFrame
is disabled (while ReactiveFrameSubmission enabled)?
That is because without AutoNeedsBeginFrame, there won't be
unsolicited frame submission. Instead, it needs to wait for
BeginFrame to submit. Without running a RunLoop to receive
BeginFrame events, frame 1 will be discarded at the Exo side
directly when frame 2 is submitted.

Bug: 1489140
Change-Id: Ia3c989eb78e4a985c96a908583f0ac2c78a976f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4940009
Commit-Queue: Jonathan Ross <jonross@chromium.org>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1209856}
  • Loading branch information
yzshen authored and Chromium LUCI CQ committed Oct 14, 2023
1 parent 85602dc commit f6c0cfe
Showing 1 changed file with 4 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ class ShellSurfacePresentationTimeRecorderTest : public test::ExoTestBase {
/*flags=*/0);
recorder_->set_fake_feedback(feedback);

// Fake damage to ensure that Commit() generates a compositor frame.
// Fake damage so that the committed frame will generate a presentation
// feedback when the next DrawAndSwap happens. Without damage the
// presentation feedback could be delayed till the next frame submission.
root_surface()->Damage(gfx::Rect(0, 0, 32, 32));
root_surface()->Commit();
recorder_->WaitForFramePresented();
Expand Down Expand Up @@ -184,6 +186,7 @@ TEST_F(ShellSurfacePresentationTimeRecorderTest,

// Fake frame submission. No FakeFrameSubmitAndPresent() because it depends
// on `recorder_`.
root_surface()->Damage(gfx::Rect(0, 0, 32, 32));
root_surface()->Commit();
test::WaitForLastFramePresentation(shell_surface_.get());
}
Expand Down

0 comments on commit f6c0cfe

Please sign in to comment.