From f94b460fda0e71294651e470dfd9c45e3875f6d2 Mon Sep 17 00:00:00 2001 From: tomnatan Date: Mon, 21 Mar 2022 12:23:47 +0000 Subject: [PATCH] [19/n] Letterbox Education: fix multi-user preference key TaskInfo#userId should be used instead of Context#getUserId because the latter isn't correct following user switch. In addition, don't mark the education as seen for the current user until the dialog enter animation is done, to make sure the dialog is fully visible to the user. Fix: 225840105 Test: atest WMShellUnitTests:LetterboxEduWindowManagerTest Change-Id: Ic99c59bc866735bdc1420513f7a3eade4253e391 --- .../LetterboxEduWindowManager.java | 14 ++- .../compatui/CompatUIControllerTest.java | 29 ++--- .../LetterboxEduWindowManagerTest.java | 103 +++++++++++++----- 3 files changed, 92 insertions(+), 54 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManager.java b/libs/WindowManager/Shell/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManager.java index cc3a3b2206f3d..35f1038a68531 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManager.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManager.java @@ -51,7 +51,7 @@ public class LetterboxEduWindowManager extends CompatUIWindowManagerAbstract { /** * The name of the {@link SharedPreferences} that holds which user has seen the Letterbox - * Education for specific packages and which user has seen the full dialog for any package. + * Education dialog. */ @VisibleForTesting static final String HAS_SEEN_LETTERBOX_EDUCATION_PREF_NAME = @@ -66,6 +66,13 @@ public class LetterboxEduWindowManager extends CompatUIWindowManagerAbstract { private final Transitions mTransitions; + /** + * The id of the current user, to associate with a boolean in {@link + * #HAS_SEEN_LETTERBOX_EDUCATION_PREF_NAME}, indicating whether that user has already seen the + * Letterbox Education dialog. + */ + private final int mUserId; + // Remember the last reported state in case visibility changes due to keyguard or IME updates. private boolean mEligibleForLetterboxEducation; @@ -98,6 +105,7 @@ public class LetterboxEduWindowManager extends CompatUIWindowManagerAbstract { mTransitions = transitions; mOnDismissCallback = onDismissCallback; mAnimationController = animationController; + mUserId = taskInfo.userId; mEligibleForLetterboxEducation = taskInfo.topActivityEligibleForLetterboxEducation; mSharedPreferences = mContext.getSharedPreferences(HAS_SEEN_LETTERBOX_EDUCATION_PREF_NAME, Context.MODE_PRIVATE); @@ -133,7 +141,6 @@ public class LetterboxEduWindowManager extends CompatUIWindowManagerAbstract { @Override protected View createLayout() { - setSeenLetterboxEducation(); mLayout = inflateLayout(); updateDialogMargins(); @@ -177,6 +184,7 @@ public class LetterboxEduWindowManager extends CompatUIWindowManagerAbstract { // Dialog has already been released. return; } + setSeenLetterboxEducation(); mLayout.setDismissOnClickListener(this::onDismiss); // Focus on the dialog title for accessibility. mLayout.getDialogTitle().sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_FOCUSED); @@ -241,7 +249,7 @@ public class LetterboxEduWindowManager extends CompatUIWindowManagerAbstract { } private String getPrefKey() { - return String.valueOf(mContext.getUserId()); + return String.valueOf(mUserId); } @VisibleForTesting diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/CompatUIControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/CompatUIControllerTest.java index 4607d8acc63e8..596100dcdead6 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/CompatUIControllerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/CompatUIControllerTest.java @@ -143,9 +143,7 @@ public class CompatUIControllerTest extends ShellTestCase { // Verify that the compat controls and letterbox education are updated with new size compat // info. - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); - clearInvocations(mController); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout, mController); taskInfo = createTaskInfo(DISPLAY_ID, TASK_ID, /* hasSizeCompat= */ true, CAMERA_COMPAT_CONTROL_TREATMENT_APPLIED); mController.onCompatInfoChanged(taskInfo, mMockTaskListener); @@ -156,9 +154,7 @@ public class CompatUIControllerTest extends ShellTestCase { true); // Verify that compat controls and letterbox education are removed with null task listener. - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); - clearInvocations(mController); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout, mController); mController.onCompatInfoChanged(createTaskInfo(DISPLAY_ID, TASK_ID, /* hasSizeCompat= */ true, CAMERA_COMPAT_CONTROL_HIDDEN), /* taskListener= */ null); @@ -181,9 +177,7 @@ public class CompatUIControllerTest extends ShellTestCase { eq(mMockTaskListener)); // Verify that the layout is created again. - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); - clearInvocations(mController); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout, mController); mController.onCompatInfoChanged(taskInfo, mMockTaskListener); verify(mMockCompatLayout, never()).updateCompatInfo(any(), any(), anyBoolean()); @@ -206,9 +200,7 @@ public class CompatUIControllerTest extends ShellTestCase { verify(mController).createLetterboxEduWindowManager(any(), eq(taskInfo), eq(mMockTaskListener)); - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); - clearInvocations(mController); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout, mController); mController.onCompatInfoChanged(taskInfo, mMockTaskListener); verify(mMockCompatLayout).updateCompatInfo(taskInfo, mMockTaskListener, /* canShow= */ @@ -217,9 +209,7 @@ public class CompatUIControllerTest extends ShellTestCase { true); // Verify that the layout is created again. - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); - clearInvocations(mController); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout, mController); mController.onCompatInfoChanged(taskInfo, mMockTaskListener); verify(mMockCompatLayout, never()).updateCompatInfo(any(), any(), anyBoolean()); @@ -294,8 +284,7 @@ public class CompatUIControllerTest extends ShellTestCase { verify(mMockLetterboxEduLayout).updateDisplayLayout(mMockDisplayLayout); // No update if the insets state is the same. - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout); mOnInsetsChangedListenerCaptor.getValue().insetsChanged(new InsetsState(insetsState)); verify(mMockCompatLayout, never()).updateDisplayLayout(mMockDisplayLayout); verify(mMockLetterboxEduLayout, never()).updateDisplayLayout(mMockDisplayLayout); @@ -368,8 +357,7 @@ public class CompatUIControllerTest extends ShellTestCase { verify(mMockCompatLayout, times(2)).updateVisibility(false); verify(mMockLetterboxEduLayout, times(2)).updateVisibility(false); - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout); // Verify button remains hidden after keyguard becomes not showing since IME is showing. mController.onKeyguardShowingChanged(false); @@ -395,8 +383,7 @@ public class CompatUIControllerTest extends ShellTestCase { verify(mMockCompatLayout, times(2)).updateVisibility(false); verify(mMockLetterboxEduLayout, times(2)).updateVisibility(false); - clearInvocations(mMockCompatLayout); - clearInvocations(mMockLetterboxEduLayout); + clearInvocations(mMockCompatLayout, mMockLetterboxEduLayout); // Verify button remains hidden after IME is hidden since keyguard is showing. mController.onImeVisibilityChanged(DISPLAY_ID, /* isShowing= */ false); diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManagerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManagerTest.java index 7d51b521a9fbc..f3a8cf45b7f8f 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManagerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/compatui/letterboxedu/LetterboxEduWindowManagerTest.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; @@ -75,6 +76,12 @@ import org.mockito.MockitoAnnotations; @SmallTest public class LetterboxEduWindowManagerTest extends ShellTestCase { + private static final int USER_ID_1 = 1; + private static final int USER_ID_2 = 2; + + private static final String PREF_KEY_1 = String.valueOf(USER_ID_1); + private static final String PREF_KEY_2 = String.valueOf(USER_ID_2); + private static final int TASK_ID = 1; private static final int TASK_WIDTH = 200; @@ -98,9 +105,10 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { @Mock private Runnable mOnDismissCallback; private SharedPreferences mSharedPreferences; - private String mPrefKey; @Nullable - private Boolean mInitialPrefValue = null; + private Boolean mInitialPrefValue1 = null; + @Nullable + private Boolean mInitialPrefValue2 = null; @Before public void setUp() { @@ -109,20 +117,28 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { mSharedPreferences = mContext.getSharedPreferences( LetterboxEduWindowManager.HAS_SEEN_LETTERBOX_EDUCATION_PREF_NAME, Context.MODE_PRIVATE); - mPrefKey = String.valueOf(mContext.getUserId()); - if (mSharedPreferences.contains(mPrefKey)) { - mInitialPrefValue = mSharedPreferences.getBoolean(mPrefKey, /* default= */ false); - mSharedPreferences.edit().remove(mPrefKey).apply(); + if (mSharedPreferences.contains(PREF_KEY_1)) { + mInitialPrefValue1 = mSharedPreferences.getBoolean(PREF_KEY_1, /* default= */ false); + mSharedPreferences.edit().remove(PREF_KEY_1).apply(); + } + if (mSharedPreferences.contains(PREF_KEY_2)) { + mInitialPrefValue2 = mSharedPreferences.getBoolean(PREF_KEY_2, /* default= */ false); + mSharedPreferences.edit().remove(PREF_KEY_2).apply(); } } @After public void tearDown() { SharedPreferences.Editor editor = mSharedPreferences.edit(); - if (mInitialPrefValue == null) { - editor.remove(mPrefKey); + if (mInitialPrefValue1 == null) { + editor.remove(PREF_KEY_1); } else { - editor.putBoolean(mPrefKey, mInitialPrefValue); + editor.putBoolean(PREF_KEY_1, mInitialPrefValue1); + } + if (mInitialPrefValue2 == null) { + editor.remove(PREF_KEY_2); + } else { + editor.putBoolean(PREF_KEY_2, mInitialPrefValue2); } editor.apply(); } @@ -136,20 +152,10 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { assertNull(windowManager.mLayout); } - @Test - public void testCreateLayout_alreadyShownToUser_doesNotCreateLayout() { - LetterboxEduWindowManager windowManager = createWindowManager(/* eligible= */ true); - mSharedPreferences.edit().putBoolean(mPrefKey, true).apply(); - - assertFalse(windowManager.createLayout(/* canShow= */ true)); - - assertNull(windowManager.mLayout); - } - @Test public void testCreateLayout_taskBarEducationIsShowing_doesNotCreateLayout() { LetterboxEduWindowManager windowManager = createWindowManager(/* eligible= */ - true, /* isTaskbarEduShowing= */ true); + true, USER_ID_1, /* isTaskbarEduShowing= */ true); assertFalse(windowManager.createLayout(/* canShow= */ true)); @@ -162,7 +168,7 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { assertTrue(windowManager.createLayout(/* canShow= */ false)); - assertFalse(mSharedPreferences.getBoolean(mPrefKey, /* default= */ false)); + assertFalse(mSharedPreferences.getBoolean(PREF_KEY_1, /* default= */ false)); assertNull(windowManager.mLayout); } @@ -172,7 +178,6 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { assertTrue(windowManager.createLayout(/* canShow= */ true)); - assertTrue(mSharedPreferences.getBoolean(mPrefKey, /* default= */ false)); LetterboxEduDialogLayout layout = windowManager.mLayout; assertNotNull(layout); verify(mViewHost).setView(eq(layout), mWindowAttrsCaptor.capture()); @@ -183,6 +188,8 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { assertNotNull(dialogTitle); spyOn(dialogTitle); + // The education shouldn't be marked as seen until enter animation is done. + assertFalse(mSharedPreferences.getBoolean(PREF_KEY_1, /* default= */ false)); // Clicking the layout does nothing until enter animation is done. layout.performClick(); verify(mAnimationController, never()).startExitAnimation(any(), any()); @@ -191,6 +198,7 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { verifyAndFinishEnterAnimation(layout); + assertTrue(mSharedPreferences.getBoolean(PREF_KEY_1, /* default= */ false)); verify(dialogTitle).sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_FOCUSED); // Exit animation should start following a click on the layout. layout.performClick(); @@ -207,13 +215,42 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { verify(mOnDismissCallback).run(); } + @Test + public void testCreateLayout_alreadyShownToUser_createsLayoutForOtherUserOnly() { + LetterboxEduWindowManager windowManager = createWindowManager(/* eligible= */ true, + USER_ID_1, /* isTaskbarEduShowing= */ false); + + assertTrue(windowManager.createLayout(/* canShow= */ true)); + + assertNotNull(windowManager.mLayout); + verifyAndFinishEnterAnimation(windowManager.mLayout); + assertTrue(mSharedPreferences.getBoolean(PREF_KEY_1, /* default= */ false)); + + windowManager.release(); + windowManager = createWindowManager(/* eligible= */ true, + USER_ID_1, /* isTaskbarEduShowing= */ false); + + assertFalse(windowManager.createLayout(/* canShow= */ true)); + assertNull(windowManager.mLayout); + + clearInvocations(mTransitions, mAnimationController); + + windowManager = createWindowManager(/* eligible= */ true, + USER_ID_2, /* isTaskbarEduShowing= */ false); + + assertTrue(windowManager.createLayout(/* canShow= */ true)); + + assertNotNull(windowManager.mLayout); + verifyAndFinishEnterAnimation(windowManager.mLayout); + assertTrue(mSharedPreferences.getBoolean(PREF_KEY_1, /* default= */ false)); + } + @Test public void testCreateLayout_windowManagerReleasedBeforeTransitionsIsIdle_doesNotStartAnim() { LetterboxEduWindowManager windowManager = createWindowManager(/* eligible= */ true); assertTrue(windowManager.createLayout(/* canShow= */ true)); - - assertTrue(mSharedPreferences.getBoolean(mPrefKey, /* default= */ false)); + assertNotNull(windowManager.mLayout); verify(mTransitions).runOnIdle(mRunOnIdleCaptor.capture()); @@ -222,6 +259,7 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { mRunOnIdleCaptor.getValue().run(); verify(mAnimationController, never()).startEnterAnimation(any(), any()); + assertFalse(mSharedPreferences.getBoolean(PREF_KEY_1, /* default= */ false)); } @Test @@ -233,7 +271,7 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { assertNotNull(layout); assertTrue(windowManager.updateCompatInfo( - createTaskInfo(/* eligible= */ true, new Rect(50, 25, 150, 75)), + createTaskInfo(/* eligible= */ true, USER_ID_1, new Rect(50, 25, 150, 75)), mTaskListener, /* canShow= */ true)); verifyLayout(layout, layout.getLayoutParams(), /* expectedWidth= */ 100, @@ -341,13 +379,13 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { } private LetterboxEduWindowManager createWindowManager(boolean eligible) { - return createWindowManager(eligible, /* isTaskbarEduShowing= */ false); + return createWindowManager(eligible, USER_ID_1, /* isTaskbarEduShowing= */ false); } private LetterboxEduWindowManager createWindowManager(boolean eligible, - boolean isTaskbarEduShowing) { + int userId, boolean isTaskbarEduShowing) { LetterboxEduWindowManager windowManager = new LetterboxEduWindowManager(mContext, - createTaskInfo(eligible), mSyncTransactionQueue, mTaskListener, + createTaskInfo(eligible, userId), mSyncTransactionQueue, mTaskListener, createDisplayLayout(), mTransitions, mOnDismissCallback, mAnimationController); @@ -375,11 +413,16 @@ public class LetterboxEduWindowManagerTest extends ShellTestCase { } private static TaskInfo createTaskInfo(boolean eligible) { - return createTaskInfo(eligible, new Rect(0, 0, TASK_WIDTH, TASK_HEIGHT)); + return createTaskInfo(eligible, USER_ID_1); } - private static TaskInfo createTaskInfo(boolean eligible, Rect bounds) { + private static TaskInfo createTaskInfo(boolean eligible, int userId) { + return createTaskInfo(eligible, userId, new Rect(0, 0, TASK_WIDTH, TASK_HEIGHT)); + } + + private static TaskInfo createTaskInfo(boolean eligible, int userId, Rect bounds) { ActivityManager.RunningTaskInfo taskInfo = new ActivityManager.RunningTaskInfo(); + taskInfo.userId = userId; taskInfo.taskId = TASK_ID; taskInfo.topActivityEligibleForLetterboxEducation = eligible; taskInfo.configuration.windowConfiguration.setBounds(bounds);