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
diff --git a/services/core/java/com/android/server/am/ActivityStackSupervisor.java b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
index c1bff36..1116fc3 100644
--- a/services/core/java/com/android/server/am/ActivityStackSupervisor.java
+++ b/services/core/java/com/android/server/am/ActivityStackSupervisor.java
@@ -2287,7 +2287,18 @@
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 @@
}
}
+ 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 @@
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;
@@ -2515,6 +2533,16 @@
}
/**
+ * 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.
*
* @param taskId Identifier of the task to be removed.
diff --git a/services/core/java/com/android/server/wm/AppWindowAnimator.java b/services/core/java/com/android/server/wm/AppWindowAnimator.java
index e3941b9..16edd35 100644
--- a/services/core/java/com/android/server/wm/AppWindowAnimator.java
+++ b/services/core/java/com/android/server/wm/AppWindowAnimator.java
@@ -161,11 +161,6 @@
} 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() {
diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java
index c20ee97..a474316 100644
--- a/services/core/java/com/android/server/wm/AppWindowToken.java
+++ b/services/core/java/com/android/server/wm/AppWindowToken.java
@@ -55,6 +55,7 @@
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 @@
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 @@
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();
diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java
index 99c085f..9e4d60a 100644
--- a/services/core/java/com/android/server/wm/Task.java
+++ b/services/core/java/com/android/server/wm/Task.java
@@ -208,10 +208,6 @@
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
diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java
index 5844b0b..3fc7d83 100644
--- a/services/core/java/com/android/server/wm/WindowManagerService.java
+++ b/services/core/java/com/android/server/wm/WindowManagerService.java
@@ -7306,4 +7306,49 @@
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();
+ }
+ }
+ }
}
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index ca5d551..d4c8b1f 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -517,11 +517,6 @@
final private Rect mTmpRect = new Rect();
/**
- * See {@link #notifyMovedInStack}.
- */
- private boolean mJustMovedInStack;
-
- /**
* Whether the window was resized by us while it was gone for layout.
*/
boolean mResizedWhileGone = false;
@@ -1998,49 +1993,6 @@
}
}
- /**
- * Notifies this window that the corresponding task has just moved in the stack.
- * <p>
- * This is used to fix the following: If we moved in the stack, and if the last clip rect was
- * empty, meaning that our task was completely offscreen, we need to keep it invisible because
- * the actual app transition that updates the visibility is delayed by a few transactions.
- * Instead of messing around with the ordering and timing how transitions and transactions are
- * executed, we introduce this little hack which prevents this window of getting visible again
- * with the wrong bounds until the app transitions has started.
- * <p>
- * This method notifies the window about that we just moved in the stack so we can apply this
- * logic in {@link WindowStateAnimator#updateSurfaceWindowCrop}
- */
- void notifyMovedInStack() {
- mJustMovedInStack = true;
-
- for (int i = mChildren.size() - 1; i >= 0; --i) {
- final WindowState c = mChildren.get(i);
- c.notifyMovedInStack();
- }
- }
-
- /**
- * See {@link #notifyMovedInStack}.
- *
- * @return Whether we just got moved in the corresponding stack.
- */
- boolean hasJustMovedInStack() {
- return mJustMovedInStack;
- }
-
- /**
- * Resets that we just moved in the corresponding stack. See {@link #notifyMovedInStack}.
- */
- void resetJustMovedInStack() {
- mJustMovedInStack = false;
-
- for (int i = mChildren.size() - 1; i >= 0; i--) {
- final WindowState c = mChildren.get(i);
- c.resetJustMovedInStack();
- }
- }
-
private final class DeadWindowEventReceiver extends InputEventReceiver {
DeadWindowEventReceiver(InputChannel inputChannel) {
super(inputChannel, mService.mH.getLooper());
diff --git a/services/core/java/com/android/server/wm/WindowStateAnimator.java b/services/core/java/com/android/server/wm/WindowStateAnimator.java
index 4b71338..48de7e4 100644
--- a/services/core/java/com/android/server/wm/WindowStateAnimator.java
+++ b/services/core/java/com/android/server/wm/WindowStateAnimator.java
@@ -1177,10 +1177,6 @@
w.transformClipRectFromScreenToSurfaceSpace(clipRect);
- // See {@link WindowState#notifyMovedInStack} for why this is necessary.
- if (w.hasJustMovedInStack() && mLastClipRect.isEmpty() && !clipRect.isEmpty()) {
- clipRect.setEmpty();
- }
return true;
}