Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a7de21af authored by András Kurucz's avatar András Kurucz
Browse files

Don't send Accessibility events during NotificationContentView changes

Don't send Accessibility events from the NotificationContentView while
it is animating a layout change. Notify the showing ContentView at the end
of the animation, that its accessibility state might have changed.

Bug: 296578272
Test: atest NotificationContentViewTest
Test: run notification jank  microbenchmark test suites with abdt
Flag: NONE
Change-Id: I3721fb20769487f6d3581df15a890abb9ec59161
parent 39cc587b
Loading
Loading
Loading
Loading
+26 −1
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ import android.view.MotionEvent;
import android.view.View;
import android.view.ViewGroup;
import android.view.ViewTreeObserver;
import android.view.accessibility.AccessibilityEvent;
import android.widget.FrameLayout;
import android.widget.ImageView;
import android.widget.LinearLayout;
@@ -45,8 +46,8 @@ import androidx.annotation.MainThread;

import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.statusbar.IStatusBarService;
import com.android.systemui.res.R;
import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin;
import com.android.systemui.res.R;
import com.android.systemui.statusbar.RemoteInputController;
import com.android.systemui.statusbar.SmartReplyController;
import com.android.systemui.statusbar.TransformableView;
@@ -898,6 +899,7 @@ public class NotificationContentView extends FrameLayout implements Notification
        // forceUpdateVisibilities cancels outstanding animations without updating the
        // mAnimationStartVisibleType. Do so here instead.
        mAnimationStartVisibleType = VISIBLE_TYPE_NONE;
        notifySubtreeForAccessibilityContentChange();
    }

    private void fireExpandedVisibleListenerIfVisible() {
@@ -980,6 +982,7 @@ public class NotificationContentView extends FrameLayout implements Notification
        // updateViewVisibilities cancels outstanding animations without updating the
        // mAnimationStartVisibleType. Do so here instead.
        mAnimationStartVisibleType = VISIBLE_TYPE_NONE;
        notifySubtreeForAccessibilityContentChange();
    }

    private void updateViewVisibility(int visibleType, int type, View view,
@@ -1029,6 +1032,7 @@ public class NotificationContentView extends FrameLayout implements Notification
                    hiddenView.setVisible(false);
                }
                mAnimationStartVisibleType = VISIBLE_TYPE_NONE;
                notifySubtreeForAccessibilityContentChange();
            }
        });
        fireExpandedVisibleListenerIfVisible();
@@ -1049,6 +1053,22 @@ public class NotificationContentView extends FrameLayout implements Notification
        }
    }

    @Override
    public void notifySubtreeAccessibilityStateChanged(View child, View source, int changeType) {
        if (isAnimatingVisibleType()) {
            // Don't send A11y events while animating to reduce Jank.
            return;
        }
        super.notifySubtreeAccessibilityStateChanged(child, source, changeType);
    }

    private void notifySubtreeForAccessibilityContentChange() {
        if (mParent != null) {
            mParent.notifySubtreeAccessibilityStateChanged(this, this,
                    AccessibilityEvent.CONTENT_CHANGE_TYPE_SUBTREE);
        }
    }

    /**
     * @param visibleType one of the static enum types in this view
     * @return the corresponding transformable view according to the given visible type
@@ -2277,6 +2297,11 @@ public class NotificationContentView extends FrameLayout implements Notification
        mHeadsUpWrapper = headsUpWrapper;
    }

    @VisibleForTesting
    protected void setAnimationStartVisibleType(int animationStartVisibleType) {
        mAnimationStartVisibleType = animationStartVisibleType;
    }

    @Override
    protected void dispatchDraw(Canvas canvas) {
        try {
+36 −3
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ import com.android.systemui.SysuiTestCase
import com.android.systemui.statusbar.notification.FeedbackIcon
import com.android.systemui.statusbar.notification.collection.NotificationEntry
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
import com.android.systemui.util.mockito.any
import com.android.systemui.util.mockito.mock
import com.android.systemui.util.mockito.whenever
import junit.framework.Assert.assertEquals
@@ -49,6 +50,8 @@ import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito
import org.mockito.Mockito.anyBoolean
import org.mockito.Mockito.anyInt
import org.mockito.Mockito.clearInvocations
import org.mockito.Mockito.doReturn
import org.mockito.Mockito.never
import org.mockito.Mockito.spy
@@ -74,7 +77,8 @@ class NotificationContentViewTest : SysuiTestCase() {
    @Before
    fun setup() {
        initMocks(this)
        fakeParent = FrameLayout(mContext, /* attrs= */ null).also { it.visibility = View.GONE }
        fakeParent =
            spy(FrameLayout(mContext, /* attrs= */ null).also { it.visibility = View.GONE })
        row =
            spy(
                ExpandableNotificationRow(mContext, /* attrs= */ null).apply {
@@ -558,6 +562,35 @@ class NotificationContentViewTest : SysuiTestCase() {
        verify(view.headsUpWrapper, never()).setAnimationsRunning(false)
    }

    @Test
    fun notifySubtreeAccessibilityStateChanged_notifiesParent() {
        // Given: a contentView is created
        val view = createContentView()
        clearInvocations(fakeParent)

        // When: the contentView is notified for an A11y change
        view.notifySubtreeAccessibilityStateChanged(view.contractedChild, view.contractedChild, 0)

        // Then: the contentView propagates the event to its parent
        verify(fakeParent).notifySubtreeAccessibilityStateChanged(any(), any(), anyInt())
    }

    @Test
    fun notifySubtreeAccessibilityStateChanged_animatingContentView_dontNotifyParent() {
        // Given: a collapsed contentView is created
        val view = createContentView()
        clearInvocations(fakeParent)

        // And: it is animating to expanded
        view.setAnimationStartVisibleType(NotificationContentView.VISIBLE_TYPE_EXPANDED)

        // When: the contentView is notified for an A11y change
        view.notifySubtreeAccessibilityStateChanged(view.contractedChild, view.contractedChild, 0)

        // Then: the contentView DOESN'T propagates the event to its parent
        verify(fakeParent, never()).notifySubtreeAccessibilityStateChanged(any(), any(), anyInt())
    }

    private fun createMockContainingNotification(notificationEntry: NotificationEntry) =
        mock<ExpandableNotificationRow>().apply {
            whenever(this.entry).thenReturn(notificationEntry)
@@ -597,7 +630,7 @@ class NotificationContentViewTest : SysuiTestCase() {
        }

    private fun createContentView(
        isSystemExpanded: Boolean,
        isSystemExpanded: Boolean = false,
        contractedView: View = createViewWithHeight(contractedHeight),
        expandedView: View = createViewWithHeight(expandedHeight),
        headsUpView: View = createViewWithHeight(contractedHeight),
@@ -647,5 +680,5 @@ private fun NotificationContentView.mockRequestLayout() {
}

private fun NotificationContentView.clearInvocations() {
    Mockito.clearInvocations(contractedWrapper, expandedWrapper, headsUpWrapper)
    clearInvocations(contractedWrapper, expandedWrapper, headsUpWrapper)
}