From fc4da62b8c5ff95ae054e9ede681080107d2e697 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 27 Aug 2020 12:24:33 -0700 Subject: [PATCH] Ignore calls to AccessibilityBridge listeners after release (#20701) AccessibilityBridge installs various listeners for Android events that invoke Flutter engine APIs. These listeners are removed in AccessibilityBridge.release. However, in some environments there may be deferred calls to the listener that will still execute even after the listener has been removed. This change sets a flag during release and ignores any listener invocations that happen after the flag is set. See https://github.com/flutter/flutter/issues/63555 and https://github.com/flutter/engine/pull/17311 --- .../io/flutter/view/AccessibilityBridge.java | 13 +++++++++++++ .../flutter/view/AccessibilityBridgeTest.java | 19 ++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/view/AccessibilityBridge.java b/shell/platform/android/io/flutter/view/AccessibilityBridge.java index b54b76b02b..d7bdf39264 100644 --- a/shell/platform/android/io/flutter/view/AccessibilityBridge.java +++ b/shell/platform/android/io/flutter/view/AccessibilityBridge.java @@ -223,6 +223,9 @@ public class AccessibilityBridge extends AccessibilityNodeProvider { @Nullable private OnAccessibilityChangeListener onAccessibilityChangeListener; + // Set to true after {@code release} has been invoked. + private boolean isReleased = false; + // Handler for all messages received from Flutter via the {@code accessibilityChannel} private final AccessibilityChannel.AccessibilityMessageHandler accessibilityMessageHandler = new AccessibilityChannel.AccessibilityMessageHandler() { @@ -274,6 +277,9 @@ public class AccessibilityBridge extends AccessibilityNodeProvider { new AccessibilityManager.AccessibilityStateChangeListener() { @Override public void onAccessibilityStateChanged(boolean accessibilityEnabled) { + if (isReleased) { + return; + } if (accessibilityEnabled) { accessibilityChannel.setAccessibilityMessageHandler(accessibilityMessageHandler); accessibilityChannel.onAndroidAccessibilityEnabled(); @@ -307,6 +313,9 @@ public class AccessibilityBridge extends AccessibilityNodeProvider { @Override public void onChange(boolean selfChange, Uri uri) { + if (isReleased) { + return; + } // Retrieve the current value of TRANSITION_ANIMATION_SCALE from the OS. String value = Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1 @@ -374,6 +383,9 @@ public class AccessibilityBridge extends AccessibilityNodeProvider { new AccessibilityManager.TouchExplorationStateChangeListener() { @Override public void onTouchExplorationStateChanged(boolean isTouchExplorationEnabled) { + if (isReleased) { + return; + } if (isTouchExplorationEnabled) { accessibilityFeatureFlags |= AccessibilityFeature.ACCESSIBLE_NAVIGATION.value; } else { @@ -421,6 +433,7 @@ public class AccessibilityBridge extends AccessibilityNodeProvider { * on this {@code AccessibilityBridge} after invoking {@code release()} is undefined. */ public void release() { + isReleased = true; // platformViewsAccessibilityDelegate should be @NonNull once the plumbing // for io.flutter.embedding.engine.android.FlutterView is done. // TODO(mattcarrol): Remove the null check once the plumbing is done. diff --git a/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java b/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java index 96ade8718a..81d87ae56a 100644 --- a/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java +++ b/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java @@ -7,11 +7,15 @@ package io.flutter.view; import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.annotation.TargetApi; import android.content.ContentResolver; import android.content.Context; import android.view.MotionEvent; @@ -33,6 +37,7 @@ import org.robolectric.annotation.Config; @Config(manifest = Config.NONE) @RunWith(RobolectricTestRunner.class) +@TargetApi(19) public class AccessibilityBridgeTest { @Test @@ -197,14 +202,26 @@ public class AccessibilityBridgeTest { public void releaseDropsChannelMessageHandler() { AccessibilityChannel mockChannel = mock(AccessibilityChannel.class); AccessibilityManager mockManager = mock(AccessibilityManager.class); + ContentResolver mockContentResolver = mock(ContentResolver.class); when(mockManager.isEnabled()).thenReturn(true); AccessibilityBridge accessibilityBridge = - setUpBridge(null, mockChannel, mockManager, null, null, null); + setUpBridge(null, mockChannel, mockManager, mockContentResolver, null, null); verify(mockChannel) .setAccessibilityMessageHandler( any(AccessibilityChannel.AccessibilityMessageHandler.class)); + ArgumentCaptor stateListenerCaptor = + ArgumentCaptor.forClass(AccessibilityManager.AccessibilityStateChangeListener.class); + ArgumentCaptor touchListenerCaptor = + ArgumentCaptor.forClass(AccessibilityManager.TouchExplorationStateChangeListener.class); + verify(mockManager).addAccessibilityStateChangeListener(stateListenerCaptor.capture()); + verify(mockManager).addTouchExplorationStateChangeListener(touchListenerCaptor.capture()); accessibilityBridge.release(); verify(mockChannel).setAccessibilityMessageHandler(null); + reset(mockChannel); + stateListenerCaptor.getValue().onAccessibilityStateChanged(true); + verify(mockChannel, never()).onAndroidAccessibilityEnabled(); + touchListenerCaptor.getValue().onTouchExplorationStateChanged(true); + verify(mockChannel, never()).setAccessibilityFeatures(anyInt()); } AccessibilityBridge setUpBridge() { -- GitLab