From 4515b2202e134014ed7007dd6600accb97827a4a Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 3 Mar 2021 16:28:20 -0800 Subject: [PATCH] Force update IME container relative layer when IME parent is changed Before, WindowContainer#assignRelativeLayer() will not set layer if the call is the same as the previous call. This could cause issue. In case the previous IME parent is removed, the relative layer of IME is also removed. However, WindowContainer won't know about that, so the next assignRelativeLayer with the same arguments will still be ignored. Now we do a force update when the IME parent is changed. Bug: 180541134 Test: atest WmTests:DisplayContentTests Test: atest WmTests:WindowContainerTests Test: verify with reproduce steps in bug Change-Id: Ie1f00146f150a5fcf818d775239bd93d5235ead3 --- .../com/android/server/wm/DisplayContent.java | 19 +++++++++----- .../android/server/wm/WindowContainer.java | 9 +++++-- .../server/wm/DisplayContentTests.java | 19 ++++++++++++++ .../server/wm/WindowContainerTests.java | 25 +++++++++++++++++++ 4 files changed, 64 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index eff4ea6536bd2..6a5310f9c0ba6 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -3859,6 +3859,9 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp if (newParent != null && newParent != mInputMethodSurfaceParent) { mInputMethodSurfaceParent = newParent; getPendingTransaction().reparent(mImeWindowsContainer.mSurfaceControl, newParent); + // When surface parent is removed, the relative layer will also be removed. We need to + // do a force update to make sure there is a layer set for the new parent. + assignRelativeLayerForIme(getPendingTransaction(), true /* forceUpdate */); scheduleAnimation(); } } @@ -4547,11 +4550,12 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp } @Override - void assignRelativeLayer(Transaction t, SurfaceControl relativeTo, int layer) { + void assignRelativeLayer(Transaction t, SurfaceControl relativeTo, int layer, + boolean forceUpdate) { if (!mNeedsLayer) { return; } - super.assignRelativeLayer(t, relativeTo, layer); + super.assignRelativeLayer(t, relativeTo, layer, forceUpdate); mNeedsLayer = false; } } @@ -4641,6 +4645,11 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp @Override void assignChildLayers(SurfaceControl.Transaction t) { + assignRelativeLayerForIme(t, false /* forceUpdate */); + super.assignChildLayers(t); + } + + private void assignRelativeLayerForIme(SurfaceControl.Transaction t, boolean forceUpdate) { mImeWindowsContainer.setNeedsLayer(); final WindowState imeTarget = mImeLayeringTarget; // In the case where we have an IME target that is not in split-screen mode IME @@ -4667,14 +4676,13 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp mImeWindowsContainer.assignRelativeLayer(t, imeTarget.getSurfaceControl(), // TODO: We need to use an extra level on the app surface to ensure // this is always above SurfaceView but always below attached window. - 1); + 1, forceUpdate); } else if (mInputMethodSurfaceParent != null) { // The IME surface parent may not be its window parent's surface // (@see #computeImeParent), so set relative layer here instead of letting the window // parent to assign layer. - mImeWindowsContainer.assignRelativeLayer(t, mInputMethodSurfaceParent, 1); + mImeWindowsContainer.assignRelativeLayer(t, mInputMethodSurfaceParent, 1, forceUpdate); } - super.assignChildLayers(t); } /** @@ -4687,7 +4695,6 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp * with {@link WindowState#assignLayer} */ void assignRelativeLayerForImeTargetChild(SurfaceControl.Transaction t, WindowContainer child) { - mImeWindowsContainer.setNeedsLayer(); child.assignRelativeLayer(t, mImeWindowsContainer.getSurfaceControl(), 1); } diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index dd4ee877c05b6..cd7df06e262f4 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -2241,15 +2241,20 @@ class WindowContainer extends ConfigurationContainer< } } - void assignRelativeLayer(Transaction t, SurfaceControl relativeTo, int layer) { + void assignRelativeLayer(Transaction t, SurfaceControl relativeTo, int layer, + boolean forceUpdate) { final boolean changed = layer != mLastLayer || mLastRelativeToLayer != relativeTo; - if (mSurfaceControl != null && changed) { + if (mSurfaceControl != null && (changed || forceUpdate)) { setRelativeLayer(t, relativeTo, layer); mLastLayer = layer; mLastRelativeToLayer = relativeTo; } } + void assignRelativeLayer(Transaction t, SurfaceControl relativeTo, int layer) { + assignRelativeLayer(t, relativeTo, layer, false /* forceUpdate */); + } + protected void setLayer(Transaction t, int layer) { // Route through surface animator to accommodate that our surface control might be diff --git a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java index 137cf6523caf2..f5602fd854a02 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/DisplayContentTests.java @@ -342,6 +342,25 @@ public class DisplayContentTests extends WindowTestsBase { verify(imeTarget.getRootDisplayArea()).placeImeContainer(imeContainer); } + @Test + public void testUpdateImeParent_forceUpdateRelativeLayer() { + final DisplayArea.Tokens imeContainer = mDisplayContent.getImeContainer(); + final ActivityRecord activity = createActivityRecord(mDisplayContent); + + final WindowState startingWin = createWindow(null, TYPE_APPLICATION_STARTING, activity, + "startingWin"); + startingWin.setHasSurface(true); + assertTrue(startingWin.canBeImeTarget()); + final SurfaceControl imeSurfaceParent = mock(SurfaceControl.class); + doReturn(imeSurfaceParent).when(mDisplayContent).computeImeParent(); + spyOn(imeContainer); + + mDisplayContent.updateImeParent(); + + // Force reassign the relative layer when the IME surface parent is changed. + verify(imeContainer).assignRelativeLayer(any(), eq(imeSurfaceParent), anyInt(), eq(true)); + } + /** * This tests stack movement between displays and proper stack's, task's and app token's display * container references updates. diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java index 99c96bd0de1bd..cc64d0c4cf455 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java @@ -54,6 +54,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.clearInvocations; import android.content.pm.ActivityInfo; import android.content.res.Configuration; @@ -1008,6 +1009,30 @@ public class WindowContainerTests extends WindowTestsBase { verify(win).clearFrozenInsetsState(); } + @Test + public void testAssignRelativeLayer() { + final WindowContainer container = new WindowContainer(mWm); + container.mSurfaceControl = mock(SurfaceControl.class); + final SurfaceAnimator surfaceAnimator = container.mSurfaceAnimator; + final SurfaceControl relativeParent = mock(SurfaceControl.class); + final SurfaceControl.Transaction t = mock(SurfaceControl.Transaction.class); + spyOn(container); + spyOn(surfaceAnimator); + + // Trigger for first relative layer call. + container.assignRelativeLayer(t, relativeParent, 1 /* layer */); + verify(surfaceAnimator).setRelativeLayer(t, relativeParent, 1 /* layer */); + + // Not trigger for the same relative layer call. + clearInvocations(surfaceAnimator); + container.assignRelativeLayer(t, relativeParent, 1 /* layer */); + verify(surfaceAnimator, never()).setRelativeLayer(t, relativeParent, 1 /* layer */); + + // Trigger for the same relative layer call if forceUpdate=true + container.assignRelativeLayer(t, relativeParent, 1 /* layer */, true /* forceUpdate */); + verify(surfaceAnimator).setRelativeLayer(t, relativeParent, 1 /* layer */); + } + /* Used so we can gain access to some protected members of the {@link WindowContainer} class */ private static class TestWindowContainer extends WindowContainer { private final int mLayer;