Merge "SurfaceFlinger: Fix duplicate callbacks" (caae5fde) · Commits · e / os / android_frameworks_native-old

services/surfaceflinger/SurfaceFlinger.cpp

+36 −35

Original line number	Original line	Diff line number	Diff line
	@@ -2022,17 +2022,23 @@ bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expected
	bool SurfaceFlinger::flushAndCommitTransactions() {		bool SurfaceFlinger::flushAndCommitTransactions() {
	ATRACE_CALL();		ATRACE_CALL();

			bool needsTraversal = false;
	if (clearTransactionFlags(eTransactionFlushNeeded)) {		if (clearTransactionFlags(eTransactionFlushNeeded)) {
	flushTransactionQueues();		needsTraversal = flushTransactionQueues();
	}		}

	const bool shouldCommit = (getTransactionFlags() & ~eTransactionFlushNeeded) \|\| mForceTraversal;		const bool shouldCommit = (getTransactionFlags() & ~eTransactionFlushNeeded) \|\| needsTraversal;
	if (shouldCommit) {		if (shouldCommit) {
	commitTransactions();		commitTransactions();
	}		}

			if (!needsTraversal) {
			// Invoke empty transaction callbacks early.
			mTransactionCallbackInvoker.sendCallbacks(false /* onCommitOnly */);
			} else {
	// Invoke OnCommit callbacks.		// Invoke OnCommit callbacks.
	mTransactionCallbackInvoker.sendCallbacks();		mTransactionCallbackInvoker.sendCallbacks(true /* onCommitOnly */);
			}

	if (transactionFlushNeeded()) {		if (transactionFlushNeeded()) {
	setTransactionFlags(eTransactionFlushNeeded);		setTransactionFlags(eTransactionFlushNeeded);
	@@ -2329,7 +2335,7 @@ void SurfaceFlinger::postComposition() {
	mVisibleRegionsWereDirtyThisFrame = false;		mVisibleRegionsWereDirtyThisFrame = false;

	mTransactionCallbackInvoker.addPresentFence(mPreviousPresentFences[0].fence);		mTransactionCallbackInvoker.addPresentFence(mPreviousPresentFences[0].fence);
	mTransactionCallbackInvoker.sendCallbacks();		mTransactionCallbackInvoker.sendCallbacks(false /* onCommitOnly */);
	mTransactionCallbackInvoker.clearCompletedTransactions();		mTransactionCallbackInvoker.clearCompletedTransactions();

	if (display && display->isInternal() && display->getPowerMode() == hal::PowerMode::ON &&		if (display && display->isInternal() && display->getPowerMode() == hal::PowerMode::ON &&
	@@ -2942,7 +2948,6 @@ void SurfaceFlinger::commitTransactionsLocked(uint32_t transactionFlags) {
	processDisplayChangesLocked();		processDisplayChangesLocked();
	processDisplayHotplugEventsLocked();		processDisplayHotplugEventsLocked();
	}		}
	mForceTraversal = false;
	mForceTransactionDisplayChange = displayTransactionNeeded;		mForceTransactionDisplayChange = displayTransactionNeeded;

	if (mSomeChildrenChanged) {		if (mSomeChildrenChanged) {
	@@ -3416,11 +3421,8 @@ uint32_t SurfaceFlinger::setTransactionFlags(uint32_t mask, TransactionSchedule
	return old;		return old;
	}		}

	void SurfaceFlinger::setTraversalNeeded() {		bool SurfaceFlinger::flushTransactionQueues() {
	mForceTraversal = true;		bool needsTraversal = false;
	}

	void SurfaceFlinger::flushTransactionQueues() {
	// to prevent onHandleDestroyed from being called while the lock is held,		// to prevent onHandleDestroyed from being called while the lock is held,
	// we must keep a copy of the transactions (specifically the composer		// we must keep a copy of the transactions (specifically the composer
	// states) around outside the scope of the lock		// states) around outside the scope of the lock
	@@ -3489,19 +3491,23 @@ void SurfaceFlinger::flushTransactionQueues() {

	// Now apply all transactions.		// Now apply all transactions.
	for (const auto& transaction : transactions) {		for (const auto& transaction : transactions) {
			needsTraversal \|=
	applyTransactionState(transaction.frameTimelineInfo, transaction.states,		applyTransactionState(transaction.frameTimelineInfo, transaction.states,
	transaction.displays, transaction.flags,		transaction.displays, transaction.flags,
	transaction.inputWindowCommands, transaction.desiredPresentTime,		transaction.inputWindowCommands,
			transaction.desiredPresentTime,
	transaction.isAutoTimestamp, transaction.buffer,		transaction.isAutoTimestamp, transaction.buffer,
	transaction.postTime, transaction.permissions,		transaction.postTime, transaction.permissions,
	transaction.hasListenerCallbacks, transaction.listenerCallbacks,		transaction.hasListenerCallbacks,
	transaction.originPid, transaction.originUid, transaction.id);		transaction.listenerCallbacks, transaction.originPid,
			transaction.originUid, transaction.id);
	if (transaction.transactionCommittedSignal) {		if (transaction.transactionCommittedSignal) {
	mTransactionCommittedSignals.emplace_back(		mTransactionCommittedSignals.emplace_back(
	std::move(transaction.transactionCommittedSignal));		std::move(transaction.transactionCommittedSignal));
	}		}
	}		}
	}		} // unlock mStateLock
			return needsTraversal;
	}		}

	bool SurfaceFlinger::transactionFlushNeeded() {		bool SurfaceFlinger::transactionFlushNeeded() {
	@@ -3707,7 +3713,7 @@ status_t SurfaceFlinger::setTransactionState(
	return NO_ERROR;		return NO_ERROR;
	}		}

	void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelineInfo,		bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelineInfo,
	const Vector<ComposerState>& states,		const Vector<ComposerState>& states,
	const Vector<DisplayState>& displays, uint32_t flags,		const Vector<DisplayState>& displays, uint32_t flags,
	const InputWindowCommands& inputWindowCommands,		const InputWindowCommands& inputWindowCommands,
	@@ -3729,12 +3735,10 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin
	mTransactionCallbackInvoker.addEmptyTransaction(listener);		mTransactionCallbackInvoker.addEmptyTransaction(listener);
	}		}

	std::unordered_set<ListenerCallbacks, ListenerCallbacksHash> listenerCallbacksWithSurfaces;
	uint32_t clientStateFlags = 0;		uint32_t clientStateFlags = 0;
	for (const ComposerState& state : states) {		for (const ComposerState& state : states) {
	clientStateFlags \|=		clientStateFlags \|= setClientStateLocked(frameTimelineInfo, state, desiredPresentTime,
	setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp,		isAutoTimestamp, postTime, permissions);
	postTime, permissions, listenerCallbacksWithSurfaces);
	if ((flags & eAnimation) && state.state.surface) {		if ((flags & eAnimation) && state.state.surface) {
	if (const auto layer = fromHandle(state.state.surface).promote(); layer) {		if (const auto layer = fromHandle(state.state.surface).promote(); layer) {
	mScheduler->recordLayerHistory(layer.get(),		mScheduler->recordLayerHistory(layer.get(),
	@@ -3744,10 +3748,6 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin
	}		}
	}		}

	// If the state doesn't require a traversal and there are callbacks, send them now
	if (!(clientStateFlags & eTraversalNeeded) && hasListenerCallbacks) {
	mTransactionCallbackInvoker.sendCallbacks();
	}
	transactionFlags \|= clientStateFlags;		transactionFlags \|= clientStateFlags;

	if (permissions & Permission::ACCESS_SURFACE_FLINGER) {		if (permissions & Permission::ACCESS_SURFACE_FLINGER) {
	@@ -3769,6 +3769,7 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin
	transactionFlags = eTransactionNeeded;		transactionFlags = eTransactionNeeded;
	}		}

			bool needsTraversal = false;
	if (transactionFlags) {		if (transactionFlags) {
	if (mInterceptor->isEnabled()) {		if (mInterceptor->isEnabled()) {
	mInterceptor->saveTransaction(states, mCurrentState.displays, displays, flags,		mInterceptor->saveTransaction(states, mCurrentState.displays, displays, flags,
	@@ -3779,7 +3780,7 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin
	// so we don't have to wake up again next frame to preform an unnecessary traversal.		// so we don't have to wake up again next frame to preform an unnecessary traversal.
	if (transactionFlags & eTraversalNeeded) {		if (transactionFlags & eTraversalNeeded) {
	transactionFlags = transactionFlags & (~eTraversalNeeded);		transactionFlags = transactionFlags & (~eTraversalNeeded);
	mForceTraversal = true;		needsTraversal = true;
	}		}
	if (transactionFlags) {		if (transactionFlags) {
	setTransactionFlags(transactionFlags);		setTransactionFlags(transactionFlags);
	@@ -3789,6 +3790,8 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin
	mAnimTransactionPending = true;		mAnimTransactionPending = true;
	}		}
	}		}

			return needsTraversal;
	}		}

	uint32_t SurfaceFlinger::setDisplayStateLocked(const DisplayState& s) {		uint32_t SurfaceFlinger::setDisplayStateLocked(const DisplayState& s) {
	@@ -3857,10 +3860,10 @@ bool SurfaceFlinger::callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermis
	return true;		return true;
	}		}

	uint32_t SurfaceFlinger::setClientStateLocked(		uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTimelineInfo,
	const FrameTimelineInfo& frameTimelineInfo, const ComposerState& composerState,		const ComposerState& composerState,
	int64_t desiredPresentTime, bool isAutoTimestamp, int64_t postTime, uint32_t permissions,		int64_t desiredPresentTime, bool isAutoTimestamp,
	std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>& outListenerCallbacks) {		int64_t postTime, uint32_t permissions) {
	const layer_state_t& s = composerState.state;		const layer_state_t& s = composerState.state;
	const bool privileged = permissions & Permission::ACCESS_SURFACE_FLINGER;		const bool privileged = permissions & Permission::ACCESS_SURFACE_FLINGER;

	@@ -3874,13 +3877,11 @@ uint32_t SurfaceFlinger::setClientStateLocked(
	ListenerCallbacks onCommitCallbacks = listener.filter(CallbackId::Type::ON_COMMIT);		ListenerCallbacks onCommitCallbacks = listener.filter(CallbackId::Type::ON_COMMIT);
	if (!onCommitCallbacks.callbackIds.empty()) {		if (!onCommitCallbacks.callbackIds.empty()) {
	filteredListeners.push_back(onCommitCallbacks);		filteredListeners.push_back(onCommitCallbacks);
	outListenerCallbacks.insert(onCommitCallbacks);
	}		}

	ListenerCallbacks onCompleteCallbacks = listener.filter(CallbackId::Type::ON_COMPLETE);		ListenerCallbacks onCompleteCallbacks = listener.filter(CallbackId::Type::ON_COMPLETE);
	if (!onCompleteCallbacks.callbackIds.empty()) {		if (!onCompleteCallbacks.callbackIds.empty()) {
	filteredListeners.push_back(onCompleteCallbacks);		filteredListeners.push_back(onCompleteCallbacks);
	outListenerCallbacks.insert(onCompleteCallbacks);
	}		}
	}		}

services/surfaceflinger/SurfaceFlinger.h

+3 −6

Original line number	Original line	Diff line number	Diff line
	@@ -712,7 +712,7 @@ private:
	/*		/*
	* Transactions		* Transactions
	*/		*/
	void applyTransactionState(const FrameTimelineInfo& info, const Vector<ComposerState>& state,		bool applyTransactionState(const FrameTimelineInfo& info, const Vector<ComposerState>& state,
	const Vector<DisplayState>& displays, uint32_t flags,		const Vector<DisplayState>& displays, uint32_t flags,
	const InputWindowCommands& inputWindowCommands,		const InputWindowCommands& inputWindowCommands,
	const int64_t desiredPresentTime, bool isAutoTimestamp,		const int64_t desiredPresentTime, bool isAutoTimestamp,
	@@ -722,15 +722,13 @@ private:
	int originPid, int originUid, uint64_t transactionId)		int originPid, int originUid, uint64_t transactionId)
	REQUIRES(mStateLock);		REQUIRES(mStateLock);
	// flush pending transaction that was presented after desiredPresentTime.		// flush pending transaction that was presented after desiredPresentTime.
	void flushTransactionQueues();		bool flushTransactionQueues();
	// Returns true if there is at least one transaction that needs to be flushed		// Returns true if there is at least one transaction that needs to be flushed
	bool transactionFlushNeeded();		bool transactionFlushNeeded();

	uint32_t setClientStateLocked(const FrameTimelineInfo&, const ComposerState&,		uint32_t setClientStateLocked(const FrameTimelineInfo&, const ComposerState&,
	int64_t desiredPresentTime, bool isAutoTimestamp,		int64_t desiredPresentTime, bool isAutoTimestamp,
	int64_t postTime, uint32_t permissions,		int64_t postTime, uint32_t permissions) REQUIRES(mStateLock);
	std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>&)
	REQUIRES(mStateLock);

	uint32_t getTransactionFlags() const;		uint32_t getTransactionFlags() const;

	@@ -1115,7 +1113,6 @@ private:
	std::vector<std::shared_ptr<CountDownLatch>> mTransactionCommittedSignals;		std::vector<std::shared_ptr<CountDownLatch>> mTransactionCommittedSignals;
	bool mAnimTransactionPending = false;		bool mAnimTransactionPending = false;
	SortedVector<sp<Layer>> mLayersPendingRemoval;		SortedVector<sp<Layer>> mLayersPendingRemoval;
	bool mForceTraversal = false;

	// global color transform states		// global color transform states
	Daltonizer mDaltonizer;		Daltonizer mDaltonizer;

services/surfaceflinger/TransactionCallbackInvoker.cpp

+8 −3

Original line number	Original line	Diff line number	Diff line
	@@ -121,7 +121,7 @@ status_t TransactionCallbackInvoker::registerUnpresentedCallbackHandle(
	return addCallbackHandle(handle, std::vector<JankData>());		return addCallbackHandle(handle, std::vector<JankData>());
	}		}

	status_t TransactionCallbackInvoker::findTransactionStats(		status_t TransactionCallbackInvoker::findOrCreateTransactionStats(
	const sp<IBinder>& listener, const std::vector<CallbackId>& callbackIds,		const sp<IBinder>& listener, const std::vector<CallbackId>& callbackIds,
	TransactionStats** outTransactionStats) {		TransactionStats** outTransactionStats) {
	auto& transactionStatsDeque = mCompletedTransactions[listener];		auto& transactionStatsDeque = mCompletedTransactions[listener];
	@@ -143,7 +143,8 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>&
	// If we can't find the transaction stats something has gone wrong. The client should call		// If we can't find the transaction stats something has gone wrong. The client should call
	// startRegistration before trying to add a callback handle.		// startRegistration before trying to add a callback handle.
	TransactionStats* transactionStats;		TransactionStats* transactionStats;
	status_t err = findTransactionStats(handle->listener, handle->callbackIds, &transactionStats);		status_t err =
			findOrCreateTransactionStats(handle->listener, handle->callbackIds, &transactionStats);
	if (err != NO_ERROR) {		if (err != NO_ERROR) {
	return err;		return err;
	}		}
	@@ -204,7 +205,7 @@ void TransactionCallbackInvoker::addPresentFence(const sp<Fence>& presentFence)
	mPresentFence = presentFence;		mPresentFence = presentFence;
	}		}

	void TransactionCallbackInvoker::sendCallbacks() {		void TransactionCallbackInvoker::sendCallbacks(bool onCommitOnly) {
	// For each listener		// For each listener
	auto completedTransactionsItr = mCompletedTransactions.begin();		auto completedTransactionsItr = mCompletedTransactions.begin();
	while (completedTransactionsItr != mCompletedTransactions.end()) {		while (completedTransactionsItr != mCompletedTransactions.end()) {
	@@ -216,6 +217,10 @@ void TransactionCallbackInvoker::sendCallbacks() {
	auto transactionStatsItr = transactionStatsDeque.begin();		auto transactionStatsItr = transactionStatsDeque.begin();
	while (transactionStatsItr != transactionStatsDeque.end()) {		while (transactionStatsItr != transactionStatsDeque.end()) {
	auto& transactionStats = *transactionStatsItr;		auto& transactionStats = *transactionStatsItr;
			if (onCommitOnly && !containsOnCommitCallbacks(transactionStats.callbackIds)) {
			transactionStatsItr++;
			continue;
			}

	// If the transaction has been latched		// If the transaction has been latched
	if (transactionStats.latchTime >= 0 &&		if (transactionStats.latchTime >= 0 &&

services/surfaceflinger/TransactionCallbackInvoker.h

+4 −6

Original line number	Original line	Diff line number	Diff line
	@@ -76,7 +76,7 @@ public:

	void addPresentFence(const sp<Fence>& presentFence);		void addPresentFence(const sp<Fence>& presentFence);

	void sendCallbacks();		void sendCallbacks(bool onCommitOnly);
	void clearCompletedTransactions() {		void clearCompletedTransactions() {
	mCompletedTransactions.clear();		mCompletedTransactions.clear();
	}		}
	@@ -86,12 +86,10 @@ public:


	private:		private:
	status_t findTransactionStats(const sp<IBinder>& listener,		status_t findOrCreateTransactionStats(const sp<IBinder>& listener,
	const std::vector<CallbackId>& callbackIds,		const std::vector<CallbackId>& callbackIds,
	TransactionStats** outTransactionStats);		TransactionStats** outTransactionStats);



	std::unordered_map<sp<IBinder>, std::deque<TransactionStats>, IListenerHash>		std::unordered_map<sp<IBinder>, std::deque<TransactionStats>, IListenerHash>
	mCompletedTransactions;		mCompletedTransactions;

services/surfaceflinger/tests/LayerCallback_test.cpp

+56 −0

Original line number	Original line	Diff line number	Diff line
	@@ -1029,4 +1029,60 @@ TEST_F(LayerCallbackTest, ExpectedPresentTime) {
	EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));		EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
	}		}

			// b202394221
			TEST_F(LayerCallbackTest, EmptyBufferStateChanges) {
			sp<SurfaceControl> bufferLayer, emptyBufferLayer;
			ASSERT_NO_FATAL_FAILURE(bufferLayer = createBufferStateLayer());
			ASSERT_NO_FATAL_FAILURE(emptyBufferLayer = createBufferStateLayer());

			Transaction transaction;
			CallbackHelper callback;
			for (size_t i = 0; i < 10; i++) {
			int err = fillTransaction(transaction, &callback, bufferLayer);
			if (err) {
			GTEST_SUCCEED() << "test not supported";
			return;
			}

			ui::Size bufferSize = getBufferSize();

			TransactionUtils::setFrame(transaction, bufferLayer,
			Rect(0, 0, bufferSize.width, bufferSize.height),
			Rect(0, 0, 32, 32));
			transaction.setPosition(emptyBufferLayer, 1 + i, 2 + i);
			transaction.apply();

			ExpectedResult expected;
			expected.addSurface(ExpectedResult::Transaction::PRESENTED, bufferLayer,
			ExpectedResult::Buffer::ACQUIRED,
			(i == 0) ? ExpectedResult::PreviousBuffer::NOT_RELEASED
			: ExpectedResult::PreviousBuffer::RELEASED);
			expected.addSurface(ExpectedResult::Transaction::PRESENTED, emptyBufferLayer,
			ExpectedResult::Buffer::NOT_ACQUIRED,
			ExpectedResult::PreviousBuffer::NOT_RELEASED);

			EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected));
			}
			ASSERT_NO_FATAL_FAILURE(callback.verifyFinalState());
			}

			// b202394221
			TEST_F(LayerCallbackTest, DISABLED_NonBufferLayerStateChanges) {
			sp<SurfaceControl> layer;
			ASSERT_NO_FATAL_FAILURE(layer = createColorLayer("ColorLayer", Color::RED));

			Transaction transaction;
			CallbackHelper callback;
			int err = fillTransaction(transaction, &callback);
			if (err) {
			GTEST_SUCCEED() << "test not supported";
			return;
			}
			transaction.setPosition(layer, 1, 2);
			transaction.apply();

			ExpectedResult expected;
			EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
			}

	} // namespace android		} // namespace android