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

Commit a8f07a7f authored by chaviw's avatar chaviw
Browse files

Fixed a few issues with starting windows

1. startingData was accessed on a different thread without a lock held.
This caused synchronization issues where startingData could be set to
null while the starting window was getting created.

2. In onFirstWindowDrawn, startingData was not being set to null if the
startingWindow didn't exist. That means if the startingWindow hadn't
been created yet, it would get created after the first window was drawn.

Fixes: 129654434
Test: Steps from bug no longer reproducible
Change-Id: Ic5086798082b9f312cbd5069a937eac95cff7a9c
parent 8e24abad
Loading
Loading
Loading
Loading
+1 −1
Original line number Original line Diff line number Diff line
@@ -478,7 +478,7 @@ public class AppTransitionController {
                    outReasons.put(windowingMode,  APP_TRANSITION_WINDOWS_DRAWN);
                    outReasons.put(windowingMode,  APP_TRANSITION_WINDOWS_DRAWN);
                } else {
                } else {
                    outReasons.put(windowingMode,
                    outReasons.put(windowingMode,
                            wtoken.startingData instanceof SplashScreenStartingData
                            wtoken.mStartingData instanceof SplashScreenStartingData
                                    ? APP_TRANSITION_SPLASH_SCREEN
                                    ? APP_TRANSITION_SPLASH_SCREEN
                                    : APP_TRANSITION_SNAPSHOT);
                                    : APP_TRANSITION_SNAPSHOT);
                }
                }
+34 −30
Original line number Original line Diff line number Diff line
@@ -203,7 +203,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
    boolean removed;
    boolean removed;


    // Information about an application starting window if displayed.
    // Information about an application starting window if displayed.
    StartingData startingData;
    StartingData mStartingData;
    WindowState startingWindow;
    WindowState startingWindow;
    StartingSurface startingSurface;
    StartingSurface startingSurface;
    boolean startingDisplayed;
    boolean startingDisplayed;
@@ -385,8 +385,8 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
            // it from behind the starting window, so there is no need for it to also be doing its
            // it from behind the starting window, so there is no need for it to also be doing its
            // own stuff.
            // own stuff.
            win.cancelAnimation();
            win.cancelAnimation();
            removeStartingWindow();
        }
        }
        removeStartingWindow();
        updateReportedVisibilityLocked();
        updateReportedVisibilityLocked();
    }
    }


@@ -874,7 +874,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
        if (DEBUG_ADD_REMOVE || DEBUG_TOKEN_MOVEMENT) Slog.v(TAG_WM, "removeAppToken: "
        if (DEBUG_ADD_REMOVE || DEBUG_TOKEN_MOVEMENT) Slog.v(TAG_WM, "removeAppToken: "
                + this + " delayed=" + delayed + " Callers=" + Debug.getCallers(4));
                + this + " delayed=" + delayed + " Callers=" + Debug.getCallers(4));


        if (startingData != null) {
        if (mStartingData != null) {
            removeStartingWindow();
            removeStartingWindow();
        }
        }


@@ -1053,7 +1053,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
            // If this is the last window and we had requested a starting transition window,
            // If this is the last window and we had requested a starting transition window,
            // well there is no point now.
            // well there is no point now.
            if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Nulling last startingData");
            if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Nulling last startingData");
            startingData = null;
            mStartingData = null;
            if (mHiddenSetFromTransferredStartingWindow) {
            if (mHiddenSetFromTransferredStartingWindow) {
                // We set the hidden state to false for the token from a transferred starting window.
                // We set the hidden state to false for the token from a transferred starting window.
                // We now reset it back to true since the starting window was the last window in the
                // We now reset it back to true since the starting window was the last window in the
@@ -1486,13 +1486,13 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
            final long origId = Binder.clearCallingIdentity();
            final long origId = Binder.clearCallingIdentity();
            try {
            try {
                // Transfer the starting window over to the new token.
                // Transfer the starting window over to the new token.
                startingData = fromToken.startingData;
                mStartingData = fromToken.mStartingData;
                startingSurface = fromToken.startingSurface;
                startingSurface = fromToken.startingSurface;
                startingDisplayed = fromToken.startingDisplayed;
                startingDisplayed = fromToken.startingDisplayed;
                fromToken.startingDisplayed = false;
                fromToken.startingDisplayed = false;
                startingWindow = tStartingWindow;
                startingWindow = tStartingWindow;
                reportedVisible = fromToken.reportedVisible;
                reportedVisible = fromToken.reportedVisible;
                fromToken.startingData = null;
                fromToken.mStartingData = null;
                fromToken.startingSurface = null;
                fromToken.startingSurface = null;
                fromToken.startingWindow = null;
                fromToken.startingWindow = null;
                fromToken.startingMoved = true;
                fromToken.startingMoved = true;
@@ -1538,13 +1538,13 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
                Binder.restoreCallingIdentity(origId);
                Binder.restoreCallingIdentity(origId);
            }
            }
            return true;
            return true;
        } else if (fromToken.startingData != null) {
        } else if (fromToken.mStartingData != null) {
            // The previous app was getting ready to show a
            // The previous app was getting ready to show a
            // starting window, but hasn't yet done so.  Steal it!
            // starting window, but hasn't yet done so.  Steal it!
            if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM,
            if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM,
                    "Moving pending starting from " + fromToken + " to " + this);
                    "Moving pending starting from " + fromToken + " to " + this);
            startingData = fromToken.startingData;
            mStartingData = fromToken.mStartingData;
            fromToken.startingData = null;
            fromToken.mStartingData = null;
            fromToken.startingMoved = true;
            fromToken.startingMoved = true;
            scheduleAddStartingWindow();
            scheduleAddStartingWindow();
            return true;
            return true;
