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

Unified Diff: cc/surfaces/compositor_frame_sink_support_unittest.cc

Issue 2785103003: [cc] CompositorFrameSinkSupport: Defer BeginFrameAck of pending frames. (Closed)
Patch Set: add comment Created 3 years, 9 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
« no previous file with comments | « cc/surfaces/compositor_frame_sink_support.cc ('k') | cc/surfaces/direct_compositor_frame_sink_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: cc/surfaces/compositor_frame_sink_support_unittest.cc
diff --git a/cc/surfaces/compositor_frame_sink_support_unittest.cc b/cc/surfaces/compositor_frame_sink_support_unittest.cc
index 3ee8a996bedfe690e8b7dcab2a76da52f9f4b5cc..2e17b46b67bd7e7e48e00185dea1c8e71016fa24 100644
--- a/cc/surfaces/compositor_frame_sink_support_unittest.cc
+++ b/cc/surfaces/compositor_frame_sink_support_unittest.cc
@@ -187,11 +187,17 @@ class CompositorFrameSinkSupportTest : public testing::Test {
supports_.push_back(CreateCompositorFrameSinkSupport(kChildFrameSink1));
supports_.push_back(CreateCompositorFrameSinkSupport(kChildFrameSink2));
// Normally, the BeginFrameSource would be registered by the Display. We
- // register it here so that BeginFrames are received by the display support,
- // for use in the PassesOnBeginFrameAcks test. Other supports do not receive
- // BeginFrames, since the frame sink hierarchy is not set up in this test.
+ // register it here so that BeginFrames are received by the supports,
+ // for use in the BeginFrameAck tests.
surface_manager_.RegisterBeginFrameSource(begin_frame_source_.get(),
kDisplayFrameSink);
+ // Setup hierarchy so that other supports receive BeginFrames, too.
+ surface_manager_.RegisterFrameSinkHierarchy(kDisplayFrameSink,
+ kParentFrameSink);
+ surface_manager_.RegisterFrameSinkHierarchy(kParentFrameSink,
+ kChildFrameSink1);
+ surface_manager_.RegisterFrameSinkHierarchy(kParentFrameSink,
+ kChildFrameSink2);
}
void TearDown() override {
@@ -958,6 +964,8 @@ TEST_F(CompositorFrameSinkSupportTest,
}
TEST_F(CompositorFrameSinkSupportTest, PassesOnBeginFrameAcks) {
+ const SurfaceId display_id = MakeSurfaceId(kDisplayFrameSink, 1);
+
// Request BeginFrames.
display_support().SetNeedsBeginFrame(true);
@@ -972,8 +980,132 @@ TEST_F(CompositorFrameSinkSupportTest, PassesOnBeginFrameAcks) {
display_support().BeginFrameDidNotSwap(ack);
EXPECT_EQ(ack, begin_frame_source()->LastAckForObserver(&display_support()));
- // TODO(eseckler): Check that the support forwards the BeginFrameAck attached
+ // Issue another BeginFrame.
+ args = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 2);
+ begin_frame_source()->TestOnBeginFrame(args);
+
+ // Check that the support forwards the BeginFrameAck attached
// to a CompositorFrame to the BeginFrameSource.
+ BeginFrameAck ack2(0, 2, 2, true);
+ CompositorFrame frame = MakeCompositorFrame();
+ frame.metadata.begin_frame_ack = ack2;
+ display_support().SubmitCompositorFrame(display_id.local_surface_id(),
+ std::move(frame));
+ EXPECT_EQ(ack2, begin_frame_source()->LastAckForObserver(&display_support()));
+}
+
+TEST_F(CompositorFrameSinkSupportTest,
+ WaitsToPassOnCompositorFrameBeginFrameAckUntilActivated) {
+ const SurfaceId parent_id1 = MakeSurfaceId(kParentFrameSink, 1);
+ const SurfaceId parent_id2 = MakeSurfaceId(kParentFrameSink, 2);
+ const SurfaceId child_id1 = MakeSurfaceId(kChildFrameSink1, 1);
+ const SurfaceId child_id2 = MakeSurfaceId(kChildFrameSink1, 2);
+
+ // Parent client submits a frame with blocked dependency on child.
+ parent_support().SetNeedsBeginFrame(true);
+ BeginFrameArgs args =
+ CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
+ begin_frame_source()->TestOnBeginFrame(args);
+ CompositorFrame frame = MakeCompositorFrame({child_id1});
+ frame.metadata.begin_frame_ack = BeginFrameAck(0, 1, 1, true);
+ parent_support().SubmitCompositorFrame(parent_id1.local_surface_id(),
+ std::move(frame));
+ // Parent doesn't ack immediately.
+ EXPECT_EQ(BeginFrameAck(),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+
+ // Parent client no longer needs BeginFrames, but parent needs to continue
+ // observing.
+ size_t num_observers = begin_frame_source()->num_observers();
+ parent_support().SetNeedsBeginFrame(false);
+ EXPECT_EQ(num_observers, begin_frame_source()->num_observers());
+
+ // On new BeginFrame, parent acknowledges the past one without updates.
+ args = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 2);
+ begin_frame_source()->TestOnBeginFrame(args);
+ EXPECT_EQ(BeginFrameAck(0, 1, 0, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+
+ // Same again. As the client doesn't have updates (isn't requesting
+ // BeginFrames), the pending ack is updated to confirm the last BeginFrame.
+ args = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 3);
+ begin_frame_source()->TestOnBeginFrame(args);
+ EXPECT_EQ(BeginFrameAck(0, 2, 0, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+ EXPECT_EQ(2u, parent_support()
+ .begin_frame_ack_for_pending_frame_for_testing()
+ ->latest_confirmed_sequence_number);
+
+ // Concurrent DidNotSwapAck from parent client updates pending ack.
+ parent_support().SetNeedsBeginFrame(true);
+ args = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 4);
+ begin_frame_source()->TestOnBeginFrame(args);
+ // Parent acknowledges prior frame.
+ EXPECT_EQ(BeginFrameAck(0, 3, 0, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+ parent_support().BeginFrameDidNotSwap(BeginFrameAck(0, 4, 4, false));
+ // Parent doesn't ack immediately.
+ EXPECT_EQ(BeginFrameAck(0, 3, 0, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+ EXPECT_EQ(4u, parent_support()
+ .begin_frame_ack_for_pending_frame_for_testing()
+ ->latest_confirmed_sequence_number);
+
+ // When child client unblocks parent, parent sends pending ack.
+ child_support1().SubmitCompositorFrame(child_id1.local_surface_id(),
+ MakeCompositorFrame());
+ EXPECT_EQ(BeginFrameAck(0, 4, 4, true),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+
+ // Parent client submits another frame with blocked dependency on child.
+ args = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 5);
+ begin_frame_source()->TestOnBeginFrame(args);
+ frame = MakeCompositorFrame({child_id2});
+ frame.metadata.begin_frame_ack = BeginFrameAck(0, 5, 5, true);
+ parent_support().SubmitCompositorFrame(parent_id1.local_surface_id(),
+ std::move(frame));
+ // Parent doesn't ack immediately.
+ EXPECT_EQ(BeginFrameAck(0, 4, 4, true),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+
+ // Submission of another CompositorFrame to parent updates pending ack.
+ args = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 6);
+ begin_frame_source()->TestOnBeginFrame(args);
+ // Parent acknowledges prior frame.
+ EXPECT_EQ(BeginFrameAck(0, 5, 4, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+ frame = MakeCompositorFrame({child_id2});
+ frame.metadata.begin_frame_ack = BeginFrameAck(0, 6, 6, true);
+ parent_support().SubmitCompositorFrame(parent_id1.local_surface_id(),
+ std::move(frame));
+ // Parent doesn't ack immediately.
+ EXPECT_EQ(BeginFrameAck(0, 5, 4, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+ EXPECT_EQ(6u, parent_support()
+ .begin_frame_ack_for_pending_frame_for_testing()
+ ->latest_confirmed_sequence_number);
+
+ // Submission of a CompositorFrame to different surface updates pending ack.
+ args = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 7);
+ begin_frame_source()->TestOnBeginFrame(args);
+ // Parent acknowledges prior frame.
+ EXPECT_EQ(BeginFrameAck(0, 6, 4, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+ frame = MakeCompositorFrame({child_id2});
+ frame.metadata.begin_frame_ack = BeginFrameAck(0, 7, 7, true);
+ parent_support().SubmitCompositorFrame(parent_id2.local_surface_id(),
+ std::move(frame));
+ // Parent doesn't ack immediately.
+ EXPECT_EQ(BeginFrameAck(0, 6, 4, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
+ EXPECT_EQ(7u, parent_support()
+ .begin_frame_ack_for_pending_frame_for_testing()
+ ->latest_confirmed_sequence_number);
+
+ // Parent acks immediately but without damage when frame is dropped.
+ parent_support().EvictFrame();
+ EXPECT_EQ(BeginFrameAck(0, 7, 4, false),
+ begin_frame_source()->LastAckForObserver(&parent_support()));
}
// Checks whether the resources are returned before we send an ack.
« no previous file with comments | « cc/surfaces/compositor_frame_sink_support.cc ('k') | cc/surfaces/direct_compositor_frame_sink_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698