SurfaceFlinger: mExpectedPresentTime should not be updated outside the
main thread
mExpectedPresentTime should be set only by the main thread when
composition starts as it is used throughout the composition stage,
and updates to this variable may cause SF to use inconsistent values
for determining the expected present time.
Test: Run TouchLatency app and observe systraces
Test: Expand notification shade and observe systraces
Bug: 178148035
Change-Id: Ie221d7e1fb0b0965460af773d3ecf0b06b5c5d7a
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index eb2c1ba..7c431a0 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -127,7 +127,6 @@
ASSERT_EQ(0u, mFlinger.getTransactionQueue().size());
// called in SurfaceFlinger::signalTransaction
EXPECT_CALL(*mMessageQueue, invalidate()).Times(1);
- EXPECT_CALL(*mVSyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillOnce(Return(systemTime()));
TransactionInfo transaction;
setupSingle(transaction, flags, syncInputWindows,
/*desiredPresentTime*/ systemTime(), /*isAutoTimestamp*/ true,
@@ -163,8 +162,6 @@
// first check will see desired present time has not passed,
// but afterwards it will look like the desired present time has passed
nsecs_t time = systemTime();
- EXPECT_CALL(*mVSyncTracker, nextAnticipatedVSyncTimeFrom(_))
- .WillOnce(Return(time + nsecs_t(5 * 1e8)));
TransactionInfo transaction;
setupSingle(transaction, flags, syncInputWindows,
/*desiredPresentTime*/ time + s2ns(1), false, FrameTimelineInfo{});
@@ -177,7 +174,11 @@
transaction.id);
nsecs_t returnedTime = systemTime();
- EXPECT_LE(returnedTime, applicationSentTime + s2ns(5));
+ if ((flags & ISurfaceComposer::eSynchronous) || syncInputWindows) {
+ EXPECT_GE(systemTime(), applicationSentTime + s2ns(5));
+ } else {
+ EXPECT_LE(returnedTime, applicationSentTime + s2ns(5));
+ }
// This transaction should have been placed on the transaction queue
auto transactionQueue = mFlinger.getTransactionQueue();
EXPECT_EQ(1u, transactionQueue.size());
@@ -187,9 +188,11 @@
ASSERT_EQ(0u, mFlinger.getTransactionQueue().size());
// called in SurfaceFlinger::signalTransaction
nsecs_t time = systemTime();
- EXPECT_CALL(*mMessageQueue, invalidate()).Times(1);
- EXPECT_CALL(*mVSyncTracker, nextAnticipatedVSyncTimeFrom(_))
- .WillOnce(Return(time + nsecs_t(5 * 1e8)));
+ if (!syncInputWindows) {
+ EXPECT_CALL(*mMessageQueue, invalidate()).Times(2);
+ } else {
+ EXPECT_CALL(*mMessageQueue, invalidate()).Times(1);
+ }
// transaction that should go on the pending thread
TransactionInfo transactionA;
setupSingle(transactionA, /*flags*/ 0, /*syncInputWindows*/ false,
@@ -229,7 +232,8 @@
// if this is an animation, this thread should be blocked for 5s
// in setTransactionState waiting for transactionA to flush. Otherwise,
// the transaction should be placed on the pending queue
- if (flags & ISurfaceComposer::eAnimation) {
+ if (flags & (ISurfaceComposer::eAnimation | ISurfaceComposer::eSynchronous) ||
+ syncInputWindows) {
EXPECT_GE(systemTime(), applicationSentTime + s2ns(5));
} else {
EXPECT_LE(systemTime(), applicationSentTime + s2ns(5));
@@ -238,18 +242,9 @@
// transaction that would goes to pending transaciton queue.
mFlinger.flushTransactionQueues();
- // check that there is one binder on the pending queue.
+ // check that the transaction was applied.
auto transactionQueue = mFlinger.getPendingTransactionQueue();
- EXPECT_EQ(1u, transactionQueue.size());
-
- auto& [applyToken, transactionStates] = *(transactionQueue.begin());
- EXPECT_EQ(2u, transactionStates.size());
-
- auto& transactionStateA = transactionStates.front();
- transactionStates.pop();
- checkEqual(transactionA, transactionStateA);
- auto& transactionStateB = transactionStates.front();
- checkEqual(transactionB, transactionStateB);
+ EXPECT_EQ(0u, transactionQueue.size());
}
bool mHasListenerCallbacks = false;
@@ -262,10 +257,6 @@
// called in SurfaceFlinger::signalTransaction
EXPECT_CALL(*mMessageQueue, invalidate()).Times(1);
- // nsecs_t time = systemTime();
- EXPECT_CALL(*mVSyncTracker, nextAnticipatedVSyncTimeFrom(_))
- .WillOnce(Return(nsecs_t(5 * 1e8)))
- .WillOnce(Return(s2ns(2)));
TransactionInfo transactionA; // transaction to go on pending queue
setupSingle(transactionA, /*flags*/ 0, /*syncInputWindows*/ false,
/*desiredPresentTime*/ s2ns(1), false, FrameTimelineInfo{});