@@ -2042,7 +2042,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
            return false;
            return false;
        }
        }


        if (startingData != null) {
        if (mStartingData != null) {
            return false;
            return false;
        }
        }


@@ -2123,7 +2123,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
        }
        }


        if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Creating SplashScreenStartingData");
        if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Creating SplashScreenStartingData");
        startingData = new SplashScreenStartingData(mWmService, pkg,
        mStartingData = new SplashScreenStartingData(mWmService, pkg,
                theme, compatInfo, nonLocalizedLabel, labelRes, icon, logo, windowFlags,
                theme, compatInfo, nonLocalizedLabel, labelRes, icon, logo, windowFlags,
                getMergedOverrideConfiguration());
                getMergedOverrideConfiguration());
        scheduleAddStartingWindow();
        scheduleAddStartingWindow();
@@ -2137,7 +2137,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
        }
        }


        if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Creating SnapshotStartingData");
        if (DEBUG_STARTING_WINDOW) Slog.v(TAG_WM, "Creating SnapshotStartingData");
        startingData = new SnapshotStartingData(mWmService, snapshot);
        mStartingData = new SnapshotStartingData(mWmService, snapshot);
        scheduleAddStartingWindow();
        scheduleAddStartingWindow();
        return true;
        return true;
    }
    }
