From 0dd0cf911b26d3f9d95c8162553c9e2f2f266def Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Fri, 27 Dec 2019 15:17:44 +0100 Subject: [PATCH] Fix issue with letterboxing - Only consider client state if new API is actually turned on. - We only sent the state the client was actually controlling. However, we then later check for visibility for state that may not even exist. Thus, we keep the state but only update the sources the client is actually controlling. - Initialize source with default visibility. This prevents issues where the source may not exist but we still check it. This fixes issues where a letterbox was accidentally placed on screen because WM was thinking client requested fullscreen flag (hiding the status bar), and other places where we checked requested visibility. Also renamed client state to requested state. Test: Open any app, open IME Bug: 111084606 Change-Id: Ibead561fc5593d8944400320f5e31dbe262612fe --- core/java/android/view/InsetsSource.java | 1 + .../view/InsetsSourceConsumerTest.java | 4 +++ .../com/android/server/wm/DisplayPolicy.java | 7 ++++-- .../com/android/server/wm/InsetsPolicy.java | 5 ++-- .../java/com/android/server/wm/Session.java | 2 +- .../server/wm/TaskSnapshotController.java | 2 +- .../server/wm/TaskSnapshotSurface.java | 2 +- .../com/android/server/wm/WindowState.java | 25 ++++++++++++++----- .../android/server/wm/WindowStateTests.java | 4 ++- 9 files changed, 38 insertions(+), 14 deletions(-) diff --git a/core/java/android/view/InsetsSource.java b/core/java/android/view/InsetsSource.java index 1a33ea9dd6fd4..324d562bfcfb7 100644 --- a/core/java/android/view/InsetsSource.java +++ b/core/java/android/view/InsetsSource.java @@ -41,6 +41,7 @@ public class InsetsSource implements Parcelable { public InsetsSource(@InternalInsetsType int type) { mType = type; mFrame = new Rect(); + mVisible = InsetsState.getDefaultVisibility(type); } public InsetsSource(InsetsSource other) { diff --git a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java index 2cf6ff3a58c10..7af833bfcba40 100644 --- a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java +++ b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java @@ -102,8 +102,12 @@ public class InsetsSourceConsumerTest { @Test public void testShow() { + + // Insets source starts out visible + mConsumer.hide(); mConsumer.show(); assertTrue("Consumer should be visible", mConsumer.isVisible()); + verify(mSpyInsetsSource).setVisible(eq(false)); verify(mSpyInsetsSource).setVisible(eq(true)); } diff --git a/services/core/java/com/android/server/wm/DisplayPolicy.java b/services/core/java/com/android/server/wm/DisplayPolicy.java index 694a73d626f8e..f9ad03f407237 100644 --- a/services/core/java/com/android/server/wm/DisplayPolicy.java +++ b/services/core/java/com/android/server/wm/DisplayPolicy.java @@ -2192,10 +2192,13 @@ public class DisplayPolicy { final boolean attachedInParent = attached != null && !layoutInScreen; final boolean requestedFullscreen = (fl & FLAG_FULLSCREEN) != 0 || (requestedSysUiFl & View.SYSTEM_UI_FLAG_FULLSCREEN) != 0 - || !win.getClientInsetsState().getSource(ITYPE_STATUS_BAR).isVisible(); + || (ViewRootImpl.sNewInsetsMode == NEW_INSETS_MODE_FULL + && !win.getRequestedInsetsState().getSource(ITYPE_STATUS_BAR).isVisible()); final boolean requestedHideNavigation = (requestedSysUiFl & View.SYSTEM_UI_FLAG_HIDE_NAVIGATION) != 0 - || !win.getClientInsetsState().getSource(ITYPE_NAVIGATION_BAR).isVisible(); + || (ViewRootImpl.sNewInsetsMode == NEW_INSETS_MODE_FULL + && !win.getRequestedInsetsState().getSource(ITYPE_NAVIGATION_BAR) + .isVisible()); // TYPE_BASE_APPLICATION windows are never considered floating here because they don't get // cropped / shifted to the displayFrame in WindowState. diff --git a/services/core/java/com/android/server/wm/InsetsPolicy.java b/services/core/java/com/android/server/wm/InsetsPolicy.java index af84836b433e9..a0089639bce6c 100644 --- a/services/core/java/com/android/server/wm/InsetsPolicy.java +++ b/services/core/java/com/android/server/wm/InsetsPolicy.java @@ -66,10 +66,11 @@ class InsetsPolicy { } mStatusBar.setVisible(focusedWin == null || focusedWin != getStatusControlTarget(focusedWin) - || focusedWin.getClientInsetsState().getSource(ITYPE_STATUS_BAR).isVisible()); + || focusedWin.getRequestedInsetsState().getSource(ITYPE_STATUS_BAR).isVisible()); mNavBar.setVisible(focusedWin == null || focusedWin != getNavControlTarget(focusedWin) - || focusedWin.getClientInsetsState().getSource(ITYPE_NAVIGATION_BAR).isVisible()); + || focusedWin.getRequestedInsetsState().getSource(ITYPE_NAVIGATION_BAR) + .isVisible()); } boolean isHidden(@InternalInsetsType int type) { diff --git a/services/core/java/com/android/server/wm/Session.java b/services/core/java/com/android/server/wm/Session.java index a175d63dce801..6d42373dc871d 100644 --- a/services/core/java/com/android/server/wm/Session.java +++ b/services/core/java/com/android/server/wm/Session.java @@ -463,7 +463,7 @@ class Session extends IWindowSession.Stub implements IBinder.DeathRecipient { final WindowState windowState = mService.windowForClientLocked(this, window, false /* throwOnError */); if (windowState != null) { - windowState.setClientInsetsState(state); + windowState.updateRequestedInsetsState(state); windowState.getDisplayContent().getInsetsPolicy().onInsetsModified( windowState, state); } diff --git a/services/core/java/com/android/server/wm/TaskSnapshotController.java b/services/core/java/com/android/server/wm/TaskSnapshotController.java index e4744dba85d30..4cb5de44bef00 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotController.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotController.java @@ -470,7 +470,7 @@ class TaskSnapshotController { final LayoutParams attrs = mainWindow.getAttrs(); final SystemBarBackgroundPainter decorPainter = new SystemBarBackgroundPainter(attrs.flags, attrs.privateFlags, attrs.systemUiVisibility, task.getTaskDescription(), - mFullSnapshotScale, mainWindow.getClientInsetsState()); + mFullSnapshotScale, mainWindow.getRequestedInsetsState()); final int width = (int) (task.getBounds().width() * mFullSnapshotScale); final int height = (int) (task.getBounds().height() * mFullSnapshotScale); diff --git a/services/core/java/com/android/server/wm/TaskSnapshotSurface.java b/services/core/java/com/android/server/wm/TaskSnapshotSurface.java index 5b458d8944305..e2a21a9e9db4f 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotSurface.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotSurface.java @@ -222,7 +222,7 @@ class TaskSnapshotSurface implements StartingSurface { final TaskSnapshotSurface snapshotSurface = new TaskSnapshotSurface(service, window, surfaceControl, snapshot, layoutParams.getTitle(), taskDescription, sysUiVis, windowFlags, windowPrivateFlags, taskBounds, - currentOrientation, topFullscreenOpaqueWindow.getClientInsetsState()); + currentOrientation, topFullscreenOpaqueWindow.getRequestedInsetsState()); window.setOuter(snapshotSurface); try { session.relayout(window, window.mSeq, layoutParams, -1, -1, View.VISIBLE, 0, -1, diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 886f11a6f8699..ec5670eff83a3 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -194,6 +194,7 @@ import android.view.InputChannel; import android.view.InputEvent; import android.view.InputEventReceiver; import android.view.InputWindowHandle; +import android.view.InsetsSource; import android.view.InsetsState; import android.view.Surface.Rotation; import android.view.SurfaceControl; @@ -637,17 +638,29 @@ class WindowState extends WindowContainer implements WindowManagerP private boolean mIsDimming = false; private @Nullable InsetsSourceProvider mControllableInsetProvider; - private InsetsState mClientInsetsState; + private InsetsState mRequestedInsetsState; private static final float DEFAULT_DIM_AMOUNT_DEAD_WINDOW = 0.5f; private KeyInterceptionInfo mKeyInterceptionInfo; - InsetsState getClientInsetsState() { - return mClientInsetsState; + /** + * @return The insets state as requested by the client, i.e. the dispatched insets state + * for which the visibilities are overridden with what the client requested. + */ + InsetsState getRequestedInsetsState() { + return mRequestedInsetsState; } - void setClientInsetsState(InsetsState state) { - mClientInsetsState = state; + /** + * @see #getRequestedInsetsState() + */ + void updateRequestedInsetsState(InsetsState state) { + + // Only update the sources the client is actually controlling. + for (int i = state.getSourcesCount(); i >= 0; i--) { + final InsetsSource source = state.sourceAt(i); + mRequestedInsetsState.addSource(source); + } } void seamlesslyRotateIfAllowed(Transaction transaction, @Rotation int oldRotation, @@ -766,7 +779,7 @@ class WindowState extends WindowContainer implements WindowManagerP mSeq = seq; mPowerManagerWrapper = powerManagerWrapper; mForceSeamlesslyRotate = token.mRoundedCornerOverlay; - mClientInsetsState = + mRequestedInsetsState = getDisplayContent().getInsetsPolicy().getInsetsForDispatch(this); if (DEBUG) { Slog.v(TAG, "Window " + this + " client=" + c.asBinder() diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java index 6d23b2ebe2429..c3494f7baacd9 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowStateTests.java @@ -421,8 +421,10 @@ public class WindowStateTests extends WindowTestsBase { .setWindow(statusBar, null /* frameProvider */); mDisplayContent.getInsetsStateController().onBarControlTargetChanged( app, null /* fakeTopControlling */, app, null /* fakeNavControlling */); + final InsetsSource source = new InsetsSource(ITYPE_STATUS_BAR); + source.setVisible(false); mDisplayContent.getInsetsStateController().getSourceProvider(ITYPE_STATUS_BAR) - .onInsetsModified(app, new InsetsSource(ITYPE_STATUS_BAR)); + .onInsetsModified(app, source); waitUntilHandlersIdle(); assertFalse(statusBar.isVisible()); }