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

Commit d8321163 authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Avoid applying unexpected sync state when checking sync finish 

For example:
A task launch transition is completed so its window finishes sync
to SYNC_STATE_NONE. And then a new activity X is launching in the
same task (given sync-group{X}). If a code path accidentally invokes
isSyncFinished to check a window that isn't in the sync-group, e.g.
the window of previous activity, then prepareSync will still be called
and change the sync state to SYNC_STATE_WAITING_FOR_DRAW.

And later when removing the window without sync-group, an exception
will be thrown by not SYNC_STATE_NONE.

So when checking isSyncFinished, if there is no sync-group from
ancestor, then behave as a getter function.

Also tweak getSyncGroup a bit to reduce method invocation.

Fix: 318642514
Test: atest SyncEngineTests#testWaitingSyncCallback
Change-Id: Idd1bc09eb9bdec1a8b0ba44fa34f417f2fa92102
parent 429020dd

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

+19 −10
Original line number Diff line number Diff line
@@ -3938,7 +3938,13 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< 
    @Nullable

    BLASTSyncEngine.SyncGroup getSyncGroup() {

        if (mSyncGroup != null) return mSyncGroup;

        if (mParent != null) return mParent.getSyncGroup();

        WindowContainer<?> parent = mParent;

        while (parent != null) {

            if (parent.mSyncGroup != null) {

                return parent.mSyncGroup;

            }

            parent = parent.mParent;

        }

        return null;

    }
@@ -3972,7 +3978,7 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< 
     * @param cancel If true, this is being finished because it is leaving the sync group rather

     *               than due to the sync group completing.

     */

    void finishSync(Transaction outMergedTransaction, BLASTSyncEngine.SyncGroup group,

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

            boolean cancel) {

        if (mSyncState == SYNC_STATE_NONE) return;

        final BLASTSyncEngine.SyncGroup syncGroup = getSyncGroup();
@@ -4000,7 +4006,8 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< 
        if (!isVisibleRequested()) {

            return true;

        }

        if (mSyncState == SYNC_STATE_NONE) {

        if (mSyncState == SYNC_STATE_NONE && getSyncGroup() != null) {

            Slog.i(TAG, "prepareSync in isSyncFinished: " + this);

            prepareSync();

        }

        if (mSyncState == SYNC_STATE_WAITING_FOR_DRAW) {
@@ -4059,16 +4066,18 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer< 
            }

            if (newParent == null) {

                // This is getting removed.

                final BLASTSyncEngine.SyncGroup syncGroup = getSyncGroup();

                if (oldParent.mSyncState != SYNC_STATE_NONE) {

                    // In order to keep the transaction in sync, merge it into the parent.

                    finishSync(oldParent.mSyncTransaction, getSyncGroup(), true /* cancel */);

                } else if (mSyncGroup != null) {

                    // This is watched directly by the sync-group, so merge this transaction into

                    // into the sync-group so it isn't lost

                    finishSync(mSyncGroup.getOrphanTransaction(), mSyncGroup, true /* cancel */);

                    finishSync(oldParent.mSyncTransaction, syncGroup, true /* cancel */);

                } else if (syncGroup != null) {

                    // This is watched by the sync-group, so merge this transaction into the

                    // sync-group for not losing the operations in the transaction.

                    finishSync(syncGroup.getOrphanTransaction(), syncGroup, true /* cancel */);

                } else {

                    throw new IllegalStateException("This container is in sync mode without a sync"

                            + " group: " + this);

                    Slog.wtf(TAG, this + " is in sync mode without a sync group");

                    // Make sure the removal transaction take effect.

                    finishSync(getPendingTransaction(), null /* group */, true /* cancel */);

                }

                return;

            } else if (mSyncGroup == null) {

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

+8 −0
Original line number Diff line number Diff line
@@ -134,6 +134,14 @@ public class SyncEngineTests extends WindowTestsBase { 
        assertFalse(r.isSyncFinished(r.getSyncGroup()));

        r.finishRelaunching();

        assertTrue(r.isSyncFinished(r.getSyncGroup()));

        assertEquals(SYNC_STATE_READY, r.mSyncState);



        // If the container has finished the sync, isSyncFinished should not change the sync state.

        final BLASTSyncEngine.SyncGroup syncGroup = r.getSyncGroup();

        r.finishSync(mTransaction, syncGroup, false /* cancel */);

        assertEquals(SYNC_STATE_NONE, r.mSyncState);

        assertTrue(r.isSyncFinished(syncGroup));

        assertEquals(SYNC_STATE_NONE, r.mSyncState);

    }



    @Test