From 864a428609d5fdd7a6e09f2644b2a2167292cfc6 Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Wed, 3 Jul 2019 15:45:59 +0200 Subject: [PATCH 1/4] Reland "Prevent dismissing starting window when reopening app" If we are changing the visibility while the app is shortly before drawing it's first frame, we cleared out the draw state, leading to a hang. We also had this issue with starting windows, but that was a fix only for starting windows, but cascaded the underlying issue. Now, we only clear out the draw state if is has fully drawn - this covers the Chrome case but doesn't clear out draw state if it's just about to show. Furthermore, we also add the window to mResizingWindows such that the client is expected to call back to us with reportDrawn, in all the edge cases. Test: go/wm-smoke Test: Open TTS settings, go back, open again Test: All the use cases from the linked bug Bug: 135706138 Bug: 135661232 Bug: 135976008 Bug: 135921478 Bug: 135780312 Bug: 135084202 Fixes: 134561008 Change-Id: I69f893a19d6426710bb0b8b0e18f3d2664cb6412 (cherry picked from commit a00ebe4606daafeb0f9daa16d800c523cf1588b8) --- .../java/com/android/server/wm/AppWindowToken.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java index 03cae429904f1..eab5e0dd270ea 100644 --- a/services/core/java/com/android/server/wm/AppWindowToken.java +++ b/services/core/java/com/android/server/wm/AppWindowToken.java @@ -79,6 +79,7 @@ import static com.android.server.wm.WindowManagerService.UPDATE_FOCUS_NORMAL; import static com.android.server.wm.WindowManagerService.UPDATE_FOCUS_WILL_PLACE_SURFACES; import static com.android.server.wm.WindowManagerService.logWithStack; import static com.android.server.wm.WindowState.LEGACY_POLICY_VISIBILITY; +import static com.android.server.wm.WindowStateAnimator.HAS_DRAWN; import static com.android.server.wm.WindowStateAnimator.STACK_CLIP_AFTER_ANIM; import static com.android.server.wm.WindowStateAnimator.STACK_CLIP_BEFORE_ANIM; @@ -540,6 +541,18 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree // If the app was already visible, don't reset the waitingToShow state. if (isHidden()) { waitingToShow = true; + + // Let's reset the draw state in order to prevent the starting window to be + // immediately dismissed when the app still has the surface. + forAllWindows(w -> { + if (w.mWinAnimator.mDrawState == HAS_DRAWN) { + w.mWinAnimator.resetDrawState(); + + // Force add to mResizingWindows, so that we are guaranteed to get + // another reportDrawn callback. + w.resetLastContentInsets(); + } + }, true /* traverseTopToBottom */); } } From 185fc7dfa4949217705060a62b6f6efaa51bc688 Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Tue, 16 Jul 2019 17:43:15 +0200 Subject: [PATCH 2/4] Only consider gone for layout if parent is gone for layout If we check getParentWindowHidden, that determines mostly actual visibility. However, we don't want that because we still would like to follow the parent's window layout lifecycle, as otherwise we may be stuck in a transition in case the parent window is hidden but the child is waiting for a layout to happen. Test: Click "Customize" on wallpaper picker, go back, ensure no transition timeout Fixes: 135976008 Change-Id: I66aeab29a81cd82b170aaf337249616b1f559848 (cherry picked from commit b52b0457e1bad14697341cb81f6d391755b009be) (cherry picked from commit 07f7d1947af6ceecb19fccf9ae44e88bae6b3f57) --- services/core/java/com/android/server/wm/WindowState.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 43ad091b08c0f..703fe4ac867b5 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -1623,7 +1623,7 @@ class WindowState extends WindowContainer implements WindowManagerP || !mRelayoutCalled || (atoken == null && mToken.isHidden()) || (atoken != null && atoken.hiddenRequested) - || isParentWindowHidden() + || isParentWindowGoneForLayout() || (mAnimatingExit && !isAnimatingLw()) || mDestroying; } @@ -3795,6 +3795,11 @@ class WindowState extends WindowContainer implements WindowManagerP return parent != null && parent.mHidden; } + private boolean isParentWindowGoneForLayout() { + final WindowState parent = getParentWindow(); + return parent != null && parent.isGoneForLayoutLw(); + } + void setWillReplaceWindow(boolean animate) { for (int i = mChildren.size() - 1; i >= 0; i--) { final WindowState c = mChildren.get(i); From bc999ddb2d36dfcfdf57fd7e4ebebbe9217cd586 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 24 Jul 2019 15:40:57 -0700 Subject: [PATCH 3/4] SharedMemory: break Cleaner reference cycle. Previously, the Cleaner we create to close the ashmem file descriptor used a thunk that held a strong reference to the FileDescriptor we wanted to clean up, which prevented the Cleaner from ever running. Break the cycle by storing the integer value of the file descriptor instead. Bug: http://b/138323667 Test: treehugger Change-Id: I613a7d035892032f9567d59acb04672957c96011 (cherry picked from commit 6ca916a657cd56158212a57601108716ce78cbe8) (cherry picked from commit 390d9e6a1806626eb521d55a36b1578d28714cc8) --- core/java/android/os/SharedMemory.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/java/android/os/SharedMemory.java b/core/java/android/os/SharedMemory.java index 57a88012a31a0..3e2ba3d3115f7 100644 --- a/core/java/android/os/SharedMemory.java +++ b/core/java/android/os/SharedMemory.java @@ -62,7 +62,7 @@ public final class SharedMemory implements Parcelable, Closeable { mMemoryRegistration = new MemoryRegistration(mSize); mCleaner = Cleaner.create(mFileDescriptor, - new Closer(mFileDescriptor, mMemoryRegistration)); + new Closer(mFileDescriptor.getInt$(), mMemoryRegistration)); } /** @@ -290,10 +290,10 @@ public final class SharedMemory implements Parcelable, Closeable { * Cleaner that closes the FD */ private static final class Closer implements Runnable { - private FileDescriptor mFd; + private int mFd; private MemoryRegistration mMemoryReference; - private Closer(FileDescriptor fd, MemoryRegistration memoryReference) { + private Closer(int fd, MemoryRegistration memoryReference) { mFd = fd; mMemoryReference = memoryReference; } @@ -301,7 +301,9 @@ public final class SharedMemory implements Parcelable, Closeable { @Override public void run() { try { - Os.close(mFd); + FileDescriptor fd = new FileDescriptor(); + fd.setInt$(mFd); + Os.close(fd); } catch (ErrnoException e) { /* swallow error */ } mMemoryReference.release(); mMemoryReference = null; From 475ca92491fe525fec4442472c532d4bec4d69dd Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Thu, 25 Jul 2019 13:54:23 -0700 Subject: [PATCH 4/4] SharedMemory: clear file descriptor when explicitly closed. We run the Cleaner in close, but after the fix in commit 6ca916a6, this no longer clears the value stored in the FileDescriptor, which means that subsequent operations on an explicitly closed SharedMemory will operate on a bogus fd number. Clearing the FileDescriptor value in close is sufficient, because Cleaner.clean is idempotent, and the only other case where it executes is when the FileDescriptor is phantom reachable, which means no one can access it to get its integer value. Bug: http://b/138392115 Bug: http://b/138323667 Test: treehugger Change-Id: I8bdb4c745466532a0712976416184c53fcf0dbf6 (cherry picked from commit a7641806ddf1099239632d53c629c062ff2168f4) (cherry picked from commit 20ab1e34273aa179053f5dc93e70c0191a39e91b) --- core/java/android/os/SharedMemory.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/java/android/os/SharedMemory.java b/core/java/android/os/SharedMemory.java index 3e2ba3d3115f7..0540e3611b52c 100644 --- a/core/java/android/os/SharedMemory.java +++ b/core/java/android/os/SharedMemory.java @@ -259,6 +259,9 @@ public final class SharedMemory implements Parcelable, Closeable { mCleaner.clean(); mCleaner = null; } + + // Cleaner.clean doesn't clear the value of the file descriptor. + mFileDescriptor.setInt$(-1); } @Override