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

Unified Diff: cc/scheduler/begin_frame_source_unittest.cc

Issue 2888043004: [cc] Add and plumb CFS::DidNotProduceFrame. (Closed)
Patch Set: remove ExternalBFS::OnDidFinishFrame and related ack tracking. Created 3 years, 7 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
Index: cc/scheduler/begin_frame_source_unittest.cc
diff --git a/cc/scheduler/begin_frame_source_unittest.cc b/cc/scheduler/begin_frame_source_unittest.cc
index a15c4be4ebb05d85d5a2d4df5c708785d9eb1c53..4cc251df59b06f706d74fff45542e3da9ea52f50 100644
--- a/cc/scheduler/begin_frame_source_unittest.cc
+++ b/cc/scheduler/begin_frame_source_unittest.cc
@@ -550,224 +550,11 @@ TEST_F(DelayBasedBeginFrameSourceTest, DoubleTickMissedFrame) {
source_->RemoveObserver(&obs);
}
-// BeginFrameObserverAckTracker testing ----------------------------------------
-class TestBeginFrameConsumer : public BeginFrameObserverBase {
- private:
- bool OnBeginFrameDerivedImpl(const BeginFrameArgs& args) override {
- // Consume the args.
- return true;
- }
- void OnBeginFrameSourcePausedChanged(bool paused) override {}
-};
-
-// Use EXPECT_TRUE instead of EXPECT_EQ for |finished| and |damage| as gcc 4.7
-// issues the following warning on EXPECT_EQ(false, x), which is turned into an
-// error with -Werror=conversion-null:
-//
-// converting 'false' to pointer type for argument 1 of
-// 'char testing::internal::IsNullLiteralHelper(testing::internal::Secret*)'
-#define EXPECT_ACK_TRACKER_STATE(finished, damage, latest_confirmed) \
- EXPECT_TRUE(finished == tracker_->AllObserversFinishedFrame()) \
- << "expected: " << finished; \
- EXPECT_TRUE(damage == tracker_->AnyObserversHadDamage()) << "expected: " \
- << damage; \
- EXPECT_EQ(latest_confirmed, tracker_->LatestConfirmedSequenceNumber())
-
-class BeginFrameObserverAckTrackerTest : public ::testing::Test {
- public:
- BeginFrameArgs current_args_;
- std::unique_ptr<BeginFrameObserverAckTracker> tracker_;
- TestBeginFrameConsumer obs1_;
- TestBeginFrameConsumer obs2_;
-
- void SetUp() override {
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1);
- tracker_.reset(new BeginFrameObserverAckTracker());
- }
-};
-
-TEST_F(BeginFrameObserverAckTrackerTest, CorrectnessWithoutObservers) {
- // Check initial state.
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // A new BeginFrame is immediately finished and confirmed.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 2);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(true, false, 2u);
-}
-
-TEST_F(BeginFrameObserverAckTrackerTest, CorrectnessWith1Observer) {
- // Check initial state.
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // After adding an observer, the BeginFrame is not finished or confirmed.
- tracker_->OnObserverAdded(&obs1_);
- EXPECT_ACK_TRACKER_STATE(false, false, 0u); // up to date to previous frame.
-
- // On removing it, the BeginFrame is back to original state.
- tracker_->OnObserverRemoved(&obs1_);
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // After adding it back, the BeginFrame is again not finished or confirmed.
- tracker_->OnObserverAdded(&obs1_);
- EXPECT_ACK_TRACKER_STATE(false, false, 0u); // up to date to previous frame.
-
- // When the observer finishes and confirms, the BeginFrame is finished
- // and confirmed.
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 1, 1, false));
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // A new BeginFrame is initially not finished or confirmed.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 2);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(false, false, 1u);
-
- // Stray ACK for an old BeginFrame is ignored.
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 1, 1, false));
- EXPECT_ACK_TRACKER_STATE(false, false, 1u);
-
- // When the observer finishes but doesn't confirm, the BeginFrame is finished
- // but not confirmed.
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 2, 1, false));
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // Damage from ACK propagates.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 3);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(false, false, 1u);
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 3, 3, true));
- EXPECT_ACK_TRACKER_STATE(true, true, 3u);
-
- // Removing an out-of-date observer confirms the latest BeginFrame.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 4);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(false, false, 3u);
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 4, 3, false));
- EXPECT_ACK_TRACKER_STATE(true, false, 3u);
- tracker_->OnObserverRemoved(&obs1_);
- EXPECT_ACK_TRACKER_STATE(true, false, 4u);
-}
-
-TEST_F(BeginFrameObserverAckTrackerTest, CorrectnessWith2Observers) {
- // Check initial state.
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // After adding observers, the BeginFrame is not finished or confirmed.
- tracker_->OnObserverAdded(&obs1_);
- EXPECT_ACK_TRACKER_STATE(false, false, 0u); // up to date to previous frame.
- tracker_->OnObserverAdded(&obs2_);
- EXPECT_ACK_TRACKER_STATE(false, false, 0u); // up to date to previous frame.
-
- // Removing one of them changes nothing. Same for adding back.
- tracker_->OnObserverRemoved(&obs1_);
- EXPECT_ACK_TRACKER_STATE(false, false, 0u);
- tracker_->OnObserverAdded(&obs1_);
- EXPECT_ACK_TRACKER_STATE(false, false, 0u);
-
- // When one observer finishes and confirms, nothing changes.
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 1, 1, false));
- EXPECT_ACK_TRACKER_STATE(false, false, 0u);
- // When both finish and confirm, the BeginFrame is finished and confirmed.
- obs2_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs2_, BeginFrameAck(0, 1, 1, false));
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // A new BeginFrame is not finished or confirmed.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 2);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(false, false, 1u);
-
- // When both observers finish but only one confirms, the BeginFrame is
- // finished but not confirmed.
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 2, 2, false));
- EXPECT_ACK_TRACKER_STATE(false, false, 1u);
- obs2_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs2_, BeginFrameAck(0, 2, 1, false));
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // With reversed confirmations in the next ACKs, the latest confirmed frame
- // increases but the latest BeginFrame remains unconfirmed.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 3);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(false, false, 1u);
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 3, 2, false));
- EXPECT_ACK_TRACKER_STATE(false, false, 1u);
- obs2_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs2_, BeginFrameAck(0, 3, 3, false));
- EXPECT_ACK_TRACKER_STATE(true, false, 2u);
-
- // Only a single ACK with damage suffices.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 4);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(false, false, 2u);
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 4, 4, true));
- EXPECT_ACK_TRACKER_STATE(false, true, 3u);
- obs2_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs2_, BeginFrameAck(0, 4, 4, false));
- EXPECT_ACK_TRACKER_STATE(true, true, 4u);
-
- // Removing the damaging observer makes no difference in this case.
- tracker_->OnObserverRemoved(&obs1_);
- EXPECT_ACK_TRACKER_STATE(true, true, 4u);
-
- // Adding the observer back considers it up to date up to the current
- // BeginFrame, because it is the last used one. Thus, the current BeginFrame
- // is still finished, too.
- tracker_->OnObserverAdded(&obs1_);
- EXPECT_ACK_TRACKER_STATE(true, true, 4u);
-
- // Adding the observer back after the next BeginFrame considers it up to date
- // up to last BeginFrame only.
- tracker_->OnObserverRemoved(&obs1_);
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 5);
- tracker_->OnBeginFrame(current_args_);
- tracker_->OnObserverAdded(&obs1_);
- EXPECT_ACK_TRACKER_STATE(false, false, 4u);
- // Both observers need to finish for the BeginFrame to be finished.
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(0, 5, 5, false));
- EXPECT_ACK_TRACKER_STATE(false, false, 4u);
- obs2_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs2_, BeginFrameAck(0, 5, 5, false));
- EXPECT_ACK_TRACKER_STATE(true, false, 5u);
-}
-
-TEST_F(BeginFrameObserverAckTrackerTest, ChangingSourceIdOnBeginFrame) {
- // Check initial state.
- EXPECT_ACK_TRACKER_STATE(true, false, 1u);
-
- // Changing source id without observer updates confirmed BeginFrame.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 1, 10);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(true, false, 10u);
-
- // Setup an observer for current BeginFrame.
- tracker_->OnObserverAdded(&obs1_);
- EXPECT_ACK_TRACKER_STATE(false, false, 9u); // up to date to previous frame.
- obs1_.OnBeginFrame(current_args_);
- tracker_->OnObserverFinishedFrame(&obs1_, BeginFrameAck(1, 10, 10, true));
- EXPECT_ACK_TRACKER_STATE(true, true, 10u);
-
- // Changing source id with an observer sets confirmed BeginFrame to invalid.
- current_args_ = CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 2, 20);
- tracker_->OnBeginFrame(current_args_);
- EXPECT_ACK_TRACKER_STATE(false, false, BeginFrameArgs::kInvalidFrameNumber);
-}
-
// ExternalBeginFrameSource testing --------------------------------------------
class MockExternalBeginFrameSourceClient
: public ExternalBeginFrameSourceClient {
public:
MOCK_METHOD1(OnNeedsBeginFrames, void(bool));
- MOCK_METHOD1(OnDidFinishFrame, void(const BeginFrameAck&));
};
class ExternalBeginFrameSourceTest : public ::testing::Test {
@@ -788,69 +575,6 @@ class ExternalBeginFrameSourceTest : public ::testing::Test {
}
};
-TEST_F(ExternalBeginFrameSourceTest, CallsOnDidFinishFrameWithoutObservers) {
- EXPECT_CALL((*client_), OnDidFinishFrame(BeginFrameAck(0, 2, 2, false)))
- .Times(1);
- source_->OnBeginFrame(CreateBeginFrameArgsForTesting(
- BEGINFRAME_FROM_HERE, 0, 2, base::TimeTicks::FromInternalValue(10000)));
-}
-
-TEST_F(ExternalBeginFrameSourceTest,
- CallsOnDidFinishFrameWhenObserverFinishes) {
- EXPECT_BEGIN_FRAME_SOURCE_PAUSED(*obs_, false);
- EXPECT_CALL((*client_), OnNeedsBeginFrames(true)).Times(1);
- source_->AddObserver(obs_.get());
-
- BeginFrameArgs args = CreateBeginFrameArgsForTesting(
- BEGINFRAME_FROM_HERE, 0, 2, base::TimeTicks::FromInternalValue(10000));
- EXPECT_BEGIN_FRAME_ARGS_USED(*obs_, args);
- source_->OnBeginFrame(args);
-
- EXPECT_CALL((*client_), OnDidFinishFrame(BeginFrameAck(0, 2, 2, true)))
- .Times(1);
- source_->DidFinishFrame(obs_.get(), BeginFrameAck(0, 2, 2, true));
-
- args = CreateBeginFrameArgsForTesting(
- BEGINFRAME_FROM_HERE, 0, 3, base::TimeTicks::FromInternalValue(20000));
- EXPECT_BEGIN_FRAME_ARGS_USED(*obs_, args);
- source_->OnBeginFrame(args);
-
- EXPECT_CALL((*client_), OnDidFinishFrame(BeginFrameAck(0, 3, 2, false)))
- .Times(1);
- source_->DidFinishFrame(obs_.get(), BeginFrameAck(0, 3, 2, false));
-}
-
-TEST_F(ExternalBeginFrameSourceTest,
- CallsOnDidFinishFrameWhenObserverDropsBeginFrame) {
- EXPECT_BEGIN_FRAME_SOURCE_PAUSED(*obs_, false);
- EXPECT_CALL((*client_), OnNeedsBeginFrames(true)).Times(1);
- source_->AddObserver(obs_.get());
-
- BeginFrameArgs args = CreateBeginFrameArgsForTesting(
- BEGINFRAME_FROM_HERE, 0, 2, base::TimeTicks::FromInternalValue(10000));
- EXPECT_BEGIN_FRAME_ARGS_DROP(*obs_, args);
- source_->OnBeginFrame(args);
- EXPECT_CALL((*client_), OnDidFinishFrame(BeginFrameAck(0, 2, 0, false)))
- .Times(1);
- source_->DidFinishFrame(obs_.get(), BeginFrameAck(0, 2, 0, false));
-}
-
-TEST_F(ExternalBeginFrameSourceTest, CallsOnDidFinishFrameWhenObserverRemoved) {
- EXPECT_BEGIN_FRAME_SOURCE_PAUSED(*obs_, false);
- EXPECT_CALL((*client_), OnNeedsBeginFrames(true)).Times(1);
- source_->AddObserver(obs_.get());
-
- BeginFrameArgs args = CreateBeginFrameArgsForTesting(
- BEGINFRAME_FROM_HERE, 0, 2, base::TimeTicks::FromInternalValue(10000));
- EXPECT_BEGIN_FRAME_ARGS_USED(*obs_, args);
- source_->OnBeginFrame(args);
-
- EXPECT_CALL((*client_), OnDidFinishFrame(BeginFrameAck(0, 2, 2, false)))
- .Times(1);
- EXPECT_CALL((*client_), OnNeedsBeginFrames(false)).Times(1);
- source_->RemoveObserver(obs_.get());
-}
-
// https://crbug.com/690127: Duplicate BeginFrame caused DCHECK crash.
TEST_F(ExternalBeginFrameSourceTest, OnBeginFrameChecksBeginFrameContinuity) {
EXPECT_BEGIN_FRAME_SOURCE_PAUSED(*obs_, false);
@@ -863,8 +587,6 @@ TEST_F(ExternalBeginFrameSourceTest, OnBeginFrameChecksBeginFrameContinuity) {
source_->OnBeginFrame(args);
// Providing same args again to OnBeginFrame() should not notify observer.
- EXPECT_CALL((*client_), OnDidFinishFrame(BeginFrameAck(0, 2, 0, false)))
- .Times(1);
source_->OnBeginFrame(args);
// Providing same args through a different ExternalBeginFrameSource also does

Powered by Google App Engine
This is Rietveld 408576698