OSDN Git Service

Fix various flashes when moving stacks.
authorRobert Carr <racarr@google.com>
Tue, 21 Mar 2017 02:04:30 +0000 (19:04 -0700)
committerRobert Carr <racarr@google.com>
Tue, 21 Mar 2017 20:38:08 +0000 (13:38 -0700)
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

services/core/java/com/android/server/am/ActivityStackSupervisor.java
services/core/java/com/android/server/wm/AppWindowAnimator.java
services/core/java/com/android/server/wm/AppWindowToken.java
services/core/java/com/android/server/wm/Task.java
services/core/java/com/android/server/wm/WindowManagerService.java
services/core/java/com/android/server/wm/WindowState.java
services/core/java/com/android/server/wm/WindowStateAnimator.java

index c1bff36..1116fc3 100644 (file)
@@ -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;
@@ -2515,6 +2533,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.
      *
      * @param taskId Identifier of the task to be removed.
index e3941b9..16edd35 100644 (file)
@@ -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() {
index c20ee97..a474316 100644 (file)
@@ -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();
index 99c085f..9e4d60a 100644 (file)
@@ -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
index 5844b0b..3fc7d83 100644 (file)
@@ -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();
+            }
+        }
+    }
 }
index ca5d551..d4c8b1f 100644 (file)
@@ -517,11 +517,6 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
     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 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
         }
     }
 
-    /**
-     * 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());
index 4b71338..48de7e4 100644 (file)
@@ -1177,10 +1177,6 @@ class WindowStateAnimator {
 
         w.transformClipRectFromScreenToSurfaceSpace(clipRect);
 
-        // See {@link WindowState#notifyMovedInStack} for why this is necessary.
-        if (w.hasJustMovedInStack() && mLastClipRect.isEmpty() && !clipRect.isEmpty()) {
-            clipRect.setEmpty();
-        }
         return true;
     }