Push existing pending state when deferring transaction

Otherwise the information from another transaction that happened
before deferring the transaction could have been deferred as well.
To fix that, we push the currently pending state if the
transaction is deferred.

Furthermore, we need to modify the behavior how flags are applied.
Imagine the following sequence of events
- Surface is hidden flags=hidden
- t1 doesn't modify the flags (0/0) but sets some other properties.
flags=hidden mask=0. mCurrentState gets pushed onto mPendingState
before t2 sets (0/hidden) (flags/mask) flags=0 mask=hidden, to
show the surface. Furthemore, we defer this transaction.
mCurrentState gets pushed onto mPendingState.
- At this point, mCurrentState.flags = 0 (shown), but
mDrawingState.flags = hidden
- doTransaction happens. c (the state to promote to drawing state)
= mCurrentState => c.flags = 0 (shown)
Since t1 isn't deferred, the 0th element of mPendingState gets
popped and applied to c. Since mask=0, c.flags = 0 still.
- c gets promoted to mDrawingState and BOOM the
surface was shown even though the barrier of the transaction
showing hasn't opened yet.

Looking closer at the logic it makes sense to just apply the mask
when modifying the current state, and then just pushing it like
all other attributes.

Test: go/wm-smoke
Bug: 78611607
Change-Id: Ia100532189ef7f59f8939c59db9b6292b41e8e77

3 files changed, 8 insertions, 6 deletions

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index b94af77967..83d08a2322 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -920,10 +920,7 @@ void Layer::pushPendingState() {
 }
 
 void Layer::popPendingState(State* stateToCommit) {
-    auto oldFlags = stateToCommit->flags;
     *stateToCommit = mPendingStates[0];
-    stateToCommit->flags =
-            (oldFlags & ~stateToCommit->mask) | (stateToCommit->flags & stateToCommit->mask);
 
     mPendingStates.removeAt(0);
     ATRACE_INT(mTransactionName.string(), mPendingStates.size());
@@ -1270,7 +1267,6 @@ bool Layer::setFlags(uint8_t flags, uint8_t mask) {
     if (mCurrentState.flags == newFlags) return false;
     mCurrentState.sequence++;
     mCurrentState.flags = newFlags;
-    mCurrentState.mask = mask;
     mCurrentState.modified = true;
     setTransactionFlags(eTransactionNeeded);
     return true;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 7342c8b987..ae80043a5d 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -186,7 +186,6 @@ public:
         uint32_t layerStack;
 
         uint8_t flags;
-        uint8_t mask;
         uint8_t reserved[2];
         int32_t sequence; // changes when visible regions can change
         bool modified;
@@ -588,6 +587,7 @@ public:
     // SurfaceFlinger to complete a transaction.
     void commitChildList();
     int32_t getZ() const;
+    void pushPendingState();
 
 protected:
     // constant
@@ -670,7 +670,6 @@ protected:
     // Returns false if the relevant frame has already been latched
     bool addSyncPoint(const std::shared_ptr<SyncPoint>& point);
 
-    void pushPendingState();
     void popPendingState(State* stateToCommit);
     bool applyPendingStates(State* stateToCommit);
 
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a120738713..730bdcd917 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3345,6 +3345,13 @@ uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState
     const uint32_t what = s.what;
     bool geometryAppliesWithResize =
             what & layer_state_t::eGeometryAppliesWithResize;
+
+    // If we are deferring transaction, make sure to push the pending state, as otherwise the
+    // pending state will also be deferred.
+    if (what & layer_state_t::eDeferTransaction) {
+        layer->pushPendingState();
+    }
+
     if (what & layer_state_t::ePositionChanged) {
         if (layer->setPosition(s.x, s.y, !geometryAppliesWithResize)) {
             flags |= eTraversalNeeded;