From 7623531af01b2ea3b326f68109ed08ba0ccf24b7 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Wed, 12 Apr 2023 17:24:37 +0800 Subject: [PATCH] Avoid NPE when removing fixed rotation transform The legacy code "checks c.getParent() != null" in resetTransform to avoid the case such as transition finished after the token is detached. While with shell transition, the rotation leash still needs to be removed but ignore reparent in case the token is detached. Also extract FixedRotationTransformStateLegacy for legacy transition. So by default the shell transition can skip the unnecessary creation of SeamlessRotator. Bug: 277697318 Test: IncompleteMotionTest Change-Id: I6e5ebd15a972d88232e7a9a46ff1ab4f557c58d7 --- .../com/android/server/wm/WindowToken.java | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/wm/WindowToken.java b/services/core/java/com/android/server/wm/WindowToken.java index 327483ebbef7e..da54b15fd0a42 100644 --- a/services/core/java/com/android/server/wm/WindowToken.java +++ b/services/core/java/com/android/server/wm/WindowToken.java @@ -119,24 +119,20 @@ class WindowToken extends WindowContainer { final DisplayInfo mDisplayInfo; final DisplayFrames mDisplayFrames; final Configuration mRotatedOverrideConfiguration; - final SeamlessRotator mRotator; + /** * The tokens that share the same transform. Their end time of transform are the same. The * list should at least contain the token who creates this state. */ final ArrayList mAssociatedTokens = new ArrayList<>(3); - final ArrayList> mRotatedContainers = new ArrayList<>(3); + boolean mIsTransforming = true; FixedRotationTransformState(DisplayInfo rotatedDisplayInfo, - DisplayFrames rotatedDisplayFrames, Configuration rotatedConfig, - int currentRotation) { + DisplayFrames rotatedDisplayFrames, Configuration rotatedConfig) { mDisplayInfo = rotatedDisplayInfo; mDisplayFrames = rotatedDisplayFrames; mRotatedOverrideConfiguration = rotatedConfig; - // This will use unrotate as rotate, so the new and old rotation are inverted. - mRotator = new SeamlessRotator(rotatedDisplayInfo.rotation, currentRotation, - rotatedDisplayInfo, true /* applyFixedTransformationHint */); } /** @@ -144,16 +140,48 @@ class WindowToken extends WindowContainer { * showing the window in a display with different rotation. */ void transform(WindowContainer container) { - mRotator.unrotate(container.getPendingTransaction(), container); - if (!mRotatedContainers.contains(container)) { - mRotatedContainers.add(container); - } + // The default implementation assumes shell transition is enabled, so the transform + // is done by getOrCreateFixedRotationLeash(). } /** * Resets the transformation of the window containers which have been rotated. This should * be called when the window has the same rotation as display. */ + void resetTransform() { + for (int i = mAssociatedTokens.size() - 1; i >= 0; --i) { + mAssociatedTokens.get(i).removeFixedRotationLeash(); + } + } + + /** The state may not only be used by self. Make sure to leave the influence by others. */ + void disassociate(WindowToken token) { + mAssociatedTokens.remove(token); + } + } + + private static class FixedRotationTransformStateLegacy extends FixedRotationTransformState { + final SeamlessRotator mRotator; + final ArrayList> mRotatedContainers = new ArrayList<>(3); + + FixedRotationTransformStateLegacy(DisplayInfo rotatedDisplayInfo, + DisplayFrames rotatedDisplayFrames, Configuration rotatedConfig, + int currentRotation) { + super(rotatedDisplayInfo, rotatedDisplayFrames, rotatedConfig); + // This will use unrotate as rotate, so the new and old rotation are inverted. + mRotator = new SeamlessRotator(rotatedDisplayInfo.rotation, currentRotation, + rotatedDisplayInfo, true /* applyFixedTransformationHint */); + } + + @Override + void transform(WindowContainer container) { + mRotator.unrotate(container.getPendingTransaction(), container); + if (!mRotatedContainers.contains(container)) { + mRotatedContainers.add(container); + } + } + + @Override void resetTransform() { for (int i = mRotatedContainers.size() - 1; i >= 0; i--) { final WindowContainer c = mRotatedContainers.get(i); @@ -164,9 +192,9 @@ class WindowToken extends WindowContainer { } } - /** The state may not only be used by self. Make sure to leave the influence by others. */ + @Override void disassociate(WindowToken token) { - mAssociatedTokens.remove(token); + super.disassociate(token); mRotatedContainers.remove(token); } } @@ -437,8 +465,11 @@ class WindowToken extends WindowContainer { if (mFixedRotationTransformState != null) { mFixedRotationTransformState.disassociate(this); } - mFixedRotationTransformState = new FixedRotationTransformState(info, displayFrames, - new Configuration(config), mDisplayContent.getRotation()); + config = new Configuration(config); + mFixedRotationTransformState = mTransitionController.isShellTransitionsEnabled() + ? new FixedRotationTransformState(info, displayFrames, config) + : new FixedRotationTransformStateLegacy(info, displayFrames, config, + mDisplayContent.getRotation()); mFixedRotationTransformState.mAssociatedTokens.add(this); mDisplayContent.getDisplayPolicy().simulateLayoutDisplay(displayFrames); onFixedRotationStatePrepared(); @@ -508,14 +539,7 @@ class WindowToken extends WindowContainer { if (state == null) { return; } - if (!mTransitionController.isShellTransitionsEnabled()) { - state.resetTransform(); - } else { - // Remove all the leashes - for (int i = state.mAssociatedTokens.size() - 1; i >= 0; --i) { - state.mAssociatedTokens.get(i).removeFixedRotationLeash(); - } - } + state.resetTransform(); // Clear the flag so if the display will be updated to the same orientation, the transform // won't take effect. state.mIsTransforming = false; @@ -589,7 +613,9 @@ class WindowToken extends WindowContainer { void removeFixedRotationLeash() { if (mFixedRotationTransformLeash == null) return; final SurfaceControl.Transaction t = getSyncTransaction(); - t.reparent(getSurfaceControl(), getParentSurfaceControl()); + if (mSurfaceControl != null) { + t.reparent(mSurfaceControl, getParentSurfaceControl()); + } t.remove(mFixedRotationTransformLeash); mFixedRotationTransformLeash = null; }