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

Commit 6914f083 authored by Robert Carr's avatar Robert Carr
Browse files

Fix various flashes when moving stacks. 

In this CL we fix two new pinned stack reparenting flashes and
implement a new approach to an old docked stack flash fix, which had
been broken in refactoring.

First we examine the case of dismissing the docked stack and
WindowState#notifyMovedInStack. Previously we invoked this
when reparenting from the docked to the fullscreen stack
(by way of position in stack). It was used to solve an issue
where we were visually hidden by the docked stack crop, but we were
still waiting on an animation pass to set the hidden flag. Our old solution
was if mJustMovedInStack was set, we would just defer updating our crop until
one animation pass had occurred.

We broke this incidentally in refactoring by not calling the
method that sets it anymore. However it's somewhat brittle so I was
hesitant to restore it. The fundamental requirement is for the
ActivityManager to perform multiple operations (change stack, update
visibility) in a single atomic step and this wasn't expressed clearly.

This mirrors some challenges we have with the pinned stack transitions
as well.

1. When dismissing the pinned stack, we move the task to the
fullscreen stack. We need a mechanism to prevent its bounds from
updating before its visibility is updated.
2. When moving to fullscreen while over home, we have layering issues
with the home stack, as we will be moved to the fullscreen stack before the
fullscreen stack is brought to the front of the home stack. This may
not seem like a visibility issue, but if the home activity were simply
hidden the layering wouldn't matter!

Evidently, all three of these issues can be solved with a batching
mechanism from ActivityManager to WindowManager. As all changes are
ultimately Surface changes, SurfaceControl.open/closeTransaction
provides such a mechanism. The only additional complication is that
normally visibility updates on SurfaceControl are deferred to the
animation thread, which may not execute within the bounds of our
transaction. This however, is easily dealt with: In AppWindowToken, if
we are becoming hidden without animation, then we simply apply this
change without waiting for the UI thread

Bug: 35396882
Bug: 34857388
Bug: 36393204
Bug: 36462635
Test: Intensive manual testing of dismissing docked and pinned stack + pinned->fullscreen transition.
Change-Id: Ic110797670cc7ff656a580fd186d4deb44fa54dd
parent 71e06d9d

services/core/java/com/android/server/am/ActivityStackSupervisor.java

