From e12aece4cad849efbbe6a806f132613a56699230 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 2 Feb 2016 22:43:27 -0800 Subject: [PATCH] Ensure surfaces stay alive until activity stop. Prior to this commit in this case of activity pause, with finishing=true the activity manager will notify us of app visibility and we will begin an exit animation. When this exit animation finishes, we will destroy the application surface (unless its eligible for saving). However there are two cases where this breaks down: 1. The exit animation finishes before the activity thread handles the stop transition. Many activities stop rendering on Pause but many do not and it is totally legal to do so. Sometimes this results in non fatal dequeue buffer errors and sometimes results in fatal errors with Pixel Buffers, etc... 2. We may resume the activity shortly after asking the window manager to pause it. If the window wasn't eligible for animation, we will immediately destroy it after being told of the visibility change following PAUSE_FINISHING. It's possible for this to complete before we process the resume. On the other hand the client happilly processes the resume and transitions back from PAUSE and then crashes once it attempts to use it's surface. In this commit we have the activity manager notify the window manager when an application has actually finished (or we have timed out waiting). For windows which have not been explicitly removed by the client, we defer destruction until we have received both this signal and the animation has completed. Bug: 26793431 Change-Id: Ib6ee8fbdd1f03450fbbfd62468a19e97774befab --- core/java/android/view/IWindowManager.aidl | 1 + .../com/android/server/am/ActivityStack.java | 3 ++ .../com/android/server/wm/AppWindowToken.java | 43 +++++++++++++++++++ .../server/wm/WindowManagerService.java | 27 ++++++++++++ .../com/android/server/wm/WindowState.java | 7 +++ .../server/wm/WindowStateAnimator.java | 33 ++++++++++---- .../src/android/view/IWindowManagerImpl.java | 5 +++ 7 files changed, 110 insertions(+), 9 deletions(-) diff --git a/core/java/android/view/IWindowManager.aidl b/core/java/android/view/IWindowManager.aidl index 1740f071c3fc8..5b9930bb34a71 100644 --- a/core/java/android/view/IWindowManager.aidl +++ b/core/java/android/view/IWindowManager.aidl @@ -166,6 +166,7 @@ interface IWindowManager in CompatibilityInfo compatInfo, CharSequence nonLocalizedLabel, int labelRes, int icon, int logo, int windowFlags, IBinder transferFrom, boolean createIfNeeded); void setAppVisibility(IBinder token, boolean visible); + void notifyAppStopped(IBinder token); void startAppFreezingScreen(IBinder token, int configChanges); void stopAppFreezingScreen(IBinder token, boolean force); void removeAppToken(IBinder token); diff --git a/services/core/java/com/android/server/am/ActivityStack.java b/services/core/java/com/android/server/am/ActivityStack.java index c352fc8198843..9442da0cee51d 100644 --- a/services/core/java/com/android/server/am/ActivityStack.java +++ b/services/core/java/com/android/server/am/ActivityStack.java @@ -1097,6 +1097,9 @@ final class ActivityStack { mHandler.removeMessages(STOP_TIMEOUT_MSG, r); r.stopped = true; r.state = ActivityState.STOPPED; + + mWindowManager.notifyAppStopped(r.appToken); + if (getVisibleBehindActivity() == r) { mStackSupervisor.requestVisibleBehindLocked(r, false); } diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java index 7ec945d74e52a..5a502c1fa013b 100644 --- a/services/core/java/com/android/server/wm/AppWindowToken.java +++ b/services/core/java/com/android/server/wm/AppWindowToken.java @@ -127,6 +127,8 @@ class AppWindowToken extends WindowToken { boolean mAlwaysFocusable; + boolean mAppStopped; + ArrayDeque mFrozenBounds = new ArrayDeque<>(); AppWindowToken(WindowManagerService _service, IApplicationToken _token, @@ -311,6 +313,47 @@ class AppWindowToken extends WindowToken { } } + // Here we destroy surfaces which have been marked as eligible by the animator, taking care + // to ensure the client has finished with them. If the client could still be using them + // we will skip destruction and try again when the client has stopped. + void destroySurfaces() { + final ArrayList allWindows = (ArrayList) allAppWindows.clone(); + final DisplayContentList displayList = new DisplayContentList(); + for (int i = allWindows.size() - 1; i >= 0; i--) { + final WindowState win = allWindows.get(i); + if (!win.mDestroying) { + continue; + } + + if (!mAppStopped && !win.mClientRemoveRequested) { + return; + } + + win.destroyOrSaveSurface(); + if (win.mRemoveOnExit) { + win.mExiting = false; + service.removeWindowInnerLocked(win); + } + final DisplayContent displayContent = win.getDisplayContent(); + if (displayContent != null && !displayList.contains(displayContent)) { + displayList.add(displayContent); + } + win.mDestroying = false; + } + for (int i = 0; i < displayList.size(); i++) { + final DisplayContent displayContent = displayList.get(i); + service.mLayersController.assignLayersLocked(displayContent.getWindowList()); + displayContent.layoutNeeded = true; + } + } + + // The application has stopped, so destroy any surfaces which were keeping alive + // in case they were still being used. + void notifyAppStopped() { + mAppStopped = true; + destroySurfaces(); + } + /** * Checks whether we should save surfaces for this app. * diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index ae6c89a4d6106..3db16c720d4e4 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -2154,6 +2154,14 @@ public class WindowManagerService extends IWindowManager.Stub if (win == null) { return; } + // We set this here instead of removeWindowLocked because we only want it to be + // true when the client has requested we remove the window. In other remove + // cases, we have to wait for activity stop to safely remove the window (as the + // client may still be using the surface). In this case though, the client has + // just dismissed a window (for example a Dialog) and activity stop isn't + // necessarily imminent, so we need to know not to wait for it after our + // hanimation (if applicable) finishes. + win.mClientRemoveRequested = true; removeWindowLocked(win); } } @@ -4187,6 +4195,24 @@ public class WindowManagerService extends IWindowManager.Stub } } + @Override + public void notifyAppStopped(IBinder token) { + if (!checkCallingPermission(android.Manifest.permission.MANAGE_APP_TOKENS, + "notifyAppStopped()")) { + throw new SecurityException("Requires MANAGE_APP_TOKENS permission"); + } + + synchronized(mWindowMap) { + final AppWindowToken wtoken; + wtoken = findAppWindowToken(token); + if (wtoken == null) { + Slog.w(TAG_WM, "Attempted to set visibility of non-existing app token: " + token); + return; + } + wtoken.notifyAppStopped(); + } + } + @Override public void setAppVisibility(IBinder token, boolean visible) { if (!checkCallingPermission(android.Manifest.permission.MANAGE_APP_TOKENS, @@ -4210,6 +4236,7 @@ public class WindowManagerService extends IWindowManager.Stub mOpeningApps.remove(wtoken); mClosingApps.remove(wtoken); + wtoken.mAppStopped = false; wtoken.waitingToShow = false; wtoken.hiddenRequested = !visible; diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 880514cd25f64..114c2121eed21 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -385,6 +385,13 @@ final class WindowState implements WindowManagerPolicy.WindowState { /** Is this window now (or just being) removed? */ boolean mRemoved; + /** + * Has the client requested we remove the window? In this case we know + * that we can dispose of it when we wish without further synchronization + * with the client + */ + boolean mClientRemoveRequested; + /** * Temp for keeping track of windows that have been removed when * rebuilding window list. diff --git a/services/core/java/com/android/server/wm/WindowStateAnimator.java b/services/core/java/com/android/server/wm/WindowStateAnimator.java index c7c9cbf3ce0eb..02012961f5bec 100644 --- a/services/core/java/com/android/server/wm/WindowStateAnimator.java +++ b/services/core/java/com/android/server/wm/WindowStateAnimator.java @@ -471,16 +471,31 @@ class WindowStateAnimator { if (WindowManagerService.localLOGV) Slog.v( TAG, "Exit animation finished in " + this + ": remove=" + mWin.mRemoveOnExit); - if (hasSurface()) { - mService.mDestroySurface.add(mWin); - mWin.mDestroying = true; - hide("finishExit"); - } - mWin.mExiting = false; - if (mWin.mRemoveOnExit) { - mService.mPendingRemove.add(mWin); - mWin.mRemoveOnExit = false; + + + mWin.mDestroying = true; + + // If we have an app token, we ask it to destroy the surface for us, + // so that it can take care to ensure the activity has actually stopped + // and the surface is not still in use. Otherwise we add the service to + // mDestroySurface and allow it to be processed in our next transaction. + if (mWin.mAppToken != null) { + if (hasSurface()) { + hide("finishExit"); + } + mWin.mAppToken.destroySurfaces(); + } else { + if (hasSurface()) { + mService.mDestroySurface.add(mWin); + hide("finishExit"); + } + mWin.mExiting = false; + if (mWin.mRemoveOnExit) { + mService.mPendingRemove.add(mWin); + mWin.mRemoveOnExit = false; + } } + mWallpaperControllerLocked.hideWallpapers(mWin); } diff --git a/tools/layoutlib/bridge/src/android/view/IWindowManagerImpl.java b/tools/layoutlib/bridge/src/android/view/IWindowManagerImpl.java index 2560c31196f02..8d1b124809a3f 100644 --- a/tools/layoutlib/bridge/src/android/view/IWindowManagerImpl.java +++ b/tools/layoutlib/bridge/src/android/view/IWindowManagerImpl.java @@ -347,6 +347,11 @@ public class IWindowManagerImpl implements IWindowManager { } + @Override + public void notifyAppStopped(IBinder token) throws RemoteException { + // TODO Auto-generated method stub + } + @Override public void setEventDispatching(boolean arg0) throws RemoteException { // TODO Auto-generated method stub