From be42921e05ba3d1946efc090054cd4a498f22b80 Mon Sep 17 00:00:00 2001 From: Ming-Shin Lu Date: Wed, 19 Jan 2022 00:54:21 +0800 Subject: [PATCH 1/4] Fix IME flicker by dispatching unrelated insets after unset IME frozen CL[1] to add InsetsStateController#updateAboveInsetsState in DC#updateImeInputAndControlTarget that after setting mImeInsetsFrozenUntilStartInput as false to unfreeze IME insets, so that increases the possibility to deliver the last IME insets change to non-IME input target window that didn't request show IME. (Since updateImeInputAndControlTarget will call WindowState#insetsChanged to client eventually) As typically the new IME insets change will deliver to the client when server receives InsetsStateController#onInsetsModified(InsetsControlTarget) from the client requests IME visible after DC#updateImeInputAndControlTarget. So in DC#updateImeInputAndControlTarget, we should ensure to unfreeze IME insets after the input target updated, in case updateAboveInsetsState may deliver unrelated IME insets change to the non-IME requester. [1]: I57357ba85501397fa5926ab4dee116c42df24506 Bug: 213522825 Test: atest ActivityRecordTests#\ testImeInsetsFrozenFlag_noDispatchVisibleInsetsWhenAppNotRequest Test: atest WindowStateTests#\ testAdjustImeInsetsVisibilityWhenSwitchingApps Test: Verify Bug 195385541 and Bug 195846009 that keyboard won't cover the edit text. Change-Id: I95c5b45bd4cf9c30ff7771b2db485c8ae2b0c6db (cherry picked from commit 938eb8fd2ded04bbcd5f32576a7bb4eaf186d56a) Merged-In:I95c5b45bd4cf9c30ff7771b2db485c8ae2b0c6db --- .../com/android/server/wm/DisplayContent.java | 8 ++-- .../server/wm/ActivityRecordTests.java | 47 +++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 1e31fdab30d08..42c6dd43ebcee 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -4122,9 +4122,6 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp * which controls the visibility and animation of the input method window. */ void updateImeInputAndControlTarget(WindowState target) { - if (target != null && target.mActivityRecord != null) { - target.mActivityRecord.mImeInsetsFrozenUntilStartInput = false; - } if (mImeInputTarget != target) { ProtoLog.i(WM_DEBUG_IME, "setInputMethodInputTarget %s", target); setImeInputTarget(target); @@ -4132,6 +4129,11 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp .getRawInsetsState().getSourceOrDefaultVisibility(ITYPE_IME)); updateImeControlTarget(); } + // Unfreeze IME insets after the new target updated, in case updateAboveInsetsState may + // deliver unrelated IME insets change to the non-IME requester. + if (target != null && target.mActivityRecord != null) { + target.mActivityRecord.mImeInsetsFrozenUntilStartInput = false; + } } void updateImeControlTarget() { diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java index 9a68b5f1b609c..dd4cc5022cd2b 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityRecordTests.java @@ -99,6 +99,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.never; @@ -131,6 +132,7 @@ import android.view.IWindowManager; import android.view.IWindowSession; import android.view.InsetsSource; import android.view.InsetsState; +import android.view.InsetsVisibilities; import android.view.RemoteAnimationAdapter; import android.view.RemoteAnimationTarget; import android.view.Surface; @@ -147,6 +149,7 @@ import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.invocation.InvocationOnMock; import java.util.ArrayList; @@ -3076,6 +3079,50 @@ public class ActivityRecordTests extends WindowTestsBase { assertEquals(state.getSource(ITYPE_IME).getFrame(), imeSource.getFrame()); } + @UseTestDisplay(addWindows = {W_ACTIVITY, W_INPUT_METHOD}) + @Test + public void testImeInsetsFrozenFlag_noDispatchVisibleInsetsWhenAppNotRequest() + throws RemoteException { + final WindowState app1 = createWindow(null, TYPE_APPLICATION, "app1"); + final WindowState app2 = createWindow(null, TYPE_APPLICATION, "app2"); + + mDisplayContent.getInsetsStateController().getSourceProvider(ITYPE_IME).setWindow( + mImeWindow, null, null); + mImeWindow.getControllableInsetProvider().setServerVisible(true); + + // Simulate app2 is closing and let app1 is visible to be IME targets. + makeWindowVisibleAndDrawn(app1, mImeWindow); + mDisplayContent.setImeLayeringTarget(app1); + mDisplayContent.updateImeInputAndControlTarget(app1); + app2.mActivityRecord.commitVisibility(false, false); + + // app1 requests IME visible. + final InsetsVisibilities requestedVisibilities = new InsetsVisibilities(); + requestedVisibilities.setVisibility(ITYPE_IME, true); + app1.setRequestedVisibilities(requestedVisibilities); + mDisplayContent.getInsetsStateController().onInsetsModified(app1); + + // Verify app1's IME insets is visible and app2's IME insets frozen flag set. + assertTrue(app1.getInsetsState().peekSource(ITYPE_IME).isVisible()); + assertTrue(app2.mActivityRecord.mImeInsetsFrozenUntilStartInput); + + // Simulate switching to app2 to make it visible to be IME targets. + makeWindowVisibleAndDrawn(app2); + spyOn(app2); + spyOn(app2.mClient); + ArgumentCaptor insetsStateCaptor = ArgumentCaptor.forClass(InsetsState.class); + doReturn(true).when(app2).isReadyToDispatchInsetsState(); + mDisplayContent.setImeLayeringTarget(app2); + mDisplayContent.updateImeInputAndControlTarget(app2); + + // Verify after unfreezing app2's IME insets state, we won't dispatch visible IME insets + // to client if the app didn't request IME visible. + assertFalse(app2.mActivityRecord.mImeInsetsFrozenUntilStartInput); + verify(app2.mClient, atLeastOnce()).insetsChanged(insetsStateCaptor.capture(), anyBoolean(), + anyBoolean()); + assertFalse(insetsStateCaptor.getAllValues().get(0).peekSource(ITYPE_IME).isVisible()); + } + @Test public void testInClosingAnimation_doNotHideSurface() { final WindowState app = createWindow(null, TYPE_APPLICATION, "app"); From 5e7baf632495b35b8f553fb342d8fab21492b96d Mon Sep 17 00:00:00 2001 From: Mariia Sandrikova Date: Tue, 18 Jan 2022 19:40:48 +0000 Subject: [PATCH 2/4] Letterbox apps in multi-window if resizeable only due to supportsPictureInPicture. Test: atest WmTests:ActivityRecordTests Fix: 204453314 Fix: 204784257 Change-Id: I664742b033ad49a77ae21b28c7ade4011d123c74 (cherry picked from commit d07c18877456800a6aabec25d6e5126606eae552) Merged-In:I664742b033ad49a77ae21b28c7ade4011d123c74 --- .../com/android/server/wm/ActivityRecord.java | 25 +++++++++++++++---- .../core/java/com/android/server/wm/Task.java | 6 ++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 393d101e98303..b0e192707fdfb 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -2722,9 +2722,13 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A } boolean isResizeable() { + return isResizeable(/* checkPictureInPictureSupport */ true); + } + + boolean isResizeable(boolean checkPictureInPictureSupport) { return mAtmService.mForceResizableActivities || ActivityInfo.isResizeableMode(info.resizeMode) - || info.supportsPictureInPicture() + || (info.supportsPictureInPicture() && checkPictureInPictureSupport) // If the activity can be embedded, it should inherit the bounds of task fragment. || isEmbedded(); } @@ -7679,10 +7683,16 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A // orientation with insets applied. return; } - // Activity should be resizable if the task is. + // Not using Task#isResizeable() or ActivityRecord#isResizeable() directly because app + // compatibility testing showed that android:supportsPictureInPicture="true" alone is not + // sufficient signal for not letterboxing an app. + // TODO(214602463): Remove multi-window check since orientation and aspect ratio + // restrictions should always be applied in multi-window. final boolean isResizeable = task != null - ? task.isResizeable() || isResizeable() - : isResizeable(); + // Activity should be resizable if the task is. + ? task.isResizeable(/* checkPictureInPictureSupport */ false) + || isResizeable(/* checkPictureInPictureSupport */ false) + : isResizeable(/* checkPictureInPictureSupport */ false); if (WindowConfiguration.inMultiWindowMode(windowingMode) && isResizeable) { // Ignore orientation request for resizable apps in multi window. return; @@ -8211,8 +8221,13 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A final float maxAspectRatio = info.getMaxAspectRatio(); final Task rootTask = getRootTask(); final float minAspectRatio = getMinAspectRatio(); + // Not using ActivityRecord#isResizeable() directly because app compatibility testing + // showed that android:supportsPictureInPicture="true" alone is not sufficient signal for + // not letterboxing an app. + // TODO(214602463): Remove multi-window check since orientation and aspect ratio + // restrictions should always be applied in multi-window. if (task == null || rootTask == null - || (inMultiWindowMode() && !shouldCreateCompatDisplayInsets() + || (inMultiWindowMode() && isResizeable(/* checkPictureInPictureSupport */ false) && !fixedOrientationLetterboxed) || (maxAspectRatio < 1 && minAspectRatio < 1 && desiredAspectRatio < 1) || isInVrUiMode(getConfiguration())) { diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index 50a68450b5f26..fd5df6f6c3906 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -2728,10 +2728,14 @@ class Task extends TaskFragment { } boolean isResizeable() { + return isResizeable(/* checkPictureInPictureSupport */ true); + } + + boolean isResizeable(boolean checkPictureInPictureSupport) { final boolean forceResizable = mAtmService.mForceResizableActivities && getActivityType() == ACTIVITY_TYPE_STANDARD; return forceResizable || ActivityInfo.isResizeableMode(mResizeMode) - || mSupportsPictureInPicture; + || (mSupportsPictureInPicture && checkPictureInPictureSupport); } /** From e392af28174684e1cf14e84fab4e11e2389b8a17 Mon Sep 17 00:00:00 2001 From: Dave Mankoff Date: Thu, 20 Jan 2022 11:36:29 -0500 Subject: [PATCH 3/4] RESTRICT AUTOMERGE Read flag resource overrides. We were only reading resource overrides on debug builds, not release builds. This issue is already addressed in the main branch. Fixes: 210114714 Test: atest SystemUITests Change-Id: I40eb02d73faa4c2476d2a4e136e08c6dcd7795a2 (cherry picked from commit 923cd9e73ea4facb1e11a79db6865340fcc2a907) Merged-In:I40eb02d73faa4c2476d2a4e136e08c6dcd7795a2 --- .../systemui/flags/FeatureFlagManager.java | 23 ++++++++++++++-- .../flags/FeatureFlagManagerTest.java | 27 +++++++++++++++---- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java b/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java index 0934b32a71e43..b920a17a0c1ca 100644 --- a/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java +++ b/packages/SystemUI/src-release/com/android/systemui/flags/FeatureFlagManager.java @@ -16,12 +16,15 @@ package com.android.systemui.flags; +import android.content.res.Resources; import android.util.SparseBooleanArray; +import androidx.annotation.BoolRes; import androidx.annotation.NonNull; import com.android.systemui.Dumpable; import com.android.systemui.dagger.SysUISingleton; +import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dump.DumpManager; import java.io.FileDescriptor; @@ -38,8 +41,11 @@ import javax.inject.Inject; @SysUISingleton public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { SparseBooleanArray mAccessedFlags = new SparseBooleanArray(); + private Resources mResources; + @Inject - public FeatureFlagManager(DumpManager dumpManager) { + public FeatureFlagManager(DumpManager dumpManager, @Main Resources resources) { + mResources = resources; dumpManager.registerDumpable("SysUIFlags", this); } @@ -51,7 +57,20 @@ public class FeatureFlagManager implements FlagReader, FlagWriter, Dumpable { @Override public boolean isEnabled(BooleanFlag flag) { - return isEnabled(flag.getId(), flag.getDefault()); + boolean def = flag.getDefault(); + if (flag.hasResourceOverride()) { + try { + def = isEnabledInOverlay(flag.getResourceOverride()); + } catch (Resources.NotFoundException e) { + // no-op + } + } + + return isEnabled(flag.getId(), def); + } + + private boolean isEnabledInOverlay(@BoolRes int resId) { + return mResources.getBoolean(resId); } @Override diff --git a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java index 634763866d028..856bbeaa85e46 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/flags/FeatureFlagManagerTest.java @@ -25,15 +25,14 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; -import android.content.Context; +import android.content.res.Resources; import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; import com.android.systemui.dump.DumpManager; -import com.android.systemui.util.settings.SecureSettings; import org.junit.After; import org.junit.Before; @@ -53,14 +52,14 @@ import java.io.StringWriter; public class FeatureFlagManagerTest extends SysuiTestCase { FeatureFlagManager mFeatureFlagManager; - @Mock private Context mContext; @Mock private DumpManager mDumpManager; + @Mock private Resources mResources; @Before public void setup() { MockitoAnnotations.initMocks(this); - mFeatureFlagManager = new FeatureFlagManager(mDumpManager); + mFeatureFlagManager = new FeatureFlagManager(mDumpManager, mResources); } @After @@ -70,6 +69,24 @@ public class FeatureFlagManagerTest extends SysuiTestCase { verifyNoMoreInteractions(mDumpManager); } + + @Test + public void testSimpleFlag() { + BooleanFlag flagA = new BooleanFlag(100, false); + BooleanFlag flagB = new BooleanFlag(200, true); + + assertThat(mFeatureFlagManager.isEnabled(flagA)).isFalse(); + assertThat(mFeatureFlagManager.isEnabled(flagB)).isTrue(); + } + + @Test + public void testResourceOverride() { + when(mResources.getBoolean(1)).thenReturn(true); + BooleanFlag flag = new BooleanFlag(100, false, 1); + + assertThat(mFeatureFlagManager.isEnabled(flag)).isTrue(); + } + @Test public void testIsEnabled() { mFeatureFlagManager.setEnabled(1, true); From c03eca68e245f7f873137b3e0c741359ead481de Mon Sep 17 00:00:00 2001 From: Ben Lin Date: Mon, 13 Dec 2021 18:00:28 -0800 Subject: [PATCH 4/4] PiP: Disable enterSplit for now. Bug: 206645013 Test: Enter split button no longer appears Change-Id: I6237dd1c88f50733ab24e45806c413d5777032e9 (cherry picked from commit eab029ce8d58e1d6d4a5e29959b424220f2106d8) Merged-In:I6237dd1c88f50733ab24e45806c413d5777032e9 --- libs/WindowManager/Shell/res/values/config.xml | 3 +++ .../src/com/android/wm/shell/pip/phone/PipMenuView.java | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/libs/WindowManager/Shell/res/values/config.xml b/libs/WindowManager/Shell/res/values/config.xml index 0cdaa206c156b..1b8032b7077b8 100644 --- a/libs/WindowManager/Shell/res/values/config.xml +++ b/libs/WindowManager/Shell/res/values/config.xml @@ -43,6 +43,9 @@ 40% + + false + 250 diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipMenuView.java b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipMenuView.java index da4bbe81a3e6a..e1475efcdb574 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipMenuView.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/pip/phone/PipMenuView.java @@ -104,8 +104,6 @@ public class PipMenuView extends FrameLayout { private static final float MENU_BACKGROUND_ALPHA = 0.3f; private static final float DISABLED_ACTION_ALPHA = 0.54f; - private static final boolean ENABLE_ENTER_SPLIT = true; - private int mMenuState; private boolean mAllowMenuTimeout = true; private boolean mAllowTouches = true; @@ -277,6 +275,8 @@ public class PipMenuView extends FrameLayout { boolean resizeMenuOnShow, boolean withDelay, boolean showResizeHandle) { mAllowMenuTimeout = allowMenuTimeout; mDidLastShowMenuResize = resizeMenuOnShow; + final boolean enableEnterSplit = + mContext.getResources().getBoolean(R.bool.config_pipEnableEnterSplitButton); if (mMenuState != menuState) { // Disallow touches if the menu needs to resize while showing, and we are transitioning // to/from a full menu state. @@ -297,7 +297,7 @@ public class PipMenuView extends FrameLayout { mDismissButton.getAlpha(), 1f); ObjectAnimator enterSplitAnim = ObjectAnimator.ofFloat(mEnterSplitButton, View.ALPHA, mEnterSplitButton.getAlpha(), - ENABLE_ENTER_SPLIT && mFocusedTaskAllowSplitScreen ? 1f : 0f); + enableEnterSplit && mFocusedTaskAllowSplitScreen ? 1f : 0f); if (menuState == MENU_STATE_FULL) { mMenuContainerAnimator.playTogether(menuAnim, settingsAnim, dismissAnim, enterSplitAnim);