+35 −7
Original line number Diff line number Diff line
@@ -2287,7 +2287,18 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D 
        mResizingTasksDuringAnimation.clear();

    }



    void moveTasksToFullscreenStackLocked(int fromStackId, boolean onTop) {

    private class MoveTaskToFullscreenArgs {

        public int fromStackId;

        public boolean onTop;

    };

    // Used only to closure over the arguments to moveTasksToFullscreenStack without

    // allocation

    private MoveTaskToFullscreenArgs mMoveToFullscreenArgs = new MoveTaskToFullscreenArgs();



    private void moveTasksToFullscreenStackInnerLocked() {

        int fromStackId = mMoveToFullscreenArgs.fromStackId;

        boolean onTop = mMoveToFullscreenArgs.onTop;



        final ActivityStack stack = getStack(fromStackId);

        if (stack == null) {

            return;
@@ -2359,6 +2370,13 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D 
        }

    }



    void moveTasksToFullscreenStackLocked(int fromStackId, boolean onTop) {

        mMoveToFullscreenArgs.fromStackId = fromStackId;

        mMoveToFullscreenArgs.onTop = onTop;



        mWindowManager.inSurfaceTransaction(this::moveTasksToFullscreenStackInnerLocked);

    }



    void resizeDockedStackLocked(Rect dockedBounds, Rect tempDockedTaskBounds,

            Rect tempDockedTaskInsetBounds, Rect tempOtherTaskBounds, Rect tempOtherTaskInsetBounds,

            boolean preserveWindows) {
@@ -2471,12 +2489,12 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D 
        return activityContainer.mStack;

    }



    /**

     * Removes the stack associated with the given {@param stackId}.  If the {@param stackId} is the

     * pinned stack, then its tasks are not explicitly removed when the stack is destroyed, but

     * instead moved back onto the fullscreen stack.

     */

    void removeStackLocked(int stackId) {



    // Used only to closure over the argument to removeStack without allocation.

    private int mRemoveStackStackId;

    void removeStackInnerLocked() {

        int stackId = mRemoveStackStackId;



        final ActivityStack stack = getStack(stackId);

        if (stack == null) {

            return;
@@ -2514,6 +2532,16 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D 
        }

    }



    /**

     * Removes the stack associated with the given {@param stackId}.  If the {@param stackId} is the

     * pinned stack, then its tasks are not explicitly removed when the stack is destroyed, but

     * instead moved back onto the fullscreen stack.

     */

    void removeStackLocked(int stackId) {

        mRemoveStackStackId = stackId;

        mWindowManager.inSurfaceTransaction(this::removeStackInnerLocked);

    }



    /**

     * Removes the task with the specified task id.

     *

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

+0 −5
Original line number Diff line number Diff line
@@ -161,11 +161,6 @@ public class AppWindowAnimator { 
        } else {

            mClearProlongedAnimation = true;

        }



        // Since we are finally starting our animation, we don't need the logic anymore to prevent

        // the app from showing again if we just moved between stacks.

        // See {@link WindowState#notifyMovedInStack}.

        mAppToken.resetJustMovedInStack();

    }



    public void setDummyAnimation() {

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

+8 −13
Original line number Diff line number Diff line
@@ -55,6 +55,7 @@ import android.os.IBinder; 
import android.os.SystemClock;

import android.util.Slog;

import android.view.IApplicationToken;

import android.view.SurfaceControl;

import android.view.WindowManager;

import android.view.WindowManagerPolicy.StartingSurface;
@@ -365,6 +366,13 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree 
                mEnteringAnimation = true;

                mService.mActivityManagerAppTransitionNotifier.onAppTransitionFinishedLocked(token);

            }

            if (hidden && !delayed) {

                SurfaceControl.openTransaction();

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

                    mChildren.get(i).mWinAnimator.hide("immediately hidden");

                }

                SurfaceControl.closeTransaction();

            }



            if (!mService.mClosingApps.contains(this) && !mService.mOpeningApps.contains(this)) {

                // The token is not closing nor opening, so even if there is an animation set, that
@@ -967,19 +975,6 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree 
        mService.mWindowPlacerLocked.performSurfacePlacement();

    }



    void resetJustMovedInStack() {

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

            (mChildren.get(i)).resetJustMovedInStack();

        }

    }



    void notifyMovedInStack() {

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

            final WindowState win = mChildren.get(winNdx);

            win.notifyMovedInStack();

        }

    }



    void setAppLayoutChanges(int changes, String reason) {

        if (!mChildren.isEmpty()) {

            final DisplayContent dc = getDisplayContent();

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

+0 −4
Original line number Diff line number Diff line
@@ -208,10 +208,6 @@ class Task extends WindowContainer<AppWindowToken> implements DimLayer.DimLayerU 
    void positionAt(int position, Rect bounds, Configuration overrideConfig) {

        mStack.positionChildAt(position, this, false /* includingParents */);

        resizeLocked(bounds, overrideConfig, false /* force */);



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

            mChildren.get(activityNdx).notifyMovedInStack();

        }

    }



    @Override

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

+45 −0
Original line number Diff line number Diff line
@@ -7306,4 +7306,49 @@ public class WindowManagerService extends IWindowManager.Stub 
        mAppFreezeListeners.remove(listener);

    }



    /**

     * WARNING: This interrupts surface updates, be careful! Don't

     * execute within the transaction for longer than you would

     * execute on an animation thread.

     * WARNING: This holds the WindowManager lock, so if exec will acquire

     * the ActivityManager lock, you should hold it BEFORE calling this

     * otherwise there is a risk of deadlock if another thread holding the AM

     * lock waits on the WM lock.

     * WARNING: This method contains locks known to the State of California

     * to cause Deadlocks and other conditions.

     *

     *

     * Begins a surface transaction with which the AM can batch operations.

     * All Surface updates performed by the WindowManager following this

     * will not appear on screen until after the call to

     * closeSurfaceTransaction.

     *

     * ActivityManager can use this to ensure multiple 'commands' will all

     * be reflected in a single frame. For example when reparenting a window

     * which was previously hidden due to it's parent properties, we may

     * need to ensure it is hidden in the same frame that the properties

     * from the new parent are inherited, otherwise it could be revealed

     * mistakenly.

     *

     *

     * TODO(b/36393204): We can investigate totally replacing #deferSurfaceLayout

     * with something like this but it seems that some existing cases of

     * deferSurfaceLayout may be a little too broad, in particular the total

     * enclosure of startActivityUnchecked which could run for quite some time.

     */

    public void inSurfaceTransaction(Runnable exec) {

        // We hold the WindowManger lock to ensure relayoutWindow

        // does not return while a Surface transaction is opening.

        // The client depends on us to have resized the surface

        // by that point (b/36462635)



        synchronized (mWindowMap) {

            SurfaceControl.openTransaction();

            try {

                exec.run();

            } finally {

                SurfaceControl.closeTransaction();

            }

        }

    }

}