From 1de9f064562e47323fee8fc1537e0f419bd48498 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Tue, 2 May 2023 19:08:02 +0000 Subject: [PATCH 1/3] Do not show Keyguard Presentation when occluded ..., in concurrent mode, and if it is the display being used for concurrent mode. When windows with FLAG_SHOW_WHEN_LOCKED are showing on top of keyguard, the keyguard state becomes "occluded". When this happens, we should not show the Keyguard Presentation on the display that's being used for concurrent mode. This fix allows activities with FLAG_SHOW_WHEN_LOCKED to present content to the non-default display when keyguard is locked. Fixes: 270671994 Bug: 271317597 Test: manual (b/270671994 comment#10) Test: atest KeyguardDisplayManagerTest Change-Id: I79235fdd42ee23f2f4178397937eec859c56d844 --- .../keyguard/KeyguardDisplayManager.java | 76 ++++++++++++++++++- .../keyguard/KeyguardDisplayManagerTest.java | 17 ++++- 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardDisplayManager.java b/packages/SystemUI/src/com/android/keyguard/KeyguardDisplayManager.java index ec8fa921c6fab..9f21a31bd2c05 100644 --- a/packages/SystemUI/src/com/android/keyguard/KeyguardDisplayManager.java +++ b/packages/SystemUI/src/com/android/keyguard/KeyguardDisplayManager.java @@ -19,27 +19,34 @@ import android.app.Presentation; import android.content.Context; import android.graphics.Color; import android.graphics.Rect; +import android.hardware.devicestate.DeviceStateManager; import android.hardware.display.DisplayManager; import android.media.MediaRouter; import android.media.MediaRouter.RouteInfo; import android.os.Bundle; import android.os.Trace; +import android.text.TextUtils; import android.util.Log; import android.util.SparseArray; import android.view.Display; +import android.view.DisplayAddress; import android.view.DisplayInfo; import android.view.LayoutInflater; import android.view.View; import android.view.WindowManager; +import androidx.annotation.Nullable; + import com.android.internal.annotations.VisibleForTesting; import com.android.keyguard.dagger.KeyguardStatusViewComponent; import com.android.systemui.R; +import com.android.systemui.dagger.SysUISingleton; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.dagger.qualifiers.UiBackground; import com.android.systemui.navigationbar.NavigationBarController; import com.android.systemui.navigationbar.NavigationBarView; import com.android.systemui.settings.DisplayTracker; +import com.android.systemui.statusbar.policy.KeyguardStateController; import java.util.concurrent.Executor; @@ -47,6 +54,7 @@ import javax.inject.Inject; import dagger.Lazy; +@SysUISingleton public class KeyguardDisplayManager { protected static final String TAG = "KeyguardDisplayManager"; private static final boolean DEBUG = KeyguardConstants.DEBUG; @@ -61,6 +69,9 @@ public class KeyguardDisplayManager { private boolean mShowing; private final DisplayInfo mTmpDisplayInfo = new DisplayInfo(); + private final DeviceStateHelper mDeviceStateHelper; + private final KeyguardStateController mKeyguardStateController; + private final SparseArray mPresentations = new SparseArray<>(); private final DisplayTracker.Callback mDisplayCallback = @@ -92,7 +103,9 @@ public class KeyguardDisplayManager { KeyguardStatusViewComponent.Factory keyguardStatusViewComponentFactory, DisplayTracker displayTracker, @Main Executor mainExecutor, - @UiBackground Executor uiBgExecutor) { + @UiBackground Executor uiBgExecutor, + DeviceStateHelper deviceStateHelper, + KeyguardStateController keyguardStateController) { mContext = context; mNavigationBarControllerLazy = navigationBarControllerLazy; mKeyguardStatusViewComponentFactory = keyguardStatusViewComponentFactory; @@ -100,6 +113,8 @@ public class KeyguardDisplayManager { mDisplayService = mContext.getSystemService(DisplayManager.class); mDisplayTracker = displayTracker; mDisplayTracker.addDisplayChangeCallback(mDisplayCallback, mainExecutor); + mDeviceStateHelper = deviceStateHelper; + mKeyguardStateController = keyguardStateController; } private boolean isKeyguardShowable(Display display) { @@ -122,6 +137,18 @@ public class KeyguardDisplayManager { } return false; } + if (mKeyguardStateController.isOccluded() + && mDeviceStateHelper.isConcurrentDisplayActive(display)) { + if (DEBUG) { + // When activities with FLAG_SHOW_WHEN_LOCKED are shown on top of Keyguard, the + // Keyguard state becomes "occluded". In this case, we should not show the + // KeyguardPresentation, since the activity is presenting content onto the + // non-default display. + Log.i(TAG, "Do not show KeyguardPresentation when occluded and concurrent" + + " display is active"); + } + return false; + } return true; } @@ -260,6 +287,53 @@ public class KeyguardDisplayManager { } + /** + * Helper used to receive device state info from {@link DeviceStateManager}. + */ + static class DeviceStateHelper implements DeviceStateManager.DeviceStateCallback { + + @Nullable + private final DisplayAddress.Physical mRearDisplayPhysicalAddress; + + // TODO(b/271317597): These device states should be defined in DeviceStateManager + private final int mConcurrentState; + private boolean mIsInConcurrentDisplayState; + + @Inject + DeviceStateHelper(Context context, + DeviceStateManager deviceStateManager, + @Main Executor mainExecutor) { + + final String rearDisplayPhysicalAddress = context.getResources().getString( + com.android.internal.R.string.config_rearDisplayPhysicalAddress); + if (TextUtils.isEmpty(rearDisplayPhysicalAddress)) { + mRearDisplayPhysicalAddress = null; + } else { + mRearDisplayPhysicalAddress = DisplayAddress + .fromPhysicalDisplayId(Long.parseLong(rearDisplayPhysicalAddress)); + } + + mConcurrentState = context.getResources().getInteger( + com.android.internal.R.integer.config_deviceStateConcurrentRearDisplay); + deviceStateManager.registerCallback(mainExecutor, this); + } + + @Override + public void onStateChanged(int state) { + // When concurrent state ends, the display also turns off. This is enforced in various + // ExtensionRearDisplayPresentationTest CTS tests. So, we don't need to invoke + // hide() since that will happen through the onDisplayRemoved callback. + mIsInConcurrentDisplayState = state == mConcurrentState; + } + + boolean isConcurrentDisplayActive(Display display) { + return mIsInConcurrentDisplayState + && mRearDisplayPhysicalAddress != null + && mRearDisplayPhysicalAddress.equals(display.getAddress()); + } + } + + @VisibleForTesting static final class KeyguardPresentation extends Presentation { private static final int VIDEO_SAFE_REGION = 80; // Percentage of display width & height diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardDisplayManagerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardDisplayManagerTest.java index 30e3d09299f2c..b3496967f5256 100644 --- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardDisplayManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardDisplayManagerTest.java @@ -25,6 +25,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.hardware.display.DisplayManagerGlobal; import android.testing.AndroidTestingRunner; @@ -38,6 +39,7 @@ import com.android.keyguard.dagger.KeyguardStatusViewComponent; import com.android.systemui.SysuiTestCase; import com.android.systemui.navigationbar.NavigationBarController; import com.android.systemui.settings.FakeDisplayTracker; +import com.android.systemui.statusbar.policy.KeyguardStateController; import org.junit.Before; import org.junit.Test; @@ -58,6 +60,10 @@ public class KeyguardDisplayManagerTest extends SysuiTestCase { private KeyguardStatusViewComponent.Factory mKeyguardStatusViewComponentFactory; @Mock private KeyguardDisplayManager.KeyguardPresentation mKeyguardPresentation; + @Mock + private KeyguardDisplayManager.DeviceStateHelper mDeviceStateHelper; + @Mock + private KeyguardStateController mKeyguardStateController; private Executor mMainExecutor = Runnable::run; private Executor mBackgroundExecutor = Runnable::run; @@ -76,7 +82,7 @@ public class KeyguardDisplayManagerTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); mManager = spy(new KeyguardDisplayManager(mContext, () -> mNavigationBarController, mKeyguardStatusViewComponentFactory, mDisplayTracker, mMainExecutor, - mBackgroundExecutor)); + mBackgroundExecutor, mDeviceStateHelper, mKeyguardStateController)); doReturn(mKeyguardPresentation).when(mManager).createPresentation(any()); mDefaultDisplay = new Display(DisplayManagerGlobal.getInstance(), Display.DEFAULT_DISPLAY, @@ -123,4 +129,13 @@ public class KeyguardDisplayManagerTest extends SysuiTestCase { mManager.show(); verify(mManager, times(1)).createPresentation(eq(mSecondaryDisplay)); } + + @Test + public void testShow_concurrentDisplayActive_occluded() { + mDisplayTracker.setAllDisplays(new Display[]{mDefaultDisplay, mSecondaryDisplay}); + + when(mDeviceStateHelper.isConcurrentDisplayActive(mSecondaryDisplay)).thenReturn(true); + when(mKeyguardStateController.isOccluded()).thenReturn(true); + verify(mManager, never()).createPresentation(eq(mSecondaryDisplay)); + } } From 50f9e8baef9639f708a4bb1741c1698c6bafbb92 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Fri, 5 May 2023 22:31:19 +0000 Subject: [PATCH 2/3] Add foreground check for controlling or requesting device state In addition to checking if the app is on top, we should also check if the app is in foreground. An app may be on top, but can be hidden by keyguard. Also note that we are doing both checks, becuase for example in multi-window mode, multiple apps may be in the foreground. However, only one of those is considered "top". Bug: 270671994 Test: android.server.wm.jetpack.area.ExtensionRearDisplayPresentationKeyguardTest#testStartRearDisplayPresentation_whenKeyguardLocked_withoutShowWhenLocked Change-Id: Iae4d67e55612e9df895a494bb1b891b546c262b9 --- .../DeviceStateManagerService.java | 54 ++++++++++++++++--- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java index 964569008accc..5d0177be18b3a 100644 --- a/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java +++ b/services/core/java/com/android/server/devicestate/DeviceStateManagerService.java @@ -17,6 +17,7 @@ package com.android.server.devicestate; import static android.Manifest.permission.CONTROL_DEVICE_STATE; +import static android.app.ActivityManager.RunningAppProcessInfo.IMPORTANCE_FOREGROUND; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.hardware.devicestate.DeviceStateManager.INVALID_DEVICE_STATE; import static android.hardware.devicestate.DeviceStateManager.MAXIMUM_DEVICE_STATE; @@ -73,6 +74,7 @@ import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.WeakHashMap; @@ -162,7 +164,7 @@ public final class DeviceStateManagerService extends SystemService { @GuardedBy("mLock") private final SparseArray mProcessRecords = new SparseArray<>(); - private Set mDeviceStatesAvailableForAppRequests; + private Set mDeviceStatesAvailableForAppRequests = new HashSet<>(); private Set mFoldedDeviceStates; @@ -879,8 +881,16 @@ public final class DeviceStateManagerService extends SystemService { * @param callingPid Process ID that is requesting this state change * @param state state that is being requested. */ - private void assertCanRequestDeviceState(int callingPid, int state) { - if (!isTopApp(callingPid) || !isStateAvailableForAppRequests(state)) { + private void assertCanRequestDeviceState(int callingPid, int callingUid, int state) { + final boolean isTopApp = isTopApp(callingPid); + final boolean isForegroundApp = isForegroundApp(callingPid, callingUid); + final boolean isStateAvailableForAppRequests = isStateAvailableForAppRequests(state); + + final boolean canRequestState = isTopApp + && isForegroundApp + && isStateAvailableForAppRequests; + + if (!canRequestState) { getContext().enforceCallingOrSelfPermission(CONTROL_DEVICE_STATE, "Permission required to request device state, " + "or the call must come from the top app " @@ -893,15 +903,43 @@ public final class DeviceStateManagerService extends SystemService { * not the top app, then check if this process holds the CONTROL_DEVICE_STATE permission. * * @param callingPid Process ID that is requesting this state change + * @param callingUid UID that is requesting this state change */ - private void assertCanControlDeviceState(int callingPid) { - if (!isTopApp(callingPid)) { + private void assertCanControlDeviceState(int callingPid, int callingUid) { + final boolean isTopApp = isTopApp(callingPid); + final boolean isForegroundApp = isForegroundApp(callingPid, callingUid); + + final boolean canControlState = isTopApp && isForegroundApp; + + if (!canControlState) { getContext().enforceCallingOrSelfPermission(CONTROL_DEVICE_STATE, "Permission required to request device state, " + "or the call must come from the top app."); } } + /** + * Checks if the caller is in the foreground. Note that callers may be the top app as returned + * from {@link #isTopApp(int)}, but not be in the foreground. For example, keyguard may be on + * top of the top app. + */ + private boolean isForegroundApp(int callingPid, int callingUid) { + try { + final List procs = + ActivityManager.getService().getRunningAppProcesses(); + for (int i = 0; i < procs.size(); i++) { + ActivityManager.RunningAppProcessInfo proc = procs.get(i); + if (proc.pid == callingPid && proc.uid == callingUid + && proc.importance <= IMPORTANCE_FOREGROUND) { + return true; + } + } + } catch (RemoteException e) { + Slog.w(TAG, "am.getRunningAppProcesses() failed", e); + } + return false; + } + private boolean isTopApp(int callingPid) { final WindowProcessController topApp = mActivityTaskManagerInternal.getTopApp(); return topApp != null && topApp.getPid() == callingPid; @@ -918,7 +956,6 @@ public final class DeviceStateManagerService extends SystemService { */ @GuardedBy("mLock") private void readStatesAvailableForRequestFromApps() { - mDeviceStatesAvailableForAppRequests = new HashSet<>(); String[] availableAppStatesConfigIdentifiers = getContext().getResources() .getStringArray(R.array.config_deviceStatesAvailableForAppRequests); for (int i = 0; i < availableAppStatesConfigIdentifiers.length; i++) { @@ -1118,7 +1155,7 @@ public final class DeviceStateManagerService extends SystemService { // Allow top processes to request a device state change // If the calling process ID is not the top app, then we check if this process // holds a permission to CONTROL_DEVICE_STATE - assertCanRequestDeviceState(callingPid, state); + assertCanRequestDeviceState(callingPid, callingUid, state); if (token == null) { throw new IllegalArgumentException("Request token must not be null."); @@ -1139,10 +1176,11 @@ public final class DeviceStateManagerService extends SystemService { @Override // Binder call public void cancelStateRequest() { final int callingPid = Binder.getCallingPid(); + final int callingUid = Binder.getCallingUid(); // Allow top processes to cancel a device state change // If the calling process ID is not the top app, then we check if this process // holds a permission to CONTROL_DEVICE_STATE - assertCanControlDeviceState(callingPid); + assertCanControlDeviceState(callingPid, callingUid); final long callingIdentity = Binder.clearCallingIdentity(); try { From 0f6b4e2348ccba918a52074be7321e08ad9d9cb0 Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Mon, 8 May 2023 22:00:59 +0000 Subject: [PATCH 3/3] Cleanup mRearDisplayPresentationController if SecurityException caught If a SecurityException occurs when invoking DeviceStateManager#requestState (e.g. if not the caller is not in foreground, or if it does not have the required permissions), we should first clean up our local state before re-throwing the SecurityException to the caller. Otherwise, subsequent attempts to startRearDisplayPresentationSession will always fail. Bug: 270671994 Test: atest ExtensionRearDisplayPresentationKeyguardTest Change-Id: Ie102b03b722f018dc093ef9ab8c5c41b141a5bd0 --- .../area/WindowAreaComponentImpl.java | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/area/WindowAreaComponentImpl.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/area/WindowAreaComponentImpl.java index 658d92cc74896..ff423c2c6e390 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/area/WindowAreaComponentImpl.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/area/WindowAreaComponentImpl.java @@ -342,11 +342,22 @@ public class WindowAreaComponentImpl implements WindowAreaComponent, mRearDisplayPresentationController); DeviceStateRequest concurrentDisplayStateRequest = DeviceStateRequest.newBuilder( mConcurrentDisplayState).build(); - mDeviceStateManager.requestState( - concurrentDisplayStateRequest, - mExecutor, - deviceStateCallback - ); + + try { + mDeviceStateManager.requestState( + concurrentDisplayStateRequest, + mExecutor, + deviceStateCallback + ); + } catch (SecurityException e) { + // If a SecurityException occurs when invoking DeviceStateManager#requestState + // (e.g. if the caller is not in the foreground, or if it does not have the required + // permissions), we should first clean up our local state before re-throwing the + // SecurityException to the caller. Otherwise, subsequent attempts to + // startRearDisplayPresentationSession will always fail. + mRearDisplayPresentationController = null; + throw e; + } } }