From 469a2087d58325be9077dc06aed07563f9025179 Mon Sep 17 00:00:00 2001 From: Casey Burkhardt Date: Tue, 13 Jun 2017 20:12:42 -0700 Subject: [PATCH] Resolve 3 inconsistencies in accessibility button API - Unify logic for detecting availability of the accessibility button - Ensure the initial visibility state is propagated to A11yMS - Ensure services only receive availability callbacks for changes Test: Manual, created test accessibility services targeting specific breakages Bug: 38345417 Change-Id: I2250b32830cdfc2ecdc1dff7b7130dced2c1db29 --- .../accessibility/AccessibilityManager.java | 10 +- .../accessibility/IAccessibilityManager.aidl | 2 +- .../AccessibilityManagerService.java | 100 +++++++++++++----- .../android/server/policy/BarController.java | 8 +- .../server/policy/PhoneWindowManager.java | 4 +- 5 files changed, 88 insertions(+), 36 deletions(-) diff --git a/core/java/android/view/accessibility/AccessibilityManager.java b/core/java/android/view/accessibility/AccessibilityManager.java index 41a8b8a4b7507..bdadcc7cd9879 100644 --- a/core/java/android/view/accessibility/AccessibilityManager.java +++ b/core/java/android/view/accessibility/AccessibilityManager.java @@ -912,14 +912,14 @@ public final class AccessibilityManager { } /** - * Notifies that the availability of the accessibility button in the system's navigation area + * Notifies that the visibility of the accessibility button in the system's navigation area * has changed. * - * @param available {@code true} if the accessibility button is available within the system + * @param shown {@code true} if the accessibility button is visible within the system * navigation area, {@code false} otherwise * @hide */ - public void notifyAccessibilityButtonAvailabilityChanged(boolean available) { + public void notifyAccessibilityButtonVisibilityChanged(boolean shown) { final IAccessibilityManager service; synchronized (mLock) { service = getServiceLocked(); @@ -928,9 +928,9 @@ public final class AccessibilityManager { } } try { - service.notifyAccessibilityButtonAvailabilityChanged(available); + service.notifyAccessibilityButtonVisibilityChanged(shown); } catch (RemoteException re) { - Log.e(LOG_TAG, "Error while dispatching accessibility button availability change", re); + Log.e(LOG_TAG, "Error while dispatching accessibility button visibility change", re); } } diff --git a/core/java/android/view/accessibility/IAccessibilityManager.aidl b/core/java/android/view/accessibility/IAccessibilityManager.aidl index 06cb5dca8afa9..3f499abd2e4d1 100644 --- a/core/java/android/view/accessibility/IAccessibilityManager.aidl +++ b/core/java/android/view/accessibility/IAccessibilityManager.aidl @@ -64,7 +64,7 @@ interface IAccessibilityManager { void notifyAccessibilityButtonClicked(); - void notifyAccessibilityButtonAvailabilityChanged(boolean available); + void notifyAccessibilityButtonVisibilityChanged(boolean available); // Requires WRITE_SECURE_SETTINGS void performAccessibilityShortcut(); diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index fa78f10f3e857..7ddc1a226194f 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -243,6 +243,8 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { private WindowsForAccessibilityCallback mWindowsForAccessibilityCallback; + private boolean mIsAccessibilityButtonShown; + private UserState getCurrentUserStateLocked() { return getUserStateLocked(mCurrentUserId); } @@ -881,21 +883,21 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { } /** - * Invoked remotely over AIDL by SysUi when the availability of the accessibility + * Invoked remotely over AIDL by SysUi when the visibility of the accessibility * button within the system's navigation area has changed. * - * @param available {@code true} if the accessibility button is available to the + * @param shown {@code true} if the accessibility button is shown to the * user, {@code false} otherwise */ @Override - public void notifyAccessibilityButtonAvailabilityChanged(boolean available) { + public void notifyAccessibilityButtonVisibilityChanged(boolean shown) { if (mContext.checkCallingOrSelfPermission(android.Manifest.permission.STATUS_BAR_SERVICE) != PackageManager.PERMISSION_GRANTED) { throw new SecurityException("Caller does not hold permission " + android.Manifest.permission.STATUS_BAR_SERVICE); } synchronized (mLock) { - notifyAccessibilityButtonAvailabilityChangedLocked(available); + notifyAccessibilityButtonVisibilityChangedLocked(shown); } } @@ -1200,13 +1202,14 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { } } - private void notifyAccessibilityButtonAvailabilityChangedLocked(boolean available) { + private void notifyAccessibilityButtonVisibilityChangedLocked(boolean available) { final UserState state = getCurrentUserStateLocked(); - state.mIsAccessibilityButtonAvailable = available; + mIsAccessibilityButtonShown = available; for (int i = state.mBoundServices.size() - 1; i >= 0; i--) { final Service service = state.mBoundServices.get(i); if (service.mRequestAccessibilityButton) { - service.notifyAccessibilityButtonAvailabilityChangedLocked(available); + service.notifyAccessibilityButtonAvailabilityChangedLocked( + service.isAccessibilityButtonAvailableLocked(state)); } } } @@ -1733,7 +1736,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { scheduleUpdateInputFilter(userState); scheduleUpdateClientsIfNeededLocked(userState); updateRelevantEventsLocked(userState); - updateAccessibilityButtonTargets(userState); + updateAccessibilityButtonTargetsLocked(userState); } private void updateAccessibilityFocusBehaviorLocked(UserState userState) { @@ -2183,18 +2186,12 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { } } - private void updateAccessibilityButtonTargets(UserState userState) { - final List services; - synchronized (mLock) { - services = userState.mBoundServices; - int numServices = services.size(); - for (int i = 0; i < numServices; i++) { - final Service service = services.get(i); - if (service.mRequestAccessibilityButton) { - boolean available = service.mComponentName.equals( - userState.mServiceAssignedToAccessibilityButton); - service.notifyAccessibilityButtonAvailabilityChangedLocked(available); - } + private void updateAccessibilityButtonTargetsLocked(UserState userState) { + for (int i = userState.mBoundServices.size() - 1; i >= 0; i--) { + final Service service = userState.mBoundServices.get(i); + if (service.mRequestAccessibilityButton) { + service.notifyAccessibilityButtonAvailabilityChangedLocked( + service.isAccessibilityButtonAvailableLocked(userState)); } } } @@ -2501,7 +2498,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { case MSG_SHOW_ACCESSIBILITY_BUTTON_CHOOSER: { showAccessibilityButtonTargetSelection(); - } + } break; } } @@ -2656,6 +2653,10 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { boolean mRequestAccessibilityButton; + boolean mReceivedAccessibilityButtonCallbackSinceBind; + + boolean mLastAccessibilityButtonCallbackState; + int mFetchFlags; long mNotificationTimeout; @@ -3596,9 +3597,8 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { return false; } userState = getCurrentUserStateLocked(); + return isAccessibilityButtonAvailableLocked(userState); } - - return mRequestAccessibilityButton && userState.mIsAccessibilityButtonAvailable; } @Override @@ -3656,6 +3656,7 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { mService = null; } mServiceInterface = null; + mReceivedAccessibilityButtonCallbackSinceBind = false; } public boolean isConnectedLocked() { @@ -3728,6 +3729,48 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { } } + private boolean isAccessibilityButtonAvailableLocked(UserState userState) { + // If the service does not request the accessibility button, it isn't available + if (!mRequestAccessibilityButton) { + return false; + } + + // If the accessibility button isn't currently shown, it cannot be available to services + if (!mIsAccessibilityButtonShown) { + return false; + } + + // If magnification is on and assigned to the accessibility button, services cannot be + if (userState.mIsNavBarMagnificationEnabled + && userState.mIsNavBarMagnificationAssignedToAccessibilityButton) { + return false; + } + + int requestingServices = 0; + for (int i = userState.mBoundServices.size() - 1; i >= 0; i--) { + final Service service = userState.mBoundServices.get(i); + if (service.mRequestAccessibilityButton) { + requestingServices++; + } + } + + if (requestingServices == 1) { + // If only a single service is requesting, it must be this service, and the + // accessibility button is available to it + return true; + } else { + // With more than one active service, we derive the target from the user's settings + if (userState.mServiceAssignedToAccessibilityButton == null) { + // If the user has not made an assignment, we treat the button as available to + // all services until the user interacts with the button to make an assignment + return true; + } else { + // If an assignment was made, it defines availability + return mComponentName.equals(userState.mServiceAssignedToAccessibilityButton); + } + } + } + /** * Notifies an accessibility service client for a scheduled event given the event type. * @@ -3875,6 +3918,13 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { } private void notifyAccessibilityButtonAvailabilityChangedInternal(boolean available) { + // Only notify the service if it's not been notified or the state has changed + if (mReceivedAccessibilityButtonCallbackSinceBind + && (mLastAccessibilityButtonCallbackState == available)) { + return; + } + mReceivedAccessibilityButtonCallbackSinceBind = true; + mLastAccessibilityButtonCallbackState = available; final IAccessibilityServiceClient listener; synchronized (mLock) { listener = mServiceInterface; @@ -4874,7 +4924,6 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { public int mSoftKeyboardShowMode = 0; - public boolean mIsAccessibilityButtonAvailable; public boolean mIsNavBarMagnificationAssignedToAccessibilityButton; public ComponentName mServiceAssignedToAccessibilityButton; @@ -4954,9 +5003,6 @@ public class AccessibilityManagerService extends IAccessibilityManager.Stub { mIsNavBarMagnificationAssignedToAccessibilityButton = false; mIsAutoclickEnabled = false; mSoftKeyboardShowMode = 0; - - // Clear state tracked from system UI - mIsAccessibilityButtonAvailable = false; } public void destroyUiAutomationService() { diff --git a/services/core/java/com/android/server/policy/BarController.java b/services/core/java/com/android/server/policy/BarController.java index 7a2808146f62d..b1792358be538 100644 --- a/services/core/java/com/android/server/policy/BarController.java +++ b/services/core/java/com/android/server/policy/BarController.java @@ -166,8 +166,14 @@ public class BarController { return change || stateChanged; } - void setOnBarVisibilityChangedListener(OnBarVisibilityChangedListener listener) { + void setOnBarVisibilityChangedListener(OnBarVisibilityChangedListener listener, + boolean invokeWithState) { mVisibilityChangeListener = listener; + if (invokeWithState) { + // Optionally report the initial window state for initialization purposes + mHandler.obtainMessage(MSG_NAV_BAR_VISIBILITY_CHANGED, + (mState == StatusBarManager.WINDOW_STATE_SHOWING) ? 1 : 0, 0).sendToTarget(); + } } protected boolean skipAnimation() { diff --git a/services/core/java/com/android/server/policy/PhoneWindowManager.java b/services/core/java/com/android/server/policy/PhoneWindowManager.java index 01eabd8f1403e..8b28df1a3058a 100644 --- a/services/core/java/com/android/server/policy/PhoneWindowManager.java +++ b/services/core/java/com/android/server/policy/PhoneWindowManager.java @@ -1045,7 +1045,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { new BarController.OnBarVisibilityChangedListener() { @Override public void onBarVisibilityChanged(boolean visible) { - mAccessibilityManager.notifyAccessibilityButtonAvailabilityChanged(visible); + mAccessibilityManager.notifyAccessibilityButtonVisibilityChanged(visible); } }; @@ -3037,7 +3037,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { mNavigationBar = win; mNavigationBarController.setWindow(win); mNavigationBarController.setOnBarVisibilityChangedListener( - mNavBarVisibilityListener); + mNavBarVisibilityListener, true); if (DEBUG_LAYOUT) Slog.i(TAG, "NAVIGATION BAR: " + mNavigationBar); break; case TYPE_NAVIGATION_BAR_PANEL: