diff options
3 files changed, 246 insertions, 129 deletions
diff --git a/services/accessibility/accessibility.aconfig b/services/accessibility/accessibility.aconfig index 3f30b0a5b059..a50fb9a4c318 100644 --- a/services/accessibility/accessibility.aconfig +++ b/services/accessibility/accessibility.aconfig @@ -138,6 +138,16 @@ flag { } flag { + name: "manager_package_monitor_logic_fix" + namespace: "accessibility" + description: "Corrects the return values of the HandleForceStop function" + bug: "337392123" + metadata { + purpose: PURPOSE_BUGFIX + } +} + +flag { name: "pinch_zoom_zero_min_span" namespace: "accessibility" description: "Whether to set min span of ScaleGestureDetector to zero." diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index c70b6419e3b8..40d3e9ae9814 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -697,7 +697,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub /** * Returns the lock object for any synchronized test blocks. - * Should not be used outside of testing. + * External classes should only use for testing. * @return lock object. */ @VisibleForTesting @@ -801,7 +801,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub * * @param packages list of packages that have stopped. * @param userState user state to be read & modified. - * @return {@code true} if a service was enabled or a button target was removed, + * @return {@code true} if the lists of enabled services or buttons were changed, * {@code false} otherwise. */ @VisibleForTesting @@ -824,6 +824,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub userState.getBindingServicesLocked().remove(comp); userState.getCrashedServicesLocked().remove(comp); enabledServicesChanged = true; + break; } } } @@ -851,132 +852,14 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub return mPackageMonitor; } - private void registerBroadcastReceivers() { - mPackageMonitor = new PackageMonitor(true) { - @Override - public void onSomePackagesChanged() { - if (mTraceManager.isA11yTracingEnabledForTypes(FLAGS_PACKAGE_BROADCAST_RECEIVER)) { - mTraceManager.logTrace(LOG_TAG + ".PM.onSomePackagesChanged", - FLAGS_PACKAGE_BROADCAST_RECEIVER); - } - - final int userId = getChangingUserId(); - List<AccessibilityServiceInfo> parsedAccessibilityServiceInfos = null; - List<AccessibilityShortcutInfo> parsedAccessibilityShortcutInfos = null; - parsedAccessibilityServiceInfos = parseAccessibilityServiceInfos(userId); - parsedAccessibilityShortcutInfos = parseAccessibilityShortcutInfos(userId); - synchronized (mLock) { - // Only the profile parent can install accessibility services. - // Therefore we ignore packages from linked profiles. - if (userId != mCurrentUserId) { - return; - } - onSomePackagesChangedLocked(parsedAccessibilityServiceInfos, - parsedAccessibilityShortcutInfos); - } - } - - @Override - public void onPackageUpdateFinished(String packageName, int uid) { - // The package should already be removed from mBoundServices, and added into - // mBindingServices in binderDied() during updating. Remove services from this - // package from mBindingServices, and then update the user state to re-bind new - // versions of them. - if (mTraceManager.isA11yTracingEnabledForTypes(FLAGS_PACKAGE_BROADCAST_RECEIVER)) { - mTraceManager.logTrace(LOG_TAG + ".PM.onPackageUpdateFinished", - FLAGS_PACKAGE_BROADCAST_RECEIVER, - "packageName=" + packageName + ";uid=" + uid); - } - final int userId = getChangingUserId(); - List<AccessibilityServiceInfo> parsedAccessibilityServiceInfos = null; - List<AccessibilityShortcutInfo> parsedAccessibilityShortcutInfos = null; - parsedAccessibilityServiceInfos = parseAccessibilityServiceInfos(userId); - parsedAccessibilityShortcutInfos = parseAccessibilityShortcutInfos(userId); - synchronized (mLock) { - if (userId != mCurrentUserId) { - return; - } - final AccessibilityUserState userState = getUserStateLocked(userId); - final boolean reboundAService = userState.getBindingServicesLocked().removeIf( - component -> component != null - && component.getPackageName().equals(packageName)) - || userState.mCrashedServices.removeIf(component -> component != null - && component.getPackageName().equals(packageName)); - // Reloads the installed services info to make sure the rebound service could - // get a new one. - userState.mInstalledServices.clear(); - final boolean configurationChanged; - configurationChanged = readConfigurationForUserStateLocked(userState, - parsedAccessibilityServiceInfos, parsedAccessibilityShortcutInfos); - if (reboundAService || configurationChanged) { - onUserStateChangedLocked(userState); - } - // Passing 0 for restoreFromSdkInt to have this migration check execute each - // time. It can make sure a11y button settings are correctly if there's an a11y - // service updated and modifies the a11y button configuration. - migrateAccessibilityButtonSettingsIfNecessaryLocked(userState, packageName, - /* restoreFromSdkInt = */0); - } - } - - @Override - public void onPackageRemoved(String packageName, int uid) { - if (mTraceManager.isA11yTracingEnabledForTypes(FLAGS_PACKAGE_BROADCAST_RECEIVER)) { - mTraceManager.logTrace(LOG_TAG + ".PM.onPackageRemoved", - FLAGS_PACKAGE_BROADCAST_RECEIVER, - "packageName=" + packageName + ";uid=" + uid); - } - - synchronized (mLock) { - final int userId = getChangingUserId(); - // Only the profile parent can install accessibility services. - // Therefore we ignore packages from linked profiles. - if (userId != mCurrentUserId) { - return; - } - onPackageRemovedLocked(packageName); - } - } - - /** - * Handles instances in which a package or packages have forcibly stopped. - * - * @param intent intent containing package event information. - * @param uid linux process user id (different from Android user id). - * @param packages array of package names that have stopped. - * @param doit whether to try and handle the stop or just log the trace. - * - * @return {@code true} if package should be restarted, {@code false} otherwise. - */ - @Override - public boolean onHandleForceStop(Intent intent, String[] packages, - int uid, boolean doit) { - if (mTraceManager.isA11yTracingEnabledForTypes(FLAGS_PACKAGE_BROADCAST_RECEIVER)) { - mTraceManager.logTrace(LOG_TAG + ".PM.onHandleForceStop", - FLAGS_PACKAGE_BROADCAST_RECEIVER, - "intent=" + intent + ";packages=" + Arrays.toString(packages) - + ";uid=" + uid + ";doit=" + doit); - } - synchronized (mLock) { - final int userId = getChangingUserId(); - // Only the profile parent can install accessibility services. - // Therefore we ignore packages from linked profiles. - if (userId != mCurrentUserId) { - return false; - } - final AccessibilityUserState userState = getUserStateLocked(userId); - - if (doit && onPackagesForceStoppedLocked(packages, userState)) { - onUserStateChangedLocked(userState); - return false; - } else { - return true; - } - } - } - }; + @VisibleForTesting + void setPackageMonitor(PackageMonitor monitor) { + mPackageMonitor = monitor; + } + private void registerBroadcastReceivers() { // package changes + mPackageMonitor = new ManagerPackageMonitor(this); mPackageMonitor.register(mContext, null, UserHandle.ALL, true); // user change and unlock @@ -992,7 +875,9 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub @Override public void onReceive(Context context, Intent intent) { if (mTraceManager.isA11yTracingEnabledForTypes(FLAGS_USER_BROADCAST_RECEIVER)) { - mTraceManager.logTrace(LOG_TAG + ".BR.onReceive", FLAGS_USER_BROADCAST_RECEIVER, + mTraceManager.logTrace( + LOG_TAG + ".BR.onReceive", + FLAGS_USER_BROADCAST_RECEIVER, "context=" + context + ";intent=" + intent); } @@ -1045,7 +930,8 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub setNonA11yToolNotificationToMatchSafetyCenter(); } }; - mContext.registerReceiverAsUser(receiver, UserHandle.ALL, filter, null, mMainHandler, + mContext.registerReceiverAsUser( + receiver, UserHandle.ALL, filter, null, mMainHandler, Context.RECEIVER_EXPORTED); if (!android.companion.virtual.flags.Flags.vdmPublicApis()) { @@ -6223,6 +6109,162 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub } } + @VisibleForTesting + public static class ManagerPackageMonitor extends PackageMonitor { + private final AccessibilityManagerService mManagerService; + public ManagerPackageMonitor(AccessibilityManagerService managerService) { + super(/* supportsPackageRestartQuery = */ true); + mManagerService = managerService; + } + + @Override + public void onSomePackagesChanged() { + if (mManagerService.mTraceManager.isA11yTracingEnabledForTypes( + FLAGS_PACKAGE_BROADCAST_RECEIVER)) { + mManagerService.mTraceManager.logTrace(LOG_TAG + ".PM.onSomePackagesChanged", + FLAGS_PACKAGE_BROADCAST_RECEIVER); + } + + final int userId = getChangingUserId(); + List<AccessibilityServiceInfo> parsedAccessibilityServiceInfos = mManagerService + .parseAccessibilityServiceInfos(userId); + List<AccessibilityShortcutInfo> parsedAccessibilityShortcutInfos = mManagerService + .parseAccessibilityShortcutInfos(userId); + synchronized (mManagerService.getLock()) { + // Only the profile parent can install accessibility services. + // Therefore we ignore packages from linked profiles. + if (userId != mManagerService.getCurrentUserIdLocked()) { + return; + } + mManagerService.onSomePackagesChangedLocked(parsedAccessibilityServiceInfos, + parsedAccessibilityShortcutInfos); + } + } + + @Override + public void onPackageUpdateFinished(String packageName, int uid) { + // The package should already be removed from mBoundServices, and added into + // mBindingServices in binderDied() during updating. Remove services from this + // package from mBindingServices, and then update the user state to re-bind new + // versions of them. + if (mManagerService.mTraceManager.isA11yTracingEnabledForTypes( + FLAGS_PACKAGE_BROADCAST_RECEIVER)) { + mManagerService.mTraceManager.logTrace( + LOG_TAG + ".PM.onPackageUpdateFinished", + FLAGS_PACKAGE_BROADCAST_RECEIVER, + "packageName=" + packageName + ";uid=" + uid); + } + final int userId = getChangingUserId(); + List<AccessibilityServiceInfo> parsedAccessibilityServiceInfos = mManagerService + .parseAccessibilityServiceInfos(userId); + List<AccessibilityShortcutInfo> parsedAccessibilityShortcutInfos = + mManagerService.parseAccessibilityShortcutInfos(userId); + synchronized (mManagerService.getLock()) { + if (userId != mManagerService.getCurrentUserIdLocked()) { + return; + } + final AccessibilityUserState userState = mManagerService.getUserStateLocked(userId); + final boolean reboundAService = userState.getBindingServicesLocked().removeIf( + component -> component != null + && component.getPackageName().equals(packageName)) + || userState.mCrashedServices.removeIf(component -> component != null + && component.getPackageName().equals(packageName)); + // Reloads the installed services info to make sure the rebound service could + // get a new one. + userState.mInstalledServices.clear(); + final boolean configurationChanged; + configurationChanged = mManagerService.readConfigurationForUserStateLocked( + userState, parsedAccessibilityServiceInfos, + parsedAccessibilityShortcutInfos); + if (reboundAService || configurationChanged) { + mManagerService.onUserStateChangedLocked(userState); + } + // Passing 0 for restoreFromSdkInt to have this migration check execute each + // time. It can make sure a11y button settings are correctly if there's an a11y + // service updated and modifies the a11y button configuration. + mManagerService.migrateAccessibilityButtonSettingsIfNecessaryLocked( + userState, packageName, /* restoreFromSdkInt = */0); + } + } + + @Override + public void onPackageRemoved(String packageName, int uid) { + if (mManagerService.mTraceManager.isA11yTracingEnabledForTypes( + FLAGS_PACKAGE_BROADCAST_RECEIVER)) { + mManagerService.mTraceManager.logTrace(LOG_TAG + ".PM.onPackageRemoved", + FLAGS_PACKAGE_BROADCAST_RECEIVER, + "packageName=" + packageName + ";uid=" + uid); + } + + synchronized (mManagerService.getLock()) { + final int userId = getChangingUserId(); + // Only the profile parent can install accessibility services. + // Therefore we ignore packages from linked profiles. + if (userId != mManagerService.getCurrentUserIdLocked()) { + return; + } + mManagerService.onPackageRemovedLocked(packageName); + } + } + + /** + * Handles instances in which a package or packages have forcibly stopped. + * + * @param intent intent containing package event information. + * @param uid linux process user id (different from Android user id). + * @param packages array of package names that have stopped. + * @param doit whether to try and handle the stop or just log the trace. + * + * @return {@code true} if doit == {@code false} + * and at least one of the provided packages is enabled. + * In any other case, returns {@code false}. + * This is to indicate whether further action is necessary. + */ + @Override + public boolean onHandleForceStop(Intent intent, String[] packages, + int uid, boolean doit) { + if (mManagerService.mTraceManager.isA11yTracingEnabledForTypes( + FLAGS_PACKAGE_BROADCAST_RECEIVER)) { + mManagerService.mTraceManager.logTrace(LOG_TAG + ".PM.onHandleForceStop", + FLAGS_PACKAGE_BROADCAST_RECEIVER, + "intent=" + intent + ";packages=" + Arrays.toString(packages) + + ";uid=" + uid + ";doit=" + doit); + } + synchronized (mManagerService.getLock()) { + final int userId = getChangingUserId(); + // Only the profile parent can install accessibility services. + // Therefore we ignore packages from linked profiles. + if (userId != mManagerService.getCurrentUserIdLocked()) { + return false; + } + final AccessibilityUserState userState = mManagerService.getUserStateLocked(userId); + + if (Flags.managerPackageMonitorLogicFix()) { + if (!doit) { + // if we're not handling the stop here, then we only need to know + // if any of the force-stopped packages are currently enabled. + return userState.mEnabledServices.stream().anyMatch( + (comp) -> Arrays.stream(packages).anyMatch( + (pkg) -> pkg.equals(comp.getPackageName())) + ); + } else if (mManagerService.onPackagesForceStoppedLocked(packages, userState)) { + mManagerService.onUserStateChangedLocked(userState); + } + return false; + } else { + // this old logic did not properly indicate when base packageMonitor's routine + // should handle stopping the package. + if (doit && mManagerService.onPackagesForceStoppedLocked(packages, userState)) { + mManagerService.onUserStateChangedLocked(userState); + return false; + } else { + return true; + } + } + } + } + } + void sendPendingWindowStateChangedEventsForAvailableWindowLocked(int windowId) { final int eventSize = mSendWindowStateChangedEventRunnables.size(); for (int i = eventSize - 1; i >= 0; i--) { diff --git a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java index cb4fc753aa4d..ca15aa2a9d52 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/AccessibilityManagerServiceTest.java @@ -30,7 +30,9 @@ import static com.android.window.flags.Flags.FLAG_ALWAYS_DRAW_MAGNIFICATION_FULL import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; @@ -42,6 +44,7 @@ import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.timeout; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -102,6 +105,7 @@ import com.android.internal.accessibility.common.ShortcutConstants.UserShortcutT import com.android.internal.accessibility.util.AccessibilityUtils; import com.android.internal.accessibility.util.ShortcutUtils; import com.android.internal.compat.IPlatformCompat; +import com.android.internal.content.PackageMonitor; import com.android.server.LocalServices; import com.android.server.accessibility.AccessibilityManagerService.AccessibilityDisplayListener; import com.android.server.accessibility.magnification.FullScreenMagnificationController; @@ -1620,6 +1624,67 @@ public class AccessibilityManagerServiceTest { .containsExactlyElementsIn(Set.of(daltonizerTile)); } + @Test + @EnableFlags(Flags.FLAG_MANAGER_PACKAGE_MONITOR_LOGIC_FIX) + public void onHandleForceStop_dontDoIt_packageEnabled_returnsTrue() { + setupShortcutTargetServices(); + AccessibilityUserState userState = mA11yms.getCurrentUserState(); + userState.mEnabledServices.addAll( + userState.mInstalledServices.stream().map( + (AccessibilityServiceInfo::getComponentName)).toList()); + String[] packages = userState.mEnabledServices.stream().map( + ComponentName::getPackageName).toList().toArray(new String[0]); + + PackageMonitor monitor = spy(mA11yms.getPackageMonitor()); + when(monitor.getChangingUserId()).thenReturn(UserHandle.USER_SYSTEM); + mA11yms.setPackageMonitor(monitor); + + assertTrue(mA11yms.getPackageMonitor().onHandleForceStop( + new Intent(), + packages, + UserHandle.USER_SYSTEM, + false + )); + } + + @Test + @EnableFlags(Flags.FLAG_MANAGER_PACKAGE_MONITOR_LOGIC_FIX) + public void onHandleForceStop_doIt_packageEnabled_returnsFalse() { + setupShortcutTargetServices(); + AccessibilityUserState userState = mA11yms.getCurrentUserState(); + userState.mEnabledServices.addAll( + userState.mInstalledServices.stream().map( + (AccessibilityServiceInfo::getComponentName)).toList()); + String[] packages = userState.mEnabledServices.stream().map( + ComponentName::getPackageName).toList().toArray(new String[0]); + + PackageMonitor monitor = spy(mA11yms.getPackageMonitor()); + when(monitor.getChangingUserId()).thenReturn(UserHandle.USER_SYSTEM); + mA11yms.setPackageMonitor(monitor); + + assertFalse(mA11yms.getPackageMonitor().onHandleForceStop( + new Intent(), + packages, + UserHandle.USER_SYSTEM, + true + )); + } + + @Test + @EnableFlags(Flags.FLAG_MANAGER_PACKAGE_MONITOR_LOGIC_FIX) + public void onHandleForceStop_dontDoIt_packageNotEnabled_returnsFalse() { + PackageMonitor monitor = spy(mA11yms.getPackageMonitor()); + when(monitor.getChangingUserId()).thenReturn(UserHandle.USER_SYSTEM); + mA11yms.setPackageMonitor(monitor); + + assertFalse(mA11yms.getPackageMonitor().onHandleForceStop( + new Intent(), + new String[]{ "FOO", "BAR"}, + UserHandle.USER_SYSTEM, + false + )); + } + private static AccessibilityServiceInfo mockAccessibilityServiceInfo( ComponentName componentName) { return mockAccessibilityServiceInfo( @@ -1630,7 +1695,7 @@ public class AccessibilityManagerServiceTest { ComponentName componentName, boolean isSystemApp, boolean isAlwaysOnService) { AccessibilityServiceInfo accessibilityServiceInfo = - Mockito.spy(new AccessibilityServiceInfo()); + spy(new AccessibilityServiceInfo()); accessibilityServiceInfo.setComponentName(componentName); ResolveInfo mockResolveInfo = Mockito.mock(ResolveInfo.class); when(accessibilityServiceInfo.getResolveInfo()).thenReturn(mockResolveInfo); |