@@ -2156,12 +2156,13 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree


        @Override
        @Override
        public void run() {
        public void run() {
            // Can be accessed without holding the global lock
            final StartingData startingData;
            synchronized (mWmService.mGlobalLock) {
            synchronized (mWmService.mGlobalLock) {
                // There can only be one adding request, silly caller!
                // There can only be one adding request, silly caller!
                mWmService.mAnimationHandler.removeCallbacks(this);
                mWmService.mAnimationHandler.removeCallbacks(this);
            }


            if (startingData == null) {
                if (mStartingData == null) {
                    // Animation has been canceled... do nothing.
                    // Animation has been canceled... do nothing.
                    if (DEBUG_STARTING_WINDOW) {
                    if (DEBUG_STARTING_WINDOW) {
                        Slog.v(TAG, "startingData was nulled out before handling"
                        Slog.v(TAG, "startingData was nulled out before handling"
@@ -2169,6 +2170,8 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
                    }
                    }
                    return;
                    return;
                }
                }
                startingData = mStartingData;
            }


            if (DEBUG_STARTING_WINDOW) {
            if (DEBUG_STARTING_WINDOW) {
                Slog.v(TAG, "Add starting " + this + ": startingData=" + startingData);
                Slog.v(TAG, "Add starting " + this + ": startingData=" + startingData);
@@ -2185,19 +2188,20 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
                synchronized (mWmService.mGlobalLock) {
                synchronized (mWmService.mGlobalLock) {
                    // If the window was successfully added, then
                    // If the window was successfully added, then
                    // we need to remove it.
                    // we need to remove it.
                    if (removed || startingData == null) {
                    if (removed || mStartingData == null) {
                        if (DEBUG_STARTING_WINDOW) {
                        if (DEBUG_STARTING_WINDOW) {
                            Slog.v(TAG, "Aborted starting " + AppWindowToken.this
                            Slog.v(TAG, "Aborted starting " + AppWindowToken.this
                                    + ": removed=" + removed + " startingData=" + startingData);
                                    + ": removed=" + removed + " startingData=" + mStartingData);
                        }
                        }
                        startingWindow = null;
                        startingWindow = null;
                        startingData = null;
                        mStartingData = null;
                        abort = true;
                        abort = true;
                    } else {
                    } else {
                        startingSurface = surface;
                        startingSurface = surface;
                    }
                    }
                    if (DEBUG_STARTING_WINDOW && !abort) {
                    if (DEBUG_STARTING_WINDOW && !abort) {
                        Slog.v(TAG, "Added starting " + AppWindowToken.this + ": startingWindow="
                        Slog.v(TAG,
                                "Added starting " + AppWindowToken.this + ": startingWindow="
                                        + startingWindow + " startingView=" + startingSurface);
                                        + startingWindow + " startingView=" + startingSurface);
                    }
                    }
                }
                }
@@ -2245,21 +2249,21 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree


    void removeStartingWindow() {
    void removeStartingWindow() {
        if (startingWindow == null) {
        if (startingWindow == null) {
            if (startingData != null) {
            if (mStartingData != null) {
                // Starting window has not been added yet, but it is scheduled to be added.
                // Starting window has not been added yet, but it is scheduled to be added.
                // Go ahead and cancel the request.
                // Go ahead and cancel the request.
                if (DEBUG_STARTING_WINDOW) {
                if (DEBUG_STARTING_WINDOW) {
                    Slog.v(TAG_WM, "Clearing startingData for token=" + this);
                    Slog.v(TAG_WM, "Clearing startingData for token=" + this);
                }
                }
                startingData = null;
                mStartingData = null;
            }
            }
            return;
            return;
        }
        }


        final WindowManagerPolicy.StartingSurface surface;
        final WindowManagerPolicy.StartingSurface surface;
        if (startingData != null) {
        if (mStartingData != null) {
            surface = startingSurface;
            surface = startingSurface;
            startingData = null;
            mStartingData = null;
            startingSurface = null;
            startingSurface = null;
            startingWindow = null;
            startingWindow = null;
            startingDisplayed = false;
            startingDisplayed = false;
@@ -2994,8 +2998,8 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
            pw.print(prefix); pw.print("inPendingTransaction=");
            pw.print(prefix); pw.print("inPendingTransaction=");
                    pw.println(inPendingTransaction);
                    pw.println(inPendingTransaction);
        }
        }
        if (startingData != null || removed || firstWindowDrawn || mIsExiting) {
        if (mStartingData != null || removed || firstWindowDrawn || mIsExiting) {
            pw.print(prefix); pw.print("startingData="); pw.print(startingData);
            pw.print(prefix); pw.print("startingData="); pw.print(mStartingData);
                    pw.print(" removed="); pw.print(removed);
                    pw.print(" removed="); pw.print(removed);
                    pw.print(" firstWindowDrawn="); pw.print(firstWindowDrawn);
                    pw.print(" firstWindowDrawn="); pw.print(firstWindowDrawn);
                    pw.print(" mIsExiting="); pw.println(mIsExiting);
                    pw.print(" mIsExiting="); pw.println(mIsExiting);
+2 −2
Original line number Original line Diff line number Diff line
@@ -504,14 +504,14 @@ public class AppWindowTokenTests extends WindowTestsBase {


    private void assertHasStartingWindow(AppWindowToken atoken) {
    private void assertHasStartingWindow(AppWindowToken atoken) {
        assertNotNull(atoken.startingSurface);
        assertNotNull(atoken.startingSurface);
        assertNotNull(atoken.startingData);
        assertNotNull(atoken.mStartingData);
        assertNotNull(atoken.startingWindow);
        assertNotNull(atoken.startingWindow);
    }
    }


    private void assertNoStartingWindow(AppWindowToken atoken) {
    private void assertNoStartingWindow(AppWindowToken atoken) {
        assertNull(atoken.startingSurface);
        assertNull(atoken.startingSurface);
        assertNull(atoken.startingWindow);
        assertNull(atoken.startingWindow);
        assertNull(atoken.startingData);
        assertNull(atoken.mStartingData);
        atoken.forAllWindows(windowState -> {
        atoken.forAllWindows(windowState -> {
            assertFalse(windowState.getBaseType() == TYPE_APPLICATION_STARTING);
            assertFalse(windowState.getBaseType() == TYPE_APPLICATION_STARTING);
        }, true);
        }, true);