Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8c1b977d authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Unset sync group if sync state does not set 

An extreme case when device is busy and slow, e.g. stress test by
script or monkey, the AsyncRotationController could be null when
calling TransitionController#collectForDisplayAreaChange, then
WindowToken#prepareSync skips the async windows without setting
sync state but sync group is still assigned. That results the
WindowContainer#finishSync not cleaning the group. And its sync
state of child windows will be set when calling isSyncFinished
("prepareSync in isSyncFinished"), which causes the window to
always use sync transaction.

Also add more log to indicate the corner cases.

Bug: 334838294
Flag: EXEMPT bugfix
Test: atest SyncEngineTests#testSkipPrepareSync

Change-Id: I35f8a1aa728ee5b66e4e9d6136f9761ed6e989bf
parent 733b315a

services/core/java/com/android/server/wm/ActivityRecord.java

+1 −2
Original line number Diff line number Diff line
@@ -11157,8 +11157,7 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A 
            boolean cancel) {

        // This override is just for getting metrics. allFinished needs to be checked before

        // finish because finish resets all the states.

        final BLASTSyncEngine.SyncGroup syncGroup = getSyncGroup();

        if (syncGroup != null && group != getSyncGroup()) return;

        if (isDifferentSyncGroup(group)) return;

        mLastAllReadyAtSync = allSyncFinished();

        super.finishSync(outMergedTransaction, group, cancel);

    }

services/core/java/com/android/server/wm/BLASTSyncEngine.java

+5 −0
Original line number Diff line number Diff line
@@ -348,6 +348,11 @@ class BLASTSyncEngine { 
                wc.setSyncGroup(this);

            }

            wc.prepareSync();

            if (wc.mSyncState == WindowContainer.SYNC_STATE_NONE && wc.mSyncGroup != null) {

                Slog.w(TAG, "addToSync: unset SyncGroup " + wc.mSyncGroup.mSyncId

                        + " for non-sync " + wc);

                wc.mSyncGroup = null;

            }

            if (mReady) {

                mWm.mWindowPlacerLocked.requestTraversal();

            }

services/core/java/com/android/server/wm/WindowContainer.java

+21 −4
Original line number Diff line number Diff line
@@ -3985,6 +3985,19 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< 
        return mSyncState != SYNC_STATE_NONE;

    }



    /**

     * Returns {@code true} if this window container belongs to a different sync group than the

     * given group.

     */

    boolean isDifferentSyncGroup(@Nullable BLASTSyncEngine.SyncGroup group) {

        if (group == null) return false;

        final BLASTSyncEngine.SyncGroup thisGroup = getSyncGroup();

        if (thisGroup == null || group == thisGroup) return false;

        Slog.d(TAG, this + " uses a different SyncGroup, current=" + thisGroup.mSyncId

                + " given=" + group.mSyncId);

        return true;

    }



    /**

     * Recursively finishes/cleans-up sync state of this subtree and collects all the sync

     * transactions into `outMergedTransaction`.
@@ -3994,10 +4007,14 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< 
     */

    void finishSync(Transaction outMergedTransaction, @Nullable BLASTSyncEngine.SyncGroup group,

            boolean cancel) {

        if (mSyncState == SYNC_STATE_NONE) return;

        final BLASTSyncEngine.SyncGroup syncGroup = getSyncGroup();

        // If it's null, then we need to clean-up anyways.

        if (syncGroup != null && group != syncGroup) return;

        if (mSyncState == SYNC_STATE_NONE) {

            if (mSyncGroup != null) {

                Slog.e(TAG, "finishSync: stale group " + mSyncGroup.mSyncId + " of " + this);

                mSyncGroup = null;

            }

            return;

        }

        if (isDifferentSyncGroup(group)) return;

        ProtoLog.v(WM_DEBUG_SYNC_ENGINE, "finishSync cancel=%b for %s", cancel, this);

        outMergedTransaction.merge(mSyncTransaction);

        for (int i = mChildren.size() - 1; i >= 0; --i) {

services/core/java/com/android/server/wm/WindowState.java

+1 −2
Original line number Diff line number Diff line
@@ -5791,8 +5791,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP 
    @Override

    void finishSync(Transaction outMergedTransaction, BLASTSyncEngine.SyncGroup group,

            boolean cancel) {

        final BLASTSyncEngine.SyncGroup syncGroup = getSyncGroup();

        if (syncGroup != null && group != syncGroup) return;

        if (isDifferentSyncGroup(group)) return;

        mPrepareSyncSeqId = 0;

        if (cancel) {

            // This is leaving sync so any buffers left in the sync have a chance of

services/tests/wmtests/src/com/android/server/wm/SyncEngineTests.java

+21 −0
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import static com.android.server.wm.WindowState.BLAST_TIMEOUT_DURATION; 


import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyInt;
@@ -416,6 +417,22 @@ public class SyncEngineTests extends WindowTestsBase { 
        assertEquals(SYNC_STATE_NONE, botChildWC.mSyncState);

    }



    @Test

    public void testSkipPrepareSync() {

        final TestWindowContainer wc = new TestWindowContainer(mWm, true /* waiter */);

        wc.mSkipPrepareSync = true;

        final BLASTSyncEngine bse = createTestBLASTSyncEngine();

        final BLASTSyncEngine.SyncGroup syncGroup = bse.prepareSyncSet(

                mock(BLASTSyncEngine.TransactionReadyListener.class), "test");

        bse.startSyncSet(syncGroup);

        bse.addToSyncSet(syncGroup.mSyncId, wc);

        assertEquals(SYNC_STATE_NONE, wc.mSyncState);

        // If the implementation of prepareSync doesn't set sync state, the sync group should also

        // be empty.

        assertNull(wc.mSyncGroup);

        assertTrue(wc.isSyncFinished(syncGroup));

    }



    @Test

    public void testNonBlastMethod() {

        mAppWindow = createWindow(null, TYPE_BASE_APPLICATION, "mAppWindow");
@@ -694,6 +711,7 @@ public class SyncEngineTests extends WindowTestsBase { 
        final boolean mWaiter;

        boolean mVisibleRequested = true;

        boolean mFillsParent = false;

        boolean mSkipPrepareSync = false;



        TestWindowContainer(WindowManagerService wms, boolean waiter) {

            super(wms);
@@ -703,6 +721,9 @@ public class SyncEngineTests extends WindowTestsBase { 


        @Override

        boolean prepareSync() {

            if (mSkipPrepareSync) {

                return false;

            }

            if (!super.prepareSync()) {

                return false;

            }