-
Notifications
You must be signed in to change notification settings - Fork 6k
Disconnect the channel message handler when releasing the AccessibilityBridge #18657
Conversation
Doh, it's the same as #17646. Thanks for fixing. Can we add a similar test? |
Added a test |
AccessibilityChannel channel = mock(AccessibilityChannel.class); | ||
AccessibilityBridge accessibilityBridge = | ||
setUpBridge(null, channel, null, null, null, null); | ||
verify(channel).setAccessibilityMessageHandler(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is already null, I'd perhaps make this a bit more real by also injecting an AccessibilityManager and let it be isEnabled() true either via mockito or shadowOf(accessibilityManager).setEnabled(true)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the test
AccessibilityManager mockManager = mock(AccessibilityManager.class); | ||
when(mockManager.isEnabled()).thenReturn(true); | ||
AccessibilityBridge accessibilityBridge = | ||
setUpBridge(null, mockChannel, mockManager, null, null, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant if you let it be isEnabled, then you can assert that the message handler is non-null before you release, at which point it's then null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a check for the non-null setAccessibilityMessageHandler
call done during AccessibilityBridge
setup before release
is called
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌 thanks!
AccessibilityManager mockManager = mock(AccessibilityManager.class); | ||
when(mockManager.isEnabled()).thenReturn(true); | ||
AccessibilityBridge accessibilityBridge = | ||
setUpBridge(null, mockChannel, mockManager, null, null, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌 thanks!
Fixes flutter/flutter#57661