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

Commit f2f7b8f5 authored by Evan Rosky's avatar Evan Rosky
Browse files

Add global tracking of the active action 

This enables usage of chain tracking without requiring
all intermediate calls to plumb it. Eventually, this will
need to be replaced, but during the migration, this allows
incremental work.

The general expectation is that when an action is started,
it will be pushed onto a stack. Then when the work for that
action is done, we pop the action off the stack (manuall)
with a call to either end or endPartial.

The difference between end and endPartial is that end is
known to always close the whole action while endPartial
is used when the sub-action it's closing may be nested.

A single action is expected to be a synchronous/atomic block
of work -- as such, we can add a primitive form safeguarding
against mismatched start/end by ensuring that there are no
open actions on other threads (since that would imply that
action wasn't ended properly).

There is, however, an edge-case which is the trick we
currently use for asyncStart. In this case, we explicitly
exit the global lock and re-enter later. So this CL also
adds some extra infra to track mismatches across those.

Besides adding some logic, the rest of this CL is mechanical
under the following rules:
- For each chain start*, add a corresponding end
- If we leave global lock before end, add a push/popAsyncStart

Bug: 325114242
Test: existing tests (mechanical refactor)
Flag: com.android.window.flags.transit_tracker_plumbing
Change-Id: I1d146adc502d55e9586472ee02df1d973e076416
parent e7ba61fd

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

+139 −13
Original line number Diff line number Diff line
@@ -23,6 +23,8 @@ import android.util.Slog; 


import com.android.window.flags.Flags;



import java.util.ArrayList;



/**

 * Represents a chain of WM actions where each action is "caused by" the prior action (except the

 * first one of course). A whole chain is associated with one Transition (in fact, the purpose
@@ -102,7 +104,7 @@ public class ActionChain { 


    /** The transition that this chain's changes belong to. */

    @Nullable

    Transition mTransition;

    private Transition mTransition;



    /** The previous action in the chain. */

    @Nullable
@@ -124,37 +126,49 @@ public class ActionChain { 
        }

    }



    private Transition getTransition() {

    @Nullable

    Transition getTransition() {

        if (!Flags.transitTrackerPlumbing()) {

            return mTmpAtm.getTransitionController().getCollectingTransition();

            return isFinishing() ? mTransition

                    : mTmpAtm.getTransitionController().getCollectingTransition();

        }

        return mTransition;

    }



    void detachTransition() {

        mTransition = null;

    }



    boolean isFinishing() {

        return mType == TYPE_FINISH;

    }



    boolean isCollecting() {

        final Transition transition = getTransition();

        return transition != null && transition.isCollecting();

    }



    /**

     * Some common checks to determine (and report) whether this chain has a collecting transition.

     * Returns the collecting transition or {@code null} if there is an issue.

     */

    private boolean expectCollecting() {

    private Transition expectCollecting() {

        final Transition transition = getTransition();

        if (transition == null) {

            Slog.e(TAG, "Can't collect into a chain with no transition");

            return false;

            return null;

        }

        if (isFinishing()) {

            Slog.e(TAG, "Trying to collect into a finished transition");

            return false;

            return null;

        }

        if (transition.mController.getCollectingTransition() != mTransition) {

            Slog.e(TAG, "Mismatch between current collecting ("

                    + transition.mController.getCollectingTransition() + ") and chain ("

                    + transition + ")");

            return false;

            return null;

        }

        return true;

        return transition;

    }



    /**
@@ -163,8 +177,17 @@ public class ActionChain { 
     */

    void collect(@NonNull WindowContainer wc) {

        if (!wc.mTransitionController.isShellTransitionsEnabled()) return;

        if (!expectCollecting()) return;

        getTransition().collect(wc);

        final Transition transition = expectCollecting();

        if (transition == null) return;

        transition.collect(wc);

    }



    private static class AsyncStart {

        final int mStackPos;

        long mThreadId;

        AsyncStart(int stackPos) {

            mStackPos = stackPos;

        }

    }



    /**
@@ -173,16 +196,50 @@ public class ActionChain { 
    static class Tracker {

        private final ActivityTaskManagerService mAtm;



        /**

         * Track the current stack of nested chain entries within a synchronous operation. Chains

         * can nest when some entry-points are, themselves, used within the logic of another

         * entry-point.

         */

        private final ArrayList<ActionChain> mStack = new ArrayList<>();



        /** thread-id of the current action. Used to detect mismatched start/end situations. */

        private long mCurrentThread;



        /** Stack of suspended actions for dealing with async-start "gaps". */

        private final ArrayList<AsyncStart> mAsyncStarts = new ArrayList<>();



        Tracker(ActivityTaskManagerService atm) {

            mAtm = atm;

        }



        private ActionChain makeChain(String source, @LinkType int type, Transition transit) {

            final ActionChain out = new ActionChain(source, type, transit);

            int base = getThreadBase();

            if (base < mStack.size()) {

                // verify thread-id matches. This isn't a perfect check, but it should be

                // reasonably effective at detecting imbalance.

                long expectedThread = mAsyncStarts.isEmpty() ? mCurrentThread

                        : mAsyncStarts.getLast().mThreadId;

                if (Thread.currentThread().getId() != expectedThread) {

                    // This means something went wrong. Reset the stack.

                    String msg = "Likely improperly balanced ActionChain: ["

                            + mStack.get(base).mSource;

                    for (int i = (base + 1); i < mStack.size(); ++i) {

                        msg += ", " + mStack.get(i).mSource;

                    }

                    Slog.wtfStack(TAG, msg + "]");

                    mStack.subList(base, mStack.size()).clear();

                }

            } else if (!mAsyncStarts.isEmpty()) {

                mAsyncStarts.getLast().mThreadId = Thread.currentThread().getId();

            } else {

                mCurrentThread = Thread.currentThread().getId();

            }

            mStack.add(new ActionChain(source, type, transit));

            if (!Flags.transitTrackerPlumbing()) {

                out.mTmpAtm = mAtm;

                mStack.getLast().mTmpAtm = mAtm;

            }

            return out;

            return mStack.getLast();

        }



        private ActionChain makeChain(String source, @LinkType int type) {
@@ -190,6 +247,75 @@ public class ActionChain { 
                    mAtm.getTransitionController().getCollectingTransition());

        }



        /**

         * async start is the one "gap" where normally-contained actions can "interrupt"

         * an ongoing one, so detect/handle those specially.

         */

        private int getThreadBase() {

            if (mAsyncStarts.isEmpty()) return 0;

            return mAsyncStarts.getLast().mStackPos;

        }



        /**

         * There are some complicated call-paths through WM which are unnecessarily messy to plumb

         * through or which travel out of the WMS/ATMS domain (eg. into policy). For these cases,

         * we assume that as long as we still have a synchronous call stack, the same initial

         * action should apply. This means we can use a stack of "nesting" chains to associate

         * deep call-paths with their shallower counterparts.

         *

         * Starting a chain will push onto the stack, calling {@link #endPartial} will pop off the

         * stack, and calling `end` here will *clear* the stack.

         *

         * Unlike {@link #endPartial}, this `end` call is for closing a top-level session. It will

         * error if its associated start/end are, themselves, nested. This is used as a safety

         * measure to catch cases where a start is missing a corresponding end.

         *

         * @see #endPartial

         */

        void end() {

            int base = getThreadBase();

            if (mStack.size() > (base + 1)) {

                String msg = "Improperly balanced ActionChain: [" + mStack.get(base).mSource;

                for (int i = (base + 1); i < mStack.size(); ++i) {

                    msg += ", " + mStack.get(i).mSource;

                }

                Slog.wtfStack(TAG, msg + "]");

            }

            mStack.subList(base, mStack.size()).clear();

        }



        /**

         * Like {@link #end} except it just simply pops without checking if it is a root-level

         * session. This should only be used when there's a chance that the associated start/end

         * will, itself, be nested.

         *

         * @see #end

         */

        void endPartial() {

            if (mStack.isEmpty()) {

                Slog.wtfStack(TAG, "Trying to double-close action-chain");

                return;

            }

            mStack.removeLast();

        }



        /**

         * Special handling during "gaps" in atomicity while using the async-start hack. The

         * "end" tracking needs to account for this and we also want to track/report how often

         * this happens.

         */

        void pushAsyncStart() {

            if (mStack.isEmpty()) {

                Slog.wtfStack(TAG, "AsyncStart outside of chain!?");

                return;

            }

            mAsyncStarts.add(new AsyncStart(mStack.size()));

        }



        void popAsyncStart() {

            mAsyncStarts.removeLast();

        }



        /**

         * Starts tracking a normal action.

         * @see #TYPE_NORMAL

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

+2 −1
Original line number Diff line number Diff line
@@ -1285,7 +1285,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { 
        }

        if (!chain.isFinishing()) {

            throw new IllegalStateException("Can't finish on a non-finishing transition "

                    + chain.mTransition);

                    + chain.getTransition());

        }

        mLogger.mFinishTimeNs = SystemClock.elapsedRealtimeNanos();

        mController.mLoggerHandler.post(mLogger::logOnFinish);
@@ -2276,6 +2276,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { 
            mFinishTransaction.apply();

        }

        mController.finishTransition(mController.mAtm.mChainTracker.startFinish("clean-up", this));

        mController.mAtm.mChainTracker.endPartial();

    }



    private void cleanUpInternal() {

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

+2 −2
Original line number Diff line number Diff line
@@ -1008,9 +1008,9 @@ class TransitionController { 
    void finishTransition(@NonNull ActionChain chain) {

        if (!chain.isFinishing()) {

            throw new IllegalStateException("Can't finish on a non-finishing transition "

                    + chain.mTransition);

                    + chain.getTransition());

        }

        final Transition record = chain.mTransition;

        final Transition record = chain.getTransition();

        // It is usually a no-op but make sure that the metric consumer is removed.

        mTransitionMetricsReporter.reportAnimationStart(record.getToken(), 0 /* startTime */);

        // It is a no-op if the transition did not change the display.

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

+61 −43

File changed.

Preview size limit exceeded, changes collapsed.