diff --git a/cc/ipc/mojo_compositor_frame_sink.mojom b/cc/ipc/mojo_compositor_frame_sink.mojom index 7bb993226520f..e4c0ae3ef542d 100644 --- a/cc/ipc/mojo_compositor_frame_sink.mojom +++ b/cc/ipc/mojo_compositor_frame_sink.mojom @@ -37,7 +37,7 @@ interface MojoCompositorFrameSink { // Notifies the frame sink that a BeginFrame was completed, but that no // CompositorFrame was produced as a result of it. - BeginFrameDidNotSwap(cc.mojom.BeginFrameAck ack); + DidNotProduceFrame(cc.mojom.BeginFrameAck ack); // Notify that the surface is no longer in use (and is okay to be evicted) so // that its resources gets returned in time. diff --git a/cc/output/compositor_frame_sink.h b/cc/output/compositor_frame_sink.h index f8b7a47b4e6c9..28b72581b5e47 100644 --- a/cc/output/compositor_frame_sink.h +++ b/cc/output/compositor_frame_sink.h @@ -25,6 +25,7 @@ class GpuMemoryBufferManager; namespace cc { +struct BeginFrameAck; class CompositorFrame; class CompositorFrameSinkClient; class LocalSurfaceId; @@ -121,6 +122,10 @@ class CC_EXPORT CompositorFrameSink { // processed in order to unthrottle the next frame. virtual void SubmitCompositorFrame(CompositorFrame frame) = 0; + // Signals that a BeginFrame issued by the BeginFrameSource provided to the + // client did not lead to a CompositorFrame submission. + virtual void DidNotProduceFrame(const BeginFrameAck& ack) = 0; + protected: // Bound to the ContextProvider to hear about when it is lost and inform the // |client_|. diff --git a/cc/output/compositor_frame_sink_unittest.cc b/cc/output/compositor_frame_sink_unittest.cc index 6a72c8d58bbe2..127eb43f9c936 100644 --- a/cc/output/compositor_frame_sink_unittest.cc +++ b/cc/output/compositor_frame_sink_unittest.cc @@ -28,6 +28,7 @@ class TestCompositorFrameSink : public CompositorFrameSink { void SubmitCompositorFrame(CompositorFrame frame) override { client_->DidReceiveCompositorFrameAck(); } + void DidNotProduceFrame(const BeginFrameAck& ack) override {} }; TEST(CompositorFrameSinkTest, ContextLossInformsClient) { diff --git a/cc/scheduler/begin_frame_source.cc b/cc/scheduler/begin_frame_source.cc index 5af7b4cd14a8d..be6394e617a88 100644 --- a/cc/scheduler/begin_frame_source.cc +++ b/cc/scheduler/begin_frame_source.cc @@ -252,112 +252,6 @@ void DelayBasedBeginFrameSource::OnTimerTick() { } } -// BeginFrameObserverAckTracker ------------------------------------------- -BeginFrameObserverAckTracker::BeginFrameObserverAckTracker() = default; - -BeginFrameObserverAckTracker::~BeginFrameObserverAckTracker() = default; - -void BeginFrameObserverAckTracker::OnBeginFrame(const BeginFrameArgs& args) { - if (current_source_id_ != args.source_id) - SourceChanged(args); - - DCHECK_GE(args.sequence_number, current_sequence_number_); - // Reset for new BeginFrame. - current_sequence_number_ = args.sequence_number; - finished_observers_.clear(); - observers_had_damage_ = false; -} - -void BeginFrameObserverAckTracker::SourceChanged(const BeginFrameArgs& args) { - current_source_id_ = args.source_id; - current_sequence_number_ = args.sequence_number; - - // Mark all observers invalid: We report an invalid frame until every observer - // has confirmed the frame. - for (auto& entry : latest_confirmed_sequence_numbers_) - entry.second = BeginFrameArgs::kInvalidFrameNumber; -} - -void BeginFrameObserverAckTracker::OnObserverFinishedFrame( - BeginFrameObserver* obs, - const BeginFrameAck& ack) { - if (ack.source_id != current_source_id_) - return; - - DCHECK_LE(ack.sequence_number, current_sequence_number_); - if (ack.sequence_number != current_sequence_number_) - return; - - finished_observers_.insert(obs); - observers_had_damage_ |= ack.has_damage; - - // We max() with the current value in |latest_confirmed_sequence_numbers_| to - // handle situations where an observer just started observing (again) and may - // acknowledge with an ancient latest_confirmed_sequence_number. - latest_confirmed_sequence_numbers_[obs] = - std::max(ack.latest_confirmed_sequence_number, - latest_confirmed_sequence_numbers_[obs]); -} - -void BeginFrameObserverAckTracker::OnObserverAdded(BeginFrameObserver* obs) { - observers_.insert(obs); - - // Since the observer didn't want BeginFrames before, we consider it - // up-to-date up to the last BeginFrame, except if it already handled the - // current BeginFrame. In which case, we consider it up-to-date up to the - // current one. - DCHECK_LT(BeginFrameArgs::kInvalidFrameNumber, current_sequence_number_); - const BeginFrameArgs& last_args = obs->LastUsedBeginFrameArgs(); - if (last_args.IsValid() && - last_args.sequence_number == current_sequence_number_ && - last_args.source_id == current_source_id_) { - latest_confirmed_sequence_numbers_[obs] = current_sequence_number_; - finished_observers_.insert(obs); - } else { - latest_confirmed_sequence_numbers_[obs] = current_sequence_number_ - 1; - } -} - -void BeginFrameObserverAckTracker::OnObserverRemoved(BeginFrameObserver* obs) { - observers_.erase(obs); - finished_observers_.erase(obs); - latest_confirmed_sequence_numbers_.erase(obs); -} - -bool BeginFrameObserverAckTracker::AllObserversFinishedFrame() const { - if (finished_observers_.size() < observers_.size()) - return false; - return base::STLIncludes(finished_observers_, observers_); -} - -bool BeginFrameObserverAckTracker::AnyObserversHadDamage() const { - return observers_had_damage_; -} - -uint64_t BeginFrameObserverAckTracker::LatestConfirmedSequenceNumber() const { - uint64_t latest_confirmed_sequence_number = current_sequence_number_; - for (const auto& entry : latest_confirmed_sequence_numbers_) { - latest_confirmed_sequence_number = - std::min(latest_confirmed_sequence_number, entry.second); - } - return latest_confirmed_sequence_number; -} - -void BeginFrameObserverAckTracker::AsValueInto( - base::trace_event::TracedValue* state) const { - state->SetInteger("current_source_id", current_source_id_); - state->SetInteger("current_sequence_number", current_sequence_number_); - state->SetInteger("num_observers", observers_.size()); - state->SetInteger("num_finished_observers", finished_observers_.size()); - state->SetBoolean("observers_had_damage", observers_had_damage_); - - state->BeginArray("latest_confirmed_sequence_numbers"); - for (const auto& kv : latest_confirmed_sequence_numbers_) { - state->AppendInteger(kv.second); - } - state->EndArray(); -} - // ExternalBeginFrameSource ----------------------------------------------- ExternalBeginFrameSource::ExternalBeginFrameSource( ExternalBeginFrameSourceClient* client) @@ -372,16 +266,11 @@ void ExternalBeginFrameSource::AsValueInto( BeginFrameSource::AsValueInto(state); state->SetBoolean("paused", paused_); - state->SetBoolean("frame_active", frame_active_); state->SetInteger("num_observers", observers_.size()); state->BeginDictionary("last_begin_frame_args"); last_begin_frame_args_.AsValueInto(state); state->EndDictionary(); - - state->BeginDictionary("ack_tracker_state"); - ack_tracker_.AsValueInto(state); - state->EndDictionary(); } void ExternalBeginFrameSource::AddObserver(BeginFrameObserver* obs) { @@ -390,7 +279,6 @@ void ExternalBeginFrameSource::AddObserver(BeginFrameObserver* obs) { bool observers_was_empty = observers_.empty(); observers_.insert(obs); - ack_tracker_.OnObserverAdded(obs); obs->OnBeginFrameSourcePausedChanged(paused_); if (observers_was_empty) client_->OnNeedsBeginFrames(true); @@ -417,8 +305,6 @@ void ExternalBeginFrameSource::RemoveObserver(BeginFrameObserver* obs) { DCHECK(observers_.find(obs) != observers_.end()); observers_.erase(obs); - ack_tracker_.OnObserverRemoved(obs); - MaybeFinishFrame(); if (observers_.empty()) { last_begin_frame_args_ = BeginFrameArgs(); client_->OnNeedsBeginFrames(false); @@ -426,10 +312,7 @@ void ExternalBeginFrameSource::RemoveObserver(BeginFrameObserver* obs) { } void ExternalBeginFrameSource::DidFinishFrame(BeginFrameObserver* obs, - const BeginFrameAck& ack) { - ack_tracker_.OnObserverFinishedFrame(obs, ack); - MaybeFinishFrame(); -} + const BeginFrameAck& ack) {} bool ExternalBeginFrameSource::IsThrottled() const { return true; @@ -445,12 +328,7 @@ void ExternalBeginFrameSource::OnSetBeginFrameSourcePaused(bool paused) { } void ExternalBeginFrameSource::OnBeginFrame(const BeginFrameArgs& args) { - if (frame_active_) - FinishFrame(); - - frame_active_ = true; last_begin_frame_args_ = args; - ack_tracker_.OnBeginFrame(args); std::unordered_set<BeginFrameObserver*> observers(observers_); for (auto* obs : observers) { // It is possible that the source in which |args| originate changes, or that @@ -465,23 +343,6 @@ void ExternalBeginFrameSource::OnBeginFrame(const BeginFrameArgs& args) { obs->OnBeginFrame(args); } } - MaybeFinishFrame(); -} - -void ExternalBeginFrameSource::MaybeFinishFrame() { - if (!frame_active_ || !ack_tracker_.AllObserversFinishedFrame()) - return; - FinishFrame(); -} - -void ExternalBeginFrameSource::FinishFrame() { - frame_active_ = false; - - BeginFrameAck ack(last_begin_frame_args_.source_id, - last_begin_frame_args_.sequence_number, - ack_tracker_.LatestConfirmedSequenceNumber(), - ack_tracker_.AnyObserversHadDamage()); - client_->OnDidFinishFrame(ack); } } // namespace cc diff --git a/cc/scheduler/begin_frame_source.h b/cc/scheduler/begin_frame_source.h index f9c47c5522dc7..98ea3771b3979 100644 --- a/cc/scheduler/begin_frame_source.h +++ b/cc/scheduler/begin_frame_source.h @@ -247,54 +247,10 @@ class CC_EXPORT DelayBasedBeginFrameSource : public SyntheticBeginFrameSource, DISALLOW_COPY_AND_ASSIGN(DelayBasedBeginFrameSource); }; -// Helper class that tracks outstanding acknowledgments from -// BeginFrameObservers. -class CC_EXPORT BeginFrameObserverAckTracker { - public: - BeginFrameObserverAckTracker(); - ~BeginFrameObserverAckTracker(); - - // The BeginFrameSource uses these methods to notify us when a BeginFrame was - // started, an observer finished a frame, or an observer was added/removed. - void OnBeginFrame(const BeginFrameArgs& args); - void OnObserverFinishedFrame(BeginFrameObserver* obs, - const BeginFrameAck& ack); - void OnObserverAdded(BeginFrameObserver* obs); - void OnObserverRemoved(BeginFrameObserver* obs); - - // Returns |true| if all the source's observers completed the current frame. - bool AllObserversFinishedFrame() const; - - // Returns |true| if any observer had damage during the current frame. - bool AnyObserversHadDamage() const; - - // Return the sequence number of the latest frame that all active observers - // have confirmed. - uint64_t LatestConfirmedSequenceNumber() const; - - void AsValueInto(base::trace_event::TracedValue* state) const; - - private: - void SourceChanged(const BeginFrameArgs& args); - - uint32_t current_source_id_ = 0; - uint64_t current_sequence_number_ = BeginFrameArgs::kStartingFrameNumber; - // Small sets, but order matters for intersection computation. - base::flat_set<BeginFrameObserver*> observers_; - base::flat_set<BeginFrameObserver*> finished_observers_; - bool observers_had_damage_ = false; - base::flat_map<BeginFrameObserver*, uint64_t> - latest_confirmed_sequence_numbers_; - - DISALLOW_COPY_AND_ASSIGN(BeginFrameObserverAckTracker); -}; - class CC_EXPORT ExternalBeginFrameSourceClient { public: // Only called when changed. Assumed false by default. virtual void OnNeedsBeginFrames(bool needs_begin_frames) = 0; - // Called when all observers have completed a frame. - virtual void OnDidFinishFrame(const BeginFrameAck& ack) = 0; }; // A BeginFrameSource that is only ticked manually. Usually the endpoint @@ -319,15 +275,10 @@ class CC_EXPORT ExternalBeginFrameSource : public BeginFrameSource { void OnBeginFrame(const BeginFrameArgs& args); protected: - void MaybeFinishFrame(); - void FinishFrame(); - BeginFrameArgs last_begin_frame_args_; std::unordered_set<BeginFrameObserver*> observers_; ExternalBeginFrameSourceClient* client_; bool paused_ = false; - bool frame_active_ = false; - BeginFrameObserverAckTracker ack_tracker_; private: DISALLOW_COPY_AND_ASSIGN(ExternalBeginFrameSource); diff --git a/cc/scheduler/begin_frame_source_unittest.cc b/cc/scheduler/begin_frame_source_unittest.cc index a15c4be4ebb05..4cc251df59b06 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 diff --git a/cc/scheduler/scheduler.cc b/cc/scheduler/scheduler.cc index cbbbe7f546db0..e3fdef521d816 100644 --- a/cc/scheduler/scheduler.cc +++ b/cc/scheduler/scheduler.cc @@ -462,6 +462,8 @@ void Scheduler::SendBeginFrameAck(const BeginFrameArgs& args, BeginFrameAck ack(args.source_id, args.sequence_number, latest_confirmed_sequence_number, did_submit); begin_frame_source_->DidFinishFrame(this, ack); + if (!did_submit) + client_->DidNotProduceFrame(ack); } // BeginImplFrame starts a compositor frame that will wait up until a deadline diff --git a/cc/scheduler/scheduler.h b/cc/scheduler/scheduler.h index a03f8ac08091f..6c9ccc4fd7172 100644 --- a/cc/scheduler/scheduler.h +++ b/cc/scheduler/scheduler.h @@ -47,6 +47,7 @@ class SchedulerClient { virtual void ScheduledActionInvalidateCompositorFrameSink() = 0; virtual void ScheduledActionPerformImplSideInvalidation() = 0; virtual void DidFinishImplFrame() = 0; + virtual void DidNotProduceFrame(const BeginFrameAck& ack) = 0; virtual void SendBeginMainFrameNotExpectedSoon() = 0; virtual void ScheduledActionBeginMainFrameNotExpectedUntil( base::TimeTicks time) = 0; diff --git a/cc/scheduler/scheduler_unittest.cc b/cc/scheduler/scheduler_unittest.cc index c920854083cde..1049445fa7f0a 100644 --- a/cc/scheduler/scheduler_unittest.cc +++ b/cc/scheduler/scheduler_unittest.cc @@ -123,6 +123,7 @@ class FakeSchedulerClient : public SchedulerClient, EXPECT_TRUE(inside_begin_impl_frame_); inside_begin_impl_frame_ = false; } + void DidNotProduceFrame(const BeginFrameAck& ack) override {} void ScheduledActionSendBeginMainFrame(const BeginFrameArgs& args) override { PushAction("ScheduledActionSendBeginMainFrame"); diff --git a/cc/surfaces/compositor_frame_sink_support.cc b/cc/surfaces/compositor_frame_sink_support.cc index 0fdb6c6855755..896536f039cf4 100644 --- a/cc/surfaces/compositor_frame_sink_support.cc +++ b/cc/surfaces/compositor_frame_sink_support.cc @@ -85,8 +85,7 @@ void CompositorFrameSinkSupport::SetNeedsBeginFrame(bool needs_begin_frame) { UpdateNeedsBeginFramesInternal(); } -void CompositorFrameSinkSupport::BeginFrameDidNotSwap( - const BeginFrameAck& ack) { +void CompositorFrameSinkSupport::DidNotProduceFrame(const BeginFrameAck& ack) { // TODO(eseckler): While a pending CompositorFrame exists (see TODO below), we // should not acknowledge immediately. Instead, we should update the ack that // will be sent to DisplayScheduler when the pending frame is activated. diff --git a/cc/surfaces/compositor_frame_sink_support.h b/cc/surfaces/compositor_frame_sink_support.h index 5eda4c0feb63e..f3440e1b369d9 100644 --- a/cc/surfaces/compositor_frame_sink_support.h +++ b/cc/surfaces/compositor_frame_sink_support.h @@ -59,7 +59,7 @@ class CC_SURFACES_EXPORT CompositorFrameSinkSupport void EvictCurrentSurface(); void SetNeedsBeginFrame(bool needs_begin_frame); - void BeginFrameDidNotSwap(const BeginFrameAck& ack); + void DidNotProduceFrame(const BeginFrameAck& ack); void SubmitCompositorFrame(const LocalSurfaceId& local_surface_id, CompositorFrame frame); void RequestCopyOfSurface(std::unique_ptr<CopyOutputRequest> request); diff --git a/cc/surfaces/direct_compositor_frame_sink.cc b/cc/surfaces/direct_compositor_frame_sink.cc index 505d38ad0914a..f214463c5e3e8 100644 --- a/cc/surfaces/direct_compositor_frame_sink.cc +++ b/cc/surfaces/direct_compositor_frame_sink.cc @@ -110,6 +110,12 @@ void DirectCompositorFrameSink::SubmitCompositorFrame(CompositorFrame frame) { std::move(frame)); } +void DirectCompositorFrameSink::DidNotProduceFrame(const BeginFrameAck& ack) { + DCHECK(!ack.has_damage); + DCHECK_LE(BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); + support_->DidNotProduceFrame(ack); +} + void DirectCompositorFrameSink::DisplayOutputSurfaceLost() { is_lost_ = true; client_->DidLoseCompositorFrameSink(); @@ -152,10 +158,4 @@ void DirectCompositorFrameSink::OnNeedsBeginFrames(bool needs_begin_frame) { support_->SetNeedsBeginFrame(needs_begin_frame); } -void DirectCompositorFrameSink::OnDidFinishFrame(const BeginFrameAck& ack) { - // If there was damage, SubmitCompositorFrame includes the ack. - if (!ack.has_damage) - support_->BeginFrameDidNotSwap(ack); -} - } // namespace cc diff --git a/cc/surfaces/direct_compositor_frame_sink.h b/cc/surfaces/direct_compositor_frame_sink.h index 06ac4c3bb15eb..e1a4e2373c530 100644 --- a/cc/surfaces/direct_compositor_frame_sink.h +++ b/cc/surfaces/direct_compositor_frame_sink.h @@ -49,6 +49,7 @@ class CC_SURFACES_EXPORT DirectCompositorFrameSink bool BindToClient(CompositorFrameSinkClient* client) override; void DetachFromClient() override; void SubmitCompositorFrame(CompositorFrame frame) override; + void DidNotProduceFrame(const BeginFrameAck& ack) override; // DisplayClient implementation. void DisplayOutputSurfaceLost() override; @@ -70,7 +71,6 @@ class CC_SURFACES_EXPORT DirectCompositorFrameSink // ExternalBeginFrameSourceClient implementation: void OnNeedsBeginFrames(bool needs_begin_frame) override; - void OnDidFinishFrame(const BeginFrameAck& ack) override; // This class is only meant to be used on a single thread. base::ThreadChecker thread_checker_; diff --git a/cc/surfaces/direct_compositor_frame_sink_unittest.cc b/cc/surfaces/direct_compositor_frame_sink_unittest.cc index bfb38757dc93f..2610fc2598a2f 100644 --- a/cc/surfaces/direct_compositor_frame_sink_unittest.cc +++ b/cc/surfaces/direct_compositor_frame_sink_unittest.cc @@ -202,6 +202,7 @@ TEST_F(DirectCompositorFrameSinkTest, AcknowledgesBeginFramesWithoutDamage) { observer.ack().sequence_number); compositor_frame_sink_client_.begin_frame_source()->DidFinishFrame( &observer, observer.ack()); + compositor_frame_sink_->DidNotProduceFrame(observer.ack()); compositor_frame_sink_client_.begin_frame_source()->RemoveObserver(&observer); // Verify that the frame sink acknowledged the last BeginFrame. diff --git a/cc/surfaces/surface_synchronization_unittest.cc b/cc/surfaces/surface_synchronization_unittest.cc index 473e5876f2e33..ae7d748770d41 100644 --- a/cc/surfaces/surface_synchronization_unittest.cc +++ b/cc/surfaces/surface_synchronization_unittest.cc @@ -986,10 +986,10 @@ TEST_F(SurfaceSynchronizationTest, PassesOnBeginFrameAcks) { CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 0, 1); begin_frame_source()->TestOnBeginFrame(args); - // Check that the support forwards a BeginFrameDidNotSwap ack to the + // Check that the support forwards a DidNotProduceFrame ack to the // BeginFrameSource. BeginFrameAck ack(0, 1, 1, false); - display_support().BeginFrameDidNotSwap(ack); + display_support().DidNotProduceFrame(ack); EXPECT_EQ(ack, begin_frame_source()->LastAckForObserver(&display_support())); // Issue another BeginFrame. diff --git a/cc/test/fake_compositor_frame_sink.cc b/cc/test/fake_compositor_frame_sink.cc index 81746a72b8cd4..ebc68428c957d 100644 --- a/cc/test/fake_compositor_frame_sink.cc +++ b/cc/test/fake_compositor_frame_sink.cc @@ -64,6 +64,8 @@ void FakeCompositorFrameSink::SubmitCompositorFrame(CompositorFrame frame) { weak_ptr_factory_.GetWeakPtr())); } +void FakeCompositorFrameSink::DidNotProduceFrame(const BeginFrameAck& ack) {} + void FakeCompositorFrameSink::DidReceiveCompositorFrameAck() { client_->DidReceiveCompositorFrameAck(); } diff --git a/cc/test/fake_compositor_frame_sink.h b/cc/test/fake_compositor_frame_sink.h index 4fe6a88901e35..49b4abe6daeda 100644 --- a/cc/test/fake_compositor_frame_sink.h +++ b/cc/test/fake_compositor_frame_sink.h @@ -63,6 +63,7 @@ class FakeCompositorFrameSink : public CompositorFrameSink { // CompositorFrameSink implementation. void SubmitCompositorFrame(CompositorFrame frame) override; + void DidNotProduceFrame(const BeginFrameAck& ack) override; bool BindToClient(CompositorFrameSinkClient* client) override; void DetachFromClient() override; diff --git a/cc/test/test_compositor_frame_sink.cc b/cc/test/test_compositor_frame_sink.cc index f9d4f46e6c491..32d4a816c7471 100644 --- a/cc/test/test_compositor_frame_sink.cc +++ b/cc/test/test_compositor_frame_sink.cc @@ -127,6 +127,9 @@ void TestCompositorFrameSink::SetLocalSurfaceId( } void TestCompositorFrameSink::SubmitCompositorFrame(CompositorFrame frame) { + DCHECK(frame.metadata.begin_frame_ack.has_damage); + DCHECK_LE(BeginFrameArgs::kStartingFrameNumber, + frame.metadata.begin_frame_ack.sequence_number); test_client_->DisplayReceivedCompositorFrame(frame); if (!delegated_local_surface_id_.is_valid()) { @@ -157,6 +160,12 @@ void TestCompositorFrameSink::SubmitCompositorFrame(CompositorFrame frame) { } } +void TestCompositorFrameSink::DidNotProduceFrame(const BeginFrameAck& ack) { + DCHECK(!ack.has_damage); + DCHECK_LE(BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); + support_->DidNotProduceFrame(ack); +} + void TestCompositorFrameSink::DidReceiveCompositorFrameAck( const ReturnedResourceArray& resources) { ReclaimResources(resources); @@ -198,8 +207,6 @@ void TestCompositorFrameSink::OnNeedsBeginFrames(bool needs_begin_frames) { support_->SetNeedsBeginFrame(needs_begin_frames); } -void TestCompositorFrameSink::OnDidFinishFrame(const BeginFrameAck& ack) {} - void TestCompositorFrameSink::SendCompositorFrameAckToClient() { client_->DidReceiveCompositorFrameAck(); } diff --git a/cc/test/test_compositor_frame_sink.h b/cc/test/test_compositor_frame_sink.h index cbab0121c1397..b26ee117dea91 100644 --- a/cc/test/test_compositor_frame_sink.h +++ b/cc/test/test_compositor_frame_sink.h @@ -79,6 +79,7 @@ class TestCompositorFrameSink : public CompositorFrameSink, void DetachFromClient() override; void SetLocalSurfaceId(const LocalSurfaceId& local_surface_id) override; void SubmitCompositorFrame(CompositorFrame frame) override; + void DidNotProduceFrame(const BeginFrameAck& ack) override; // CompositorFrameSinkSupportClient implementation. void DidReceiveCompositorFrameAck( @@ -97,7 +98,6 @@ class TestCompositorFrameSink : public CompositorFrameSink, private: // ExternalBeginFrameSource implementation. void OnNeedsBeginFrames(bool needs_begin_frames) override; - void OnDidFinishFrame(const BeginFrameAck& ack) override; void SendCompositorFrameAckToClient(); diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc index 54dc7bbd336bc..896c994a1cc1b 100644 --- a/cc/trees/layer_tree_host_impl.cc +++ b/cc/trees/layer_tree_host_impl.cc @@ -1939,6 +1939,11 @@ void LayerTreeHostImpl::DidFinishImplFrame() { decoded_image_tracker_.NotifyFrameFinished(); } +void LayerTreeHostImpl::DidNotProduceFrame(const BeginFrameAck& ack) { + if (compositor_frame_sink_) + compositor_frame_sink_->DidNotProduceFrame(ack); +} + void LayerTreeHostImpl::UpdateViewportContainerSizes() { LayerImpl* inner_container = active_tree_->InnerViewportContainerLayer(); LayerImpl* outer_container = active_tree_->OuterViewportContainerLayer(); diff --git a/cc/trees/layer_tree_host_impl.h b/cc/trees/layer_tree_host_impl.h index 55fea0d13099c..b8553cb323b23 100644 --- a/cc/trees/layer_tree_host_impl.h +++ b/cc/trees/layer_tree_host_impl.h @@ -415,6 +415,7 @@ class CC_EXPORT LayerTreeHostImpl virtual void WillBeginImplFrame(const BeginFrameArgs& args); virtual void DidFinishImplFrame(); + void DidNotProduceFrame(const BeginFrameAck& ack); void DidModifyTilePriorities(); LayerTreeImpl* active_tree() { return active_tree_.get(); } diff --git a/cc/trees/proxy_impl.cc b/cc/trees/proxy_impl.cc index 3d3b4a6539d88..01a96109c89b3 100644 --- a/cc/trees/proxy_impl.cc +++ b/cc/trees/proxy_impl.cc @@ -483,6 +483,11 @@ void ProxyImpl::DidFinishImplFrame() { layer_tree_host_impl_->DidFinishImplFrame(); } +void ProxyImpl::DidNotProduceFrame(const BeginFrameAck& ack) { + DCHECK(IsImplThread()); + layer_tree_host_impl_->DidNotProduceFrame(ack); +} + void ProxyImpl::ScheduledActionSendBeginMainFrame(const BeginFrameArgs& args) { DCHECK(IsImplThread()); unsigned int begin_frame_id = nextBeginFrameId++; diff --git a/cc/trees/proxy_impl.h b/cc/trees/proxy_impl.h index 46071cdd58dc2..0953ae7a0e139 100644 --- a/cc/trees/proxy_impl.h +++ b/cc/trees/proxy_impl.h @@ -98,6 +98,7 @@ class CC_EXPORT ProxyImpl : public NON_EXPORTED_BASE(LayerTreeHostImplClient), // SchedulerClient implementation void WillBeginImplFrame(const BeginFrameArgs& args) override; void DidFinishImplFrame() override; + void DidNotProduceFrame(const BeginFrameAck& ack) override; void ScheduledActionSendBeginMainFrame(const BeginFrameArgs& args) override; DrawResult ScheduledActionDrawIfPossible() override; DrawResult ScheduledActionDrawForced() override; diff --git a/cc/trees/single_thread_proxy.cc b/cc/trees/single_thread_proxy.cc index 777df13fbaf20..1769c19fe60d3 100644 --- a/cc/trees/single_thread_proxy.cc +++ b/cc/trees/single_thread_proxy.cc @@ -807,6 +807,11 @@ void SingleThreadProxy::DidFinishImplFrame() { #endif } +void SingleThreadProxy::DidNotProduceFrame(const BeginFrameAck& ack) { + DebugScopedSetImplThread impl(task_runner_provider_); + layer_tree_host_impl_->DidNotProduceFrame(ack); +} + void SingleThreadProxy::DidReceiveCompositorFrameAck() { layer_tree_host_->DidReceiveCompositorFrameAck(); } diff --git a/cc/trees/single_thread_proxy.h b/cc/trees/single_thread_proxy.h index 54e01cfdee56a..54842c41bf277 100644 --- a/cc/trees/single_thread_proxy.h +++ b/cc/trees/single_thread_proxy.h @@ -62,6 +62,7 @@ class CC_EXPORT SingleThreadProxy : public Proxy, // SchedulerClient implementation void WillBeginImplFrame(const BeginFrameArgs& args) override; void DidFinishImplFrame() override; + void DidNotProduceFrame(const BeginFrameAck& ack) override; void ScheduledActionSendBeginMainFrame(const BeginFrameArgs& args) override; DrawResult ScheduledActionDrawIfPossible() override; DrawResult ScheduledActionDrawForced() override; diff --git a/components/exo/surface.cc b/components/exo/surface.cc index 66784aebb22e0..d68aa04186364 100644 --- a/components/exo/surface.cc +++ b/components/exo/surface.cc @@ -426,9 +426,14 @@ void Surface::Commit() { CommitSurfaceHierarchy(); } - if (begin_frame_source_ && current_begin_frame_ack_.sequence_number != - cc::BeginFrameArgs::kInvalidFrameNumber) { - begin_frame_source_->DidFinishFrame(this, current_begin_frame_ack_); + if (current_begin_frame_ack_.sequence_number != + cc::BeginFrameArgs::kInvalidFrameNumber) { + if (begin_frame_source_) + begin_frame_source_->DidFinishFrame(this, current_begin_frame_ack_); + if (!current_begin_frame_ack_.has_damage) { + compositor_frame_sink_holder_->GetCompositorFrameSink() + ->DidNotProduceFrame(current_begin_frame_ack_); + } current_begin_frame_ack_.sequence_number = cc::BeginFrameArgs::kInvalidFrameNumber; } diff --git a/components/viz/frame_sinks/gpu_compositor_frame_sink.cc b/components/viz/frame_sinks/gpu_compositor_frame_sink.cc index 81acb033feac9..dd020e30a3971 100644 --- a/components/viz/frame_sinks/gpu_compositor_frame_sink.cc +++ b/components/viz/frame_sinks/gpu_compositor_frame_sink.cc @@ -50,9 +50,9 @@ void GpuCompositorFrameSink::SubmitCompositorFrame( support_->SubmitCompositorFrame(local_surface_id, std::move(frame)); } -void GpuCompositorFrameSink::BeginFrameDidNotSwap( +void GpuCompositorFrameSink::DidNotProduceFrame( const cc::BeginFrameAck& begin_frame_ack) { - support_->BeginFrameDidNotSwap(begin_frame_ack); + support_->DidNotProduceFrame(begin_frame_ack); } void GpuCompositorFrameSink::DidReceiveCompositorFrameAck( diff --git a/components/viz/frame_sinks/gpu_compositor_frame_sink.h b/components/viz/frame_sinks/gpu_compositor_frame_sink.h index 1d0e043362b6c..83cbf2f211d1c 100644 --- a/components/viz/frame_sinks/gpu_compositor_frame_sink.h +++ b/components/viz/frame_sinks/gpu_compositor_frame_sink.h @@ -41,7 +41,7 @@ class GpuCompositorFrameSink void SetNeedsBeginFrame(bool needs_begin_frame) override; void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; - void BeginFrameDidNotSwap(const cc::BeginFrameAck& begin_frame_ack) override; + void DidNotProduceFrame(const cc::BeginFrameAck& begin_frame_ack) override; // cc::mojom::MojoCompositorFrameSinkPrivate: void ClaimTemporaryReference(const cc::SurfaceId& surface_id) override; diff --git a/components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc b/components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc index b614fe8811b7d..850b6c7bbc74b 100644 --- a/components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc +++ b/components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc @@ -89,9 +89,9 @@ void GpuRootCompositorFrameSink::SubmitCompositorFrame( support_->SubmitCompositorFrame(local_surface_id, std::move(frame)); } -void GpuRootCompositorFrameSink::BeginFrameDidNotSwap( +void GpuRootCompositorFrameSink::DidNotProduceFrame( const cc::BeginFrameAck& begin_frame_ack) { - support_->BeginFrameDidNotSwap(begin_frame_ack); + support_->DidNotProduceFrame(begin_frame_ack); } void GpuRootCompositorFrameSink::ClaimTemporaryReference( diff --git a/components/viz/frame_sinks/gpu_root_compositor_frame_sink.h b/components/viz/frame_sinks/gpu_root_compositor_frame_sink.h index dc973c2d5b4ad..18df306f331cd 100644 --- a/components/viz/frame_sinks/gpu_root_compositor_frame_sink.h +++ b/components/viz/frame_sinks/gpu_root_compositor_frame_sink.h @@ -59,7 +59,7 @@ class GpuRootCompositorFrameSink void SetNeedsBeginFrame(bool needs_begin_frame) override; void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; - void BeginFrameDidNotSwap(const cc::BeginFrameAck& begin_frame_ack) override; + void DidNotProduceFrame(const cc::BeginFrameAck& begin_frame_ack) override; // cc::mojom::MojoCompositorFrameSinkPrivate: void ClaimTemporaryReference(const cc::SurfaceId& surface_id) override; diff --git a/content/browser/compositor/gpu_vsync_begin_frame_source.cc b/content/browser/compositor/gpu_vsync_begin_frame_source.cc index f9414105a00a3..7e43be479adc6 100644 --- a/content/browser/compositor/gpu_vsync_begin_frame_source.cc +++ b/content/browser/compositor/gpu_vsync_begin_frame_source.cc @@ -39,6 +39,4 @@ void GpuVSyncBeginFrameSource::OnNeedsBeginFrames(bool needs_begin_frames) { vsync_control_->SetNeedsVSync(needs_begin_frames); } -void GpuVSyncBeginFrameSource::OnDidFinishFrame(const cc::BeginFrameAck& ack) {} - } // namespace content diff --git a/content/browser/compositor/gpu_vsync_begin_frame_source.h b/content/browser/compositor/gpu_vsync_begin_frame_source.h index a701f98433f42..a3ddd6b078638 100644 --- a/content/browser/compositor/gpu_vsync_begin_frame_source.h +++ b/content/browser/compositor/gpu_vsync_begin_frame_source.h @@ -27,7 +27,6 @@ class GpuVSyncBeginFrameSource : public cc::ExternalBeginFrameSource, // cc::ExternalBeginFrameSourceClient implementation. void OnNeedsBeginFrames(bool needs_begin_frames) override; - void OnDidFinishFrame(const cc::BeginFrameAck& ack) override; void OnVSync(base::TimeTicks timestamp, base::TimeDelta interval); diff --git a/content/browser/frame_host/render_widget_host_view_child_frame.cc b/content/browser/frame_host/render_widget_host_view_child_frame.cc index 2b1a3292c4b65..6ecee9eba6026 100644 --- a/content/browser/frame_host/render_widget_host_view_child_frame.cc +++ b/content/browser/frame_host/render_widget_host_view_child_frame.cc @@ -409,9 +409,9 @@ void RenderWidgetHostViewChildFrame::SubmitCompositorFrame( ProcessCompositorFrame(local_surface_id, std::move(frame)); } -void RenderWidgetHostViewChildFrame::OnBeginFrameDidNotSwap( +void RenderWidgetHostViewChildFrame::OnDidNotProduceFrame( const cc::BeginFrameAck& ack) { - support_->BeginFrameDidNotSwap(ack); + support_->DidNotProduceFrame(ack); } void RenderWidgetHostViewChildFrame::OnSurfaceChanged( diff --git a/content/browser/frame_host/render_widget_host_view_child_frame.h b/content/browser/frame_host/render_widget_host_view_child_frame.h index 551c56cc266c6..844d36241994b 100644 --- a/content/browser/frame_host/render_widget_host_view_child_frame.h +++ b/content/browser/frame_host/render_widget_host_view_child_frame.h @@ -112,7 +112,7 @@ class CONTENT_EXPORT RenderWidgetHostViewChildFrame override; void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; - void OnBeginFrameDidNotSwap(const cc::BeginFrameAck& ack) override; + void OnDidNotProduceFrame(const cc::BeginFrameAck& ack) override; void OnSurfaceChanged(const cc::SurfaceInfo& surface_info) override; // Since the URL of content rendered by this class is not displayed in // the URL bar, this method does not need an implementation. diff --git a/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc b/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc index 1867a2903b2c3..ec62e2bf8004b 100644 --- a/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc +++ b/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc @@ -245,7 +245,7 @@ TEST_F(RenderWidgetHostViewChildFrameTest, FrameEviction) { } // Tests that BeginFrameAcks are forwarded correctly from the -// SwapCompositorFrame and OnBeginFrameDidNotSwap IPCs through the +// SwapCompositorFrame and DidNotProduceFrame IPCs through the // CompositorFrameSinkSupport. TEST_F(RenderWidgetHostViewChildFrameTest, ForwardsBeginFrameAcks) { gfx::Size view_size(100, 100); @@ -280,9 +280,9 @@ TEST_F(RenderWidgetHostViewChildFrameTest, ForwardsBeginFrameAcks) { cc::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, source_id, 6u); source.TestOnBeginFrame(args); - // Explicit ack through OnBeginFrameDidNotSwap is forwarded. + // Explicit ack through OnDidNotProduceFrame is forwarded. cc::BeginFrameAck ack(source_id, 6, 4, false); - view_->OnBeginFrameDidNotSwap(ack); + view_->OnDidNotProduceFrame(ack); EXPECT_EQ(ack, source.LastAckForObserver(view_->support_.get())); } diff --git a/content/browser/renderer_host/browser_compositor_view_mac.h b/content/browser/renderer_host/browser_compositor_view_mac.h index 1c73b7f3a9dd2..16599b12409c9 100644 --- a/content/browser/renderer_host/browser_compositor_view_mac.h +++ b/content/browser/renderer_host/browser_compositor_view_mac.h @@ -60,7 +60,7 @@ class BrowserCompositorMac : public DelegatedFrameHostClient { cc::mojom::MojoCompositorFrameSinkClient* renderer_compositor_frame_sink); void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame); - void OnBeginFrameDidNotSwap(const cc::BeginFrameAck& ack); + void OnDidNotProduceFrame(const cc::BeginFrameAck& ack); void SetHasTransparentBackground(bool transparent); void SetDisplayColorProfile(const gfx::ICCProfile& icc_profile); void UpdateVSyncParameters(const base::TimeTicks& timebase, diff --git a/content/browser/renderer_host/browser_compositor_view_mac.mm b/content/browser/renderer_host/browser_compositor_view_mac.mm index fba983b37fb09..4121584c970f5 100644 --- a/content/browser/renderer_host/browser_compositor_view_mac.mm +++ b/content/browser/renderer_host/browser_compositor_view_mac.mm @@ -293,9 +293,8 @@ void BrowserCompositorMac::SubmitCompositorFrame( std::move(frame)); } -void BrowserCompositorMac::OnBeginFrameDidNotSwap( - const cc::BeginFrameAck& ack) { - delegated_frame_host_->BeginFrameDidNotSwap(ack); +void BrowserCompositorMac::OnDidNotProduceFrame(const cc::BeginFrameAck& ack) { + delegated_frame_host_->DidNotProduceFrame(ack); } void BrowserCompositorMac::SetHasTransparentBackground(bool transparent) { diff --git a/content/browser/renderer_host/delegated_frame_host.cc b/content/browser/renderer_host/delegated_frame_host.cc index f03a59eb3dea4..db6020d1f70c0 100644 --- a/content/browser/renderer_host/delegated_frame_host.cc +++ b/content/browser/renderer_host/delegated_frame_host.cc @@ -219,7 +219,7 @@ void DelegatedFrameHost::SetNeedsBeginFrames(bool needs_begin_frames) { support_->SetNeedsBeginFrame(needs_begin_frames); } -void DelegatedFrameHost::BeginFrameDidNotSwap(const cc::BeginFrameAck& ack) { +void DelegatedFrameHost::DidNotProduceFrame(const cc::BeginFrameAck& ack) { DidFinishFrame(ack); cc::BeginFrameAck modified_ack = ack; @@ -232,7 +232,7 @@ void DelegatedFrameHost::BeginFrameDidNotSwap(const cc::BeginFrameAck& ack) { latest_confirmed_begin_frame_sequence_number_; } - support_->BeginFrameDidNotSwap(modified_ack); + support_->DidNotProduceFrame(modified_ack); } bool DelegatedFrameHost::ShouldSkipFrame(const gfx::Size& size_in_dip) { @@ -421,7 +421,7 @@ void DelegatedFrameHost::SubmitCompositorFrame( renderer_compositor_frame_sink_->DidReceiveCompositorFrameAck(resources); skipped_frames_ = true; - BeginFrameDidNotSwap(ack); + DidNotProduceFrame(ack); return; } diff --git a/content/browser/renderer_host/delegated_frame_host.h b/content/browser/renderer_host/delegated_frame_host.h index 6cb7a4986418b..067ccc2dd9d41 100644 --- a/content/browser/renderer_host/delegated_frame_host.h +++ b/content/browser/renderer_host/delegated_frame_host.h @@ -171,7 +171,7 @@ class CONTENT_EXPORT DelegatedFrameHost gfx::Point* transformed_point); void SetNeedsBeginFrames(bool needs_begin_frames); - void BeginFrameDidNotSwap(const cc::BeginFrameAck& ack); + void DidNotProduceFrame(const cc::BeginFrameAck& ack); // Exposed for tests. cc::SurfaceId SurfaceIdForTesting() const { diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc index e105587aa390b..ac87fb69e43d1 100644 --- a/content/browser/renderer_host/render_widget_host_impl.cc +++ b/content/browser/renderer_host/render_widget_host_impl.cc @@ -558,7 +558,7 @@ bool RenderWidgetHostImpl::OnMessageReceived(const IPC::Message &msg) { OnUpdateScreenRectsAck) IPC_MESSAGE_HANDLER(ViewHostMsg_RequestMove, OnRequestMove) IPC_MESSAGE_HANDLER(ViewHostMsg_SetTooltipText, OnSetTooltipText) - IPC_MESSAGE_HANDLER(ViewHostMsg_BeginFrameDidNotSwap, BeginFrameDidNotSwap) + IPC_MESSAGE_HANDLER(ViewHostMsg_DidNotProduceFrame, DidNotProduceFrame) IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateRect, OnUpdateRect) IPC_MESSAGE_HANDLER(ViewHostMsg_SetCursor, OnSetCursor) IPC_MESSAGE_HANDLER(ViewHostMsg_TextInputStateChanged, @@ -1929,13 +1929,13 @@ void RenderWidgetHostImpl::OnRequestMove(const gfx::Rect& pos) { } } -void RenderWidgetHostImpl::BeginFrameDidNotSwap(const cc::BeginFrameAck& ack) { +void RenderWidgetHostImpl::DidNotProduceFrame(const cc::BeginFrameAck& ack) { // |has_damage| is not transmitted. cc::BeginFrameAck modified_ack = ack; modified_ack.has_damage = false; if (view_) - view_->OnBeginFrameDidNotSwap(modified_ack); + view_->OnDidNotProduceFrame(modified_ack); } void RenderWidgetHostImpl::OnUpdateRect( diff --git a/content/browser/renderer_host/render_widget_host_impl.h b/content/browser/renderer_host/render_widget_host_impl.h index 871f8ed71e2e9..2cf98079c1fed 100644 --- a/content/browser/renderer_host/render_widget_host_impl.h +++ b/content/browser/renderer_host/render_widget_host_impl.h @@ -589,7 +589,7 @@ class CONTENT_EXPORT RenderWidgetHostImpl void SetNeedsBeginFrame(bool needs_begin_frame) override; void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; - void BeginFrameDidNotSwap(const cc::BeginFrameAck& ack) override; + void DidNotProduceFrame(const cc::BeginFrameAck& ack) override; void EvictCurrentSurface() override {} protected: @@ -651,7 +651,6 @@ class CONTENT_EXPORT RenderWidgetHostImpl void OnRequestMove(const gfx::Rect& pos); void OnSetTooltipText(const base::string16& tooltip_text, blink::WebTextDirection text_direction_hint); - void OnBeginFrameDidNotSwap(const cc::BeginFrameAck& ack); void OnUpdateRect(const ViewHostMsg_UpdateRect_Params& params); void OnQueueSyntheticGesture(const SyntheticGesturePacket& gesture_packet); void OnSetCursor(const WebCursor& cursor); diff --git a/content/browser/renderer_host/render_widget_host_view_android.cc b/content/browser/renderer_host/render_widget_host_view_android.cc index 4d30cc7010800..b9898a869d9c3 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.cc +++ b/content/browser/renderer_host/render_widget_host_view_android.cc @@ -1211,7 +1211,7 @@ void RenderWidgetHostViewAndroid::SubmitCompositorFrame( DestroyDelegatedContent(); ack.has_damage = false; - OnBeginFrameDidNotSwap(ack); + OnDidNotProduceFrame(ack); } else { delegated_frame_host_->SubmitCompositorFrame(local_surface_id, std::move(frame)); @@ -1242,8 +1242,16 @@ void RenderWidgetHostViewAndroid::DestroyDelegatedContent() { delegated_frame_host_->DestroyDelegatedContent(); } -void RenderWidgetHostViewAndroid::OnBeginFrameDidNotSwap( +void RenderWidgetHostViewAndroid::OnDidNotProduceFrame( const cc::BeginFrameAck& ack) { + if (!delegated_frame_host_) { + // We are not using the browser compositor and there's no DisplayScheduler + // that needs to be notified about the BeginFrameAck, so we can drop it. + DCHECK(!using_browser_compositor_); + return; + } + + delegated_frame_host_->DidNotProduceFrame(ack); AcknowledgeBeginFrame(ack); } @@ -2071,7 +2079,7 @@ void RenderWidgetHostViewAndroid::OnDetachCompositor() { void RenderWidgetHostViewAndroid::OnBeginFrame(const cc::BeginFrameArgs& args) { TRACE_EVENT0("cc,benchmark", "RenderWidgetHostViewAndroid::OnBeginFrame"); if (!host_) { - OnBeginFrameDidNotSwap( + OnDidNotProduceFrame( cc::BeginFrameAck(args.source_id, args.sequence_number, cc::BeginFrameArgs::kInvalidFrameNumber, false)); return; @@ -2084,8 +2092,8 @@ void RenderWidgetHostViewAndroid::OnBeginFrame(const cc::BeginFrameArgs& args) { uint64_t confirmed = cc::BeginFrameArgs::kInvalidFrameNumber; if (args.source_id == latest_confirmed_begin_frame_source_id_) confirmed = latest_confirmed_begin_frame_sequence_number_; - OnBeginFrameDidNotSwap(cc::BeginFrameAck( - args.source_id, args.sequence_number, confirmed, false)); + OnDidNotProduceFrame(cc::BeginFrameAck(args.source_id, args.sequence_number, + confirmed, false)); return; } @@ -2109,8 +2117,8 @@ void RenderWidgetHostViewAndroid::OnBeginFrame(const cc::BeginFrameArgs& args) { ClearBeginFrameRequest(BEGIN_FRAME); SendBeginFrame(args); } else { - OnBeginFrameDidNotSwap(cc::BeginFrameAck( - args.source_id, args.sequence_number, args.sequence_number, false)); + OnDidNotProduceFrame(cc::BeginFrameAck(args.source_id, args.sequence_number, + args.sequence_number, false)); } } diff --git a/content/browser/renderer_host/render_widget_host_view_android.h b/content/browser/renderer_host/render_widget_host_view_android.h index e953c65e1aefd..c5cf9d70df876 100644 --- a/content/browser/renderer_host/render_widget_host_view_android.h +++ b/content/browser/renderer_host/render_widget_host_view_android.h @@ -148,7 +148,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAndroid override; void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; - void OnBeginFrameDidNotSwap(const cc::BeginFrameAck& ack) override; + void OnDidNotProduceFrame(const cc::BeginFrameAck& ack) override; void ClearCompositorFrame() override; void SetIsInVR(bool is_in_vr) override; bool IsInVR() const override; diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc index d44b13ec78f9a..21cd1a62b9538 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura.cc @@ -944,9 +944,10 @@ void RenderWidgetHostViewAura::SubmitCompositorFrame( selection.end); } -void RenderWidgetHostViewAura::OnBeginFrameDidNotSwap( +void RenderWidgetHostViewAura::OnDidNotProduceFrame( const cc::BeginFrameAck& ack) { - delegated_frame_host_->BeginFrameDidNotSwap(ack); + if (delegated_frame_host_) + delegated_frame_host_->DidNotProduceFrame(ack); } void RenderWidgetHostViewAura::ClearCompositorFrame() { diff --git a/content/browser/renderer_host/render_widget_host_view_aura.h b/content/browser/renderer_host/render_widget_host_view_aura.h index b0975162ca8df..0a10f5b9abfab 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura.h +++ b/content/browser/renderer_host/render_widget_host_view_aura.h @@ -169,7 +169,7 @@ class CONTENT_EXPORT RenderWidgetHostViewAura override; void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; - void OnBeginFrameDidNotSwap(const cc::BeginFrameAck& ack) override; + void OnDidNotProduceFrame(const cc::BeginFrameAck& ack) override; void ClearCompositorFrame() override; void DidStopFlinging() override; void OnDidNavigateMainFrameToNewPage() override; diff --git a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc index 75d3fc5591993..b203dbf5309fa 100644 --- a/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc +++ b/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc @@ -3306,8 +3306,8 @@ class LastObserverTracker : public cc::FakeExternalBeginFrameSource::Client { } // namespace // Tests that BeginFrameAcks are forwarded correctly from the -// SwapCompositorFrame and OnBeginFrameDidNotSwap IPCs through -// DelegatedFrameHost and its CompositorFrameSinkSupport. +// SwapCompositorFrame and OnDidNotProduceFrame IPCs through DelegatedFrameHost +// and its CompositorFrameSinkSupport. TEST_F(RenderWidgetHostViewAuraTest, ForwardsBeginFrameAcks) { gfx::Rect view_rect(100, 100); gfx::Size frame_size = view_rect.size(); @@ -3354,9 +3354,9 @@ TEST_F(RenderWidgetHostViewAuraTest, ForwardsBeginFrameAcks) { cc::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, source_id, 6u); source.TestOnBeginFrame(args); - // Explicit ack through OnBeginFrameDidNotSwap is forwarded. + // Explicit ack through OnDidNotProduceFrame is forwarded. cc::BeginFrameAck ack(source_id, 6, 4, false); - view_->OnBeginFrameDidNotSwap(ack); + view_->OnDidNotProduceFrame(ack); EXPECT_EQ(ack, source.LastAckForObserver(observer_tracker.last_observer_)); } @@ -3413,10 +3413,10 @@ TEST_F(RenderWidgetHostViewAuraTest, ForwardsBeginFrameAcks) { BEGINFRAME_FROM_HERE, source_id, 11u); source.TestOnBeginFrame(args); - // Explicit ack through OnBeginFrameDidNotSwap is forwarded with invalid + // Explicit ack through OnDidNotProduceFrame is forwarded with invalid // latest_confirmed_sequence_number. cc::BeginFrameAck ack(source_id, 11, 11, false); - view_->OnBeginFrameDidNotSwap(ack); + view_->OnDidNotProduceFrame(ack); ack.latest_confirmed_sequence_number = cc::BeginFrameArgs::kInvalidFrameNumber; EXPECT_EQ(ack, source.LastAckForObserver(observer_tracker.last_observer_)); @@ -3444,9 +3444,9 @@ TEST_F(RenderWidgetHostViewAuraTest, ForwardsBeginFrameAcks) { BEGINFRAME_FROM_HERE, source_id, 13u); source.TestOnBeginFrame(args); - // Explicit ack through OnBeginFrameDidNotSwap is forwarded. + // Explicit ack through OnDidNotProduceFrame is forwarded. cc::BeginFrameAck ack(source_id, 13, 13, false); - view_->OnBeginFrameDidNotSwap(ack); + view_->OnDidNotProduceFrame(ack); EXPECT_EQ(ack, source.LastAckForObserver(observer_tracker.last_observer_)); } diff --git a/content/browser/renderer_host/render_widget_host_view_base.h b/content/browser/renderer_host/render_widget_host_view_base.h index e756ed927c580..c04ab0e05563a 100644 --- a/content/browser/renderer_host/render_widget_host_view_base.h +++ b/content/browser/renderer_host/render_widget_host_view_base.h @@ -229,7 +229,7 @@ class CONTENT_EXPORT RenderWidgetHostViewBase : public RenderWidgetHostView, virtual void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) = 0; - virtual void OnBeginFrameDidNotSwap(const cc::BeginFrameAck& ack) {} + virtual void OnDidNotProduceFrame(const cc::BeginFrameAck& ack) {} virtual void OnSurfaceChanged(const cc::SurfaceInfo& surface_info) {} // This method exists to allow removing of displayed graphics, after a new diff --git a/content/browser/renderer_host/render_widget_host_view_mac.h b/content/browser/renderer_host/render_widget_host_view_mac.h index 1de683f60d380..ac198a4424545 100644 --- a/content/browser/renderer_host/render_widget_host_view_mac.h +++ b/content/browser/renderer_host/render_widget_host_view_mac.h @@ -311,7 +311,7 @@ class CONTENT_EXPORT RenderWidgetHostViewMac override; void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; - void OnBeginFrameDidNotSwap(const cc::BeginFrameAck& ack) override; + void OnDidNotProduceFrame(const cc::BeginFrameAck& ack) override; void ClearCompositorFrame() override; BrowserAccessibilityManager* CreateBrowserAccessibilityManager( BrowserAccessibilityDelegate* delegate, bool for_root_frame) override; diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm index b27aebf241f04..efe95f0a6e749 100644 --- a/content/browser/renderer_host/render_widget_host_view_mac.mm +++ b/content/browser/renderer_host/render_widget_host_view_mac.mm @@ -1446,9 +1446,9 @@ void RenderWidgetHostViewMac::SubmitCompositorFrame( UpdateDisplayVSyncParameters(); } -void RenderWidgetHostViewMac::OnBeginFrameDidNotSwap( +void RenderWidgetHostViewMac::OnDidNotProduceFrame( const cc::BeginFrameAck& ack) { - browser_compositor_->OnBeginFrameDidNotSwap(ack); + browser_compositor_->OnDidNotProduceFrame(ack); } void RenderWidgetHostViewMac::ClearCompositorFrame() { diff --git a/content/common/view_messages.h b/content/common/view_messages.h index ec623cf506a4e..b3ed1ac858330 100644 --- a/content/common/view_messages.h +++ b/content/common/view_messages.h @@ -806,8 +806,7 @@ IPC_MESSAGE_ROUTED2(ViewHostMsg_FrameSwapMessages, // Sent if the BeginFrame did not cause a SwapCompositorFrame (e.g. because no // updates were required or because it was aborted in the renderer). -IPC_MESSAGE_ROUTED1(ViewHostMsg_BeginFrameDidNotSwap, - cc::BeginFrameAck /* ack */) +IPC_MESSAGE_ROUTED1(ViewHostMsg_DidNotProduceFrame, cc::BeginFrameAck /* ack */) // Send back a string to be recorded by UserMetrics. IPC_MESSAGE_CONTROL1(ViewHostMsg_UserMetricsRecordAction, diff --git a/content/renderer/android/synchronous_compositor_frame_sink.cc b/content/renderer/android/synchronous_compositor_frame_sink.cc index be2a84041928d..254ff01ad8617 100644 --- a/content/renderer/android/synchronous_compositor_frame_sink.cc +++ b/content/renderer/android/synchronous_compositor_frame_sink.cc @@ -28,6 +28,7 @@ #include "cc/surfaces/local_surface_id_allocator.h" #include "cc/surfaces/surface_manager.h" #include "content/common/android/sync_compositor_messages.h" +#include "content/common/view_messages.h" #include "content/renderer/android/synchronous_compositor_filter.h" #include "content/renderer/android/synchronous_compositor_registry.h" #include "content/renderer/gpu/frame_swap_message_queue.h" @@ -313,6 +314,13 @@ void SynchronousCompositorFrameSink::SubmitCompositorFrame( did_submit_frame_ = true; } +void SynchronousCompositorFrameSink::DidNotProduceFrame( + const cc::BeginFrameAck& ack) { + DCHECK(!ack.has_damage); + DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); + Send(new ViewHostMsg_DidNotProduceFrame(routing_id_, ack)); +} + void SynchronousCompositorFrameSink::CancelFallbackTick() { fallback_tick_.Cancel(); fallback_tick_pending_ = false; diff --git a/content/renderer/android/synchronous_compositor_frame_sink.h b/content/renderer/android/synchronous_compositor_frame_sink.h index 579fc717bf384..bab49796d99d8 100644 --- a/content/renderer/android/synchronous_compositor_frame_sink.h +++ b/content/renderer/android/synchronous_compositor_frame_sink.h @@ -86,6 +86,7 @@ class SynchronousCompositorFrameSink bool BindToClient(cc::CompositorFrameSinkClient* sink_client) override; void DetachFromClient() override; void SubmitCompositorFrame(cc::CompositorFrame frame) override; + void DidNotProduceFrame(const cc::BeginFrameAck& ack) override; void Invalidate() override; // Partial SynchronousCompositor API implementation. diff --git a/content/renderer/gpu/compositor_external_begin_frame_source.cc b/content/renderer/gpu/compositor_external_begin_frame_source.cc index 4c1608158b3c2..781c9d78b42eb 100644 --- a/content/renderer/gpu/compositor_external_begin_frame_source.cc +++ b/content/renderer/gpu/compositor_external_begin_frame_source.cc @@ -70,14 +70,6 @@ void CompositorExternalBeginFrameSource::OnNeedsBeginFrames( Send(new ViewHostMsg_SetNeedsBeginFrames(routing_id_, needs_begin_frames)); } -void CompositorExternalBeginFrameSource::OnDidFinishFrame( - const cc::BeginFrameAck& ack) { - DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); - // If there was damage, ViewHostMsg_SwapCompositorFrame includes the ack. - if (!ack.has_damage) - Send(new ViewHostMsg_BeginFrameDidNotSwap(routing_id_, ack)); -} - void CompositorExternalBeginFrameSource::OnMessageReceived( const IPC::Message& message) { DCHECK(CalledOnValidThread()); diff --git a/content/renderer/gpu/compositor_external_begin_frame_source.h b/content/renderer/gpu/compositor_external_begin_frame_source.h index d8826e06fc614..41eb692ca383c 100644 --- a/content/renderer/gpu/compositor_external_begin_frame_source.h +++ b/content/renderer/gpu/compositor_external_begin_frame_source.h @@ -49,7 +49,6 @@ class CompositorExternalBeginFrameSource // cc::ExternalBeginFrameSourceClient implementation. void OnNeedsBeginFrames(bool need_begin_frames) override; - void OnDidFinishFrame(const cc::BeginFrameAck& ack) override; private: class CompositorExternalBeginFrameSourceProxy diff --git a/content/renderer/gpu/renderer_compositor_frame_sink.cc b/content/renderer/gpu/renderer_compositor_frame_sink.cc index 2be7c4fb9e583..6409c54e9051d 100644 --- a/content/renderer/gpu/renderer_compositor_frame_sink.cc +++ b/content/renderer/gpu/renderer_compositor_frame_sink.cc @@ -123,6 +123,7 @@ void RendererCompositorFrameSink::DetachFromClient() { void RendererCompositorFrameSink::SubmitCompositorFrame( cc::CompositorFrame frame) { // We should only submit CompositorFrames with valid BeginFrameAcks. + DCHECK(frame.metadata.begin_frame_ack.has_damage); DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, frame.metadata.begin_frame_ack.sequence_number); auto new_surface_properties = @@ -154,6 +155,13 @@ void RendererCompositorFrameSink::SubmitCompositorFrame( } } +void RendererCompositorFrameSink::DidNotProduceFrame( + const cc::BeginFrameAck& ack) { + DCHECK(!ack.has_damage); + DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); + sink_->DidNotProduceFrame(ack); +} + void RendererCompositorFrameSink::OnMessageReceived( const IPC::Message& message) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -188,14 +196,6 @@ void RendererCompositorFrameSink::OnNeedsBeginFrames(bool needs_begin_frames) { sink_->SetNeedsBeginFrame(needs_begin_frames); } -void RendererCompositorFrameSink::OnDidFinishFrame( - const cc::BeginFrameAck& ack) { - DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); - // If there was damage, ViewHostMsg_SwapCompositorFrame includes the ack. - if (!ack.has_damage) - sink_->BeginFrameDidNotSwap(ack); -} - void RendererCompositorFrameSink::EstablishMojoConnection() { cc::mojom::MojoCompositorFrameSinkPtr sink; cc::mojom::MojoCompositorFrameSinkRequest sink_request = diff --git a/content/renderer/gpu/renderer_compositor_frame_sink.h b/content/renderer/gpu/renderer_compositor_frame_sink.h index 5ae7430a4d81f..eff76052902d9 100644 --- a/content/renderer/gpu/renderer_compositor_frame_sink.h +++ b/content/renderer/gpu/renderer_compositor_frame_sink.h @@ -71,6 +71,7 @@ class RendererCompositorFrameSink bool BindToClient(cc::CompositorFrameSinkClient* client) override; void DetachFromClient() override; void SubmitCompositorFrame(cc::CompositorFrame frame) override; + void DidNotProduceFrame(const cc::BeginFrameAck& ack) override; private: class RendererCompositorFrameSinkProxy @@ -104,7 +105,6 @@ class RendererCompositorFrameSink // cc::ExternalBeginFrameSourceClient implementation. void OnNeedsBeginFrames(bool need_begin_frames) override; - void OnDidFinishFrame(const cc::BeginFrameAck& ack) override; void EstablishMojoConnection(); diff --git a/services/ui/public/cpp/client_compositor_frame_sink.cc b/services/ui/public/cpp/client_compositor_frame_sink.cc index 487ef4dba3bf4..6a5ec5573b2fa 100644 --- a/services/ui/public/cpp/client_compositor_frame_sink.cc +++ b/services/ui/public/cpp/client_compositor_frame_sink.cc @@ -77,6 +77,7 @@ void ClientCompositorFrameSink::SubmitCompositorFrame( if (!compositor_frame_sink_) return; + DCHECK(frame.metadata.begin_frame_ack.has_damage); DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, frame.metadata.begin_frame_ack.sequence_number); @@ -91,6 +92,13 @@ void ClientCompositorFrameSink::SubmitCompositorFrame( std::move(frame)); } +void ClientCompositorFrameSink::DidNotProduceFrame( + const cc::BeginFrameAck& ack) { + DCHECK(!ack.has_damage); + DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); + compositor_frame_sink_->DidNotProduceFrame(ack); +} + ClientCompositorFrameSink::ClientCompositorFrameSink( scoped_refptr<cc::ContextProvider> context_provider, gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager, @@ -133,12 +141,6 @@ void ClientCompositorFrameSink::OnNeedsBeginFrames(bool needs_begin_frames) { compositor_frame_sink_->SetNeedsBeginFrame(needs_begin_frames); } -void ClientCompositorFrameSink::OnDidFinishFrame(const cc::BeginFrameAck& ack) { - // If there was damage, the submitted CompositorFrame includes the ack. - if (!ack.has_damage) - compositor_frame_sink_->BeginFrameDidNotSwap(ack); -} - ClientCompositorFrameSinkBinding::~ClientCompositorFrameSinkBinding() {} ClientCompositorFrameSinkBinding::ClientCompositorFrameSinkBinding( diff --git a/services/ui/public/cpp/client_compositor_frame_sink.h b/services/ui/public/cpp/client_compositor_frame_sink.h index 889cfa46697fc..d21fe0f5ee553 100644 --- a/services/ui/public/cpp/client_compositor_frame_sink.h +++ b/services/ui/public/cpp/client_compositor_frame_sink.h @@ -38,6 +38,7 @@ class ClientCompositorFrameSink void DetachFromClient() override; void SetLocalSurfaceId(const cc::LocalSurfaceId& local_surface_id) override; void SubmitCompositorFrame(cc::CompositorFrame frame) override; + void DidNotProduceFrame(const cc::BeginFrameAck& ack) override; private: ClientCompositorFrameSink( @@ -55,7 +56,6 @@ class ClientCompositorFrameSink // cc::ExternalBeginFrameSourceClient implementation. void OnNeedsBeginFrames(bool needs_begin_frames) override; - void OnDidFinishFrame(const cc::BeginFrameAck& ack) override; gfx::Size last_submitted_frame_size_; cc::LocalSurfaceId local_surface_id_; diff --git a/services/ui/ws/compositor_frame_sink_client_binding.cc b/services/ui/ws/compositor_frame_sink_client_binding.cc index 9ce826cb8d501..4532063b01467 100644 --- a/services/ui/ws/compositor_frame_sink_client_binding.cc +++ b/services/ui/ws/compositor_frame_sink_client_binding.cc @@ -37,9 +37,9 @@ void CompositorFrameSinkClientBinding::SubmitCompositorFrame( std::move(frame)); } -void CompositorFrameSinkClientBinding::BeginFrameDidNotSwap( +void CompositorFrameSinkClientBinding::DidNotProduceFrame( const cc::BeginFrameAck& ack) { - compositor_frame_sink_->BeginFrameDidNotSwap(ack); + compositor_frame_sink_->DidNotProduceFrame(ack); } void CompositorFrameSinkClientBinding::EvictCurrentSurface() { diff --git a/services/ui/ws/compositor_frame_sink_client_binding.h b/services/ui/ws/compositor_frame_sink_client_binding.h index 5d455841c5979..a55c89298720b 100644 --- a/services/ui/ws/compositor_frame_sink_client_binding.h +++ b/services/ui/ws/compositor_frame_sink_client_binding.h @@ -33,7 +33,7 @@ class CompositorFrameSinkClientBinding void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame) override; void SetNeedsBeginFrame(bool needs_begin_frame) override; - void BeginFrameDidNotSwap(const cc::BeginFrameAck& ack) override; + void DidNotProduceFrame(const cc::BeginFrameAck& ack) override; void EvictCurrentSurface() override; mojo::Binding<cc::mojom::MojoCompositorFrameSinkClient> binding_; diff --git a/services/ui/ws/frame_generator.cc b/services/ui/ws/frame_generator.cc index 23c61d4f49751..4b72293ba1683 100644 --- a/services/ui/ws/frame_generator.cc +++ b/services/ui/ws/frame_generator.cc @@ -82,7 +82,7 @@ void FrameGenerator::OnBeginFrame(const cc::BeginFrameArgs& begin_frame_args) { begin_frame_args.source_id, begin_frame_args.sequence_number, begin_frame_args.sequence_number, false); if (begin_frame_args.type == cc::BeginFrameArgs::MISSED) { - compositor_frame_sink_->BeginFrameDidNotSwap(current_begin_frame_ack_); + compositor_frame_sink_->DidNotProduceFrame(current_begin_frame_ack_); return; } diff --git a/services/ui/ws/frame_generator_unittest.cc b/services/ui/ws/frame_generator_unittest.cc index bc4aff45b538e..179e3342cb00a 100644 --- a/services/ui/ws/frame_generator_unittest.cc +++ b/services/ui/ws/frame_generator_unittest.cc @@ -46,8 +46,11 @@ class TestClientBinding : public cc::mojom::MojoCompositorFrameSink, cc::CompositorFrame frame) override { ++frames_submitted_; last_frame_ = std::move(frame); - begin_frame_source_->DidFinishFrame(this, - last_frame_.metadata.begin_frame_ack); + last_begin_frame_ack_ = last_frame_.metadata.begin_frame_ack; + } + + void DidNotProduceFrame(const cc::BeginFrameAck& ack) override { + last_begin_frame_ack_ = ack; } void SetNeedsBeginFrame(bool needs_begin_frame) override { @@ -61,11 +64,6 @@ class TestClientBinding : public cc::mojom::MojoCompositorFrameSink, begin_frame_source_->RemoveObserver(this); } - void BeginFrameDidNotSwap(const cc::BeginFrameAck& ack) override { - if (observing_begin_frames_) - begin_frame_source_->DidFinishFrame(this, ack); - } - void EvictCurrentSurface() override {} // cc::BeginFrameObserver implementation. @@ -94,6 +92,10 @@ class TestClientBinding : public cc::mojom::MojoCompositorFrameSink, int frames_submitted() const { return frames_submitted_; } + const cc::BeginFrameAck& last_begin_frame_ack() const { + return last_begin_frame_ack_; + } + private: cc::mojom::MojoCompositorFrameSinkClient* sink_client_; cc::BeginFrameArgs last_begin_frame_args_; @@ -101,6 +103,7 @@ class TestClientBinding : public cc::mojom::MojoCompositorFrameSink, cc::BeginFrameSource* begin_frame_source_ = nullptr; bool observing_begin_frames_ = false; int frames_submitted_ = 0; + cc::BeginFrameAck last_begin_frame_ack_; }; class FrameGeneratorTest : public testing::Test { @@ -149,8 +152,8 @@ class FrameGeneratorTest : public testing::Test { int NumberOfFramesReceived() const { return binding_->frames_submitted(); } - const cc::BeginFrameAck& LastBeginFrameAck() { - return begin_frame_source_->LastAckForObserver(binding_); + const cc::BeginFrameAck& LastBeginFrameAck() const { + return binding_->last_begin_frame_ack(); } const cc::CompositorFrameMetadata& LastMetadata() const { diff --git a/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp b/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp index 13ff8e42ec18c..b51d02e8d83df 100644 --- a/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp +++ b/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp @@ -434,7 +434,7 @@ void OffscreenCanvasFrameDispatcherImpl::OnBeginFrame( if (pending_compositor_frames_ >= kMaxPendingCompositorFrames || (begin_frame_args.type == cc::BeginFrameArgs::MISSED && base::TimeTicks::Now() > begin_frame_args.deadline)) { - sink_->BeginFrameDidNotSwap(current_begin_frame_ack_); + sink_->DidNotProduceFrame(current_begin_frame_ack_); return; } diff --git a/ui/android/delegated_frame_host_android.cc b/ui/android/delegated_frame_host_android.cc index 3963a3adb544e..2d9389e506360 100644 --- a/ui/android/delegated_frame_host_android.cc +++ b/ui/android/delegated_frame_host_android.cc @@ -94,6 +94,11 @@ void DelegatedFrameHostAndroid::SubmitCompositorFrame( } } +void DelegatedFrameHostAndroid::DidNotProduceFrame( + const cc::BeginFrameAck& ack) { + support_->DidNotProduceFrame(ack); +} + cc::FrameSinkId DelegatedFrameHostAndroid::GetFrameSinkId() const { return frame_sink_id_; } @@ -183,12 +188,6 @@ void DelegatedFrameHostAndroid::OnNeedsBeginFrames(bool needs_begin_frames) { support_->SetNeedsBeginFrame(needs_begin_frames); } -void DelegatedFrameHostAndroid::OnDidFinishFrame(const cc::BeginFrameAck& ack) { - // If there was damage, SubmitCompositorFrame includes the ack. - if (!ack.has_damage) - support_->BeginFrameDidNotSwap(ack); -} - void DelegatedFrameHostAndroid::CreateNewCompositorFrameSinkSupport() { constexpr bool is_root = false; constexpr bool handles_frame_sink_id_invalidation = false; diff --git a/ui/android/delegated_frame_host_android.h b/ui/android/delegated_frame_host_android.h index d6065d4ffc73f..0e0991e628f28 100644 --- a/ui/android/delegated_frame_host_android.h +++ b/ui/android/delegated_frame_host_android.h @@ -48,6 +48,7 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid void SubmitCompositorFrame(const cc::LocalSurfaceId& local_surface_id, cc::CompositorFrame frame); + void DidNotProduceFrame(const cc::BeginFrameAck& ack); void DestroyDelegatedContent(); @@ -82,7 +83,6 @@ class UI_ANDROID_EXPORT DelegatedFrameHostAndroid // cc::ExternalBeginFrameSourceClient implementation. void OnNeedsBeginFrames(bool needs_begin_frames) override; - void OnDidFinishFrame(const cc::BeginFrameAck& ack) override; void CreateNewCompositorFrameSinkSupport(); diff --git a/ui/aura/local/compositor_frame_sink_local.cc b/ui/aura/local/compositor_frame_sink_local.cc index 9674689634bdc..2aa89b2de22c2 100644 --- a/ui/aura/local/compositor_frame_sink_local.cc +++ b/ui/aura/local/compositor_frame_sink_local.cc @@ -61,6 +61,9 @@ void CompositorFrameSinkLocal::SubmitCompositorFrame( cc::CompositorFrame frame) { DCHECK(thread_checker_); DCHECK(thread_checker_->CalledOnValidThread()); + DCHECK(frame.metadata.begin_frame_ack.has_damage); + DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, + frame.metadata.begin_frame_ack.sequence_number); cc::LocalSurfaceId old_local_surface_id = local_surface_id_; if (!frame.render_pass_list.empty()) { @@ -80,6 +83,15 @@ void CompositorFrameSinkLocal::SubmitCompositorFrame( } } +void CompositorFrameSinkLocal::DidNotProduceFrame( + const cc::BeginFrameAck& ack) { + DCHECK(thread_checker_); + DCHECK(thread_checker_->CalledOnValidThread()); + DCHECK(!ack.has_damage); + DCHECK_LE(cc::BeginFrameArgs::kStartingFrameNumber, ack.sequence_number); + support_->DidNotProduceFrame(ack); +} + void CompositorFrameSinkLocal::DidReceiveCompositorFrameAck( const cc::ReturnedResourceArray& resources) { DCHECK(thread_checker_); @@ -112,12 +124,4 @@ void CompositorFrameSinkLocal::OnNeedsBeginFrames(bool needs_begin_frames) { support_->SetNeedsBeginFrame(needs_begin_frames); } -void CompositorFrameSinkLocal::OnDidFinishFrame(const cc::BeginFrameAck& ack) { - DCHECK(thread_checker_); - DCHECK(thread_checker_->CalledOnValidThread()); - // If there was damage, the submitted CompositorFrame includes the ack. - if (!ack.has_damage) - support_->BeginFrameDidNotSwap(ack); -} - } // namespace aura diff --git a/ui/aura/local/compositor_frame_sink_local.h b/ui/aura/local/compositor_frame_sink_local.h index 65afe361958dc..0be946a4cc14c 100644 --- a/ui/aura/local/compositor_frame_sink_local.h +++ b/ui/aura/local/compositor_frame_sink_local.h @@ -43,6 +43,7 @@ class CompositorFrameSinkLocal : public cc::CompositorFrameSink, bool BindToClient(cc::CompositorFrameSinkClient* client) override; void DetachFromClient() override; void SubmitCompositorFrame(cc::CompositorFrame frame) override; + void DidNotProduceFrame(const cc::BeginFrameAck& ack) override; // cc::CompositorFrameSinkSupportClient: void DidReceiveCompositorFrameAck( @@ -54,7 +55,6 @@ class CompositorFrameSinkLocal : public cc::CompositorFrameSink, // cc::ExternalBeginFrameSourceClient: void OnNeedsBeginFrames(bool needs_begin_frames) override; - void OnDidFinishFrame(const cc::BeginFrameAck& ack) override; private: const cc::FrameSinkId frame_sink_id_;