Reland: AccessibilityEvent throttling on Android with improvements

This CL relands AccessibilityEvent throttling on Android.

Original CL: http://crrev.com/c/2067443
Revert: http://crrev.com/c/2132927

This change was reverted to prevent regression before M83 was cut,
but with this CL it will be relanded along with the improvements
added here.

This CL improves the throttling logic by separating it from the
WebContentsAccessibilityImpl into it's own class, which we have
called AccessibilityEventDispatcher. Whenever the WCAI wishes to
send an event, it sends it to the dispatcher. The dispatcher can
then act as a pass-through, throttle the event, discard the event,
etc. as determined by preset rules that are opaque to the
WebContentsAccessibilityImpl. This will make unit testing easier
and allows us to make wholesale changes to our throttling without
affecting implementations.

This CL also adds an improvement by taking into account the
|virtualViewId| of whatever node is trying to send an event. Until
now we have relied solely on the eventType and ensured that no
two events of the same type are sent in quick succession (if the
eventType is one we are throttling). This creates an issue
where two separate nodes with different |virtualViewId|'s can
each request an event of some type to be sent, but only one will
get through.

We fix this problem by making our throttling check not just the
eventType but also |virtualViewId|. This requires some sort of 2D
structure, or a key of Pair or some wrapper object that will
hold the two ints (|virtualViewId| and eventType). Here we use a
long in place of a Pair, by bitshifting |virtualViewId| left 32
bits and doing a bitwise OR with eventType. This gives us a 64bit
long that is essentially two int's concatenated in memory, which
will be a unique id for a |virtualViewId|-eventType pairing.

This CL also adds an improvement by allowing us to specify a delay
time on a per eventType basis. Before this change we had a simple
Set that contains a list of eventTypes to delay. We have now made
this a map from eventType to delay time in ms, so that we can
potentially delay different events by different amounts.


Change-Id: I7431253a715cc712a81b1f6ac2b39d19e91daa8e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2139905Reviewed-by: Mark Schillaci <mschillaci@google.com>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/master@{#759354}

content/public/android/BUILD.gn

@@ -186,6 +186,7 @@ android_library("content_java") {
     "java/src/org/chromium/content/browser/ViewEventSinkImpl.java",
     "java/src/org/chromium/content/browser/WindowEventObserver.java",
     "java/src/org/chromium/content/browser/WindowEventObserverManager.java",
+    "java/src/org/chromium/content/browser/accessibility/AccessibilityEventDispatcher.java",
     "java/src/org/chromium/content/browser/accessibility/BrowserAccessibilityState.java",
     "java/src/org/chromium/content/browser/accessibility/KitKatWebContentsAccessibility.java",
     "java/src/org/chromium/content/browser/accessibility/LollipopWebContentsAccessibility.java",

content/public/android/java/src/org/chromium/content/browser/accessibility/AccessibilityEventDispatcher.java

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.content.browser.accessibility;
+
+import android.view.accessibility.AccessibilityEvent;
+
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * This class works as an intermediary between {@link WebContentsAccessibilityImpl} and the OS to
+ * throttle and queue AccessibilityEvents that are sent in quick succession. This ensures we do
+ * not overload the system and create lag by sending superfluous events.
+ */
+public class AccessibilityEventDispatcher {
+    // Default delay for throttling of successive AccessibilityEvents in milliseconds.
+    private static final int ACCESSIBILITY_EVENT_DEFAULT_DELAY = 100;
+
+    // Maps an AccessibilityEvent type to a throttle delay in milliseconds. This is populated once
+    // in the constructor.
+    private Map<Integer, Integer> mEventThrottleDelays = new HashMap<Integer, Integer>();
+
+    // For events being throttled (see: |mEventsToThrottle|), this array will map the eventType
+    // to the last time (long in milliseconds) such an event has been sent.
+    private Map<Long, Long> mEventLastFiredTimes = new HashMap<Long, Long>();
+
+    // For events being throttled (see: |mEventsToThrottle|), this array will map the eventType
+    // to a single Runnable that will send an event after some delay.
+    private Map<Long, Runnable> mPendingEvents = new HashMap<Long, Runnable>();
+
+    // Implementation of the callback interface to {@link WebContentsAccessibilityImpl} so that we
+    // can maintain a connection through JNI to the native code.
+    private Client mClient;
+
+    /**
+     * Callback interface to link {@link WebContentsAccessibilityImpl} with an instance of the
+     * {@link AccessibilityEventDispatcher} so we can separate out queueing/throttling logic into a
+     * dispatcher while still maintaining a connection through the JNI to native code.
+     */
+    interface Client {
+        /**
+         * Post a Runnable to a view's message queue.
+         *
+         * @param toPost                The Runnable to post.
+         * @param delayInMilliseconds   The delay in milliseconds before running.
+         */
+        void postRunnable(Runnable toPost, long delayInMilliseconds);
+
+        /**
+         * Remove a Runnable from a view's message queue.
+         *
+         * @param toRemove              The Runnable to remove.
+         */
+        void removeRunnable(Runnable toRemove);
+
+        /**
+         * Build an AccessibilityEvent for the given id and type. Requires a connection through the
+         * JNI to the native code to populate the fields of the event. If successfully built, then
+         * send the event and return true, otherwise return false.
+         *
+         * @param virtualViewId         This virtualViewId for the view trying to send an event.
+         * @param eventType             The AccessibilityEvent type.
+         * @return                      boolean value of whether event was sent.
+         */
+        boolean dispatchEvent(int virtualViewId, int eventType);
+    }
+
+    /**
+     *  Create an AccessibilityEventDispatcher and define the delays for event types.
+     */
+    public AccessibilityEventDispatcher(Client mClient) {
+        this.mClient = mClient;
+
+        // Define our delays on a per event type basis.
+        mEventThrottleDelays.put(
+                AccessibilityEvent.TYPE_VIEW_SCROLLED, ACCESSIBILITY_EVENT_DEFAULT_DELAY);
+        mEventThrottleDelays.put(
+                AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED, ACCESSIBILITY_EVENT_DEFAULT_DELAY);
+    }
+
+    /**
+     * Enqueue an AccessibilityEvent. This method will leverage our throttling and queue, and
+     * check the appropriate amount of time we should delay, if at all, before building/sending this
+     * event. When ready, this will handle the delayed construction and dispatching of this event.
+     *
+     * @param virtualViewId     This virtualViewId for the view trying to send an event.
+     * @param eventType         The AccessibilityEvent type.
+     */
+    public void enqueueEvent(int virtualViewId, int eventType) {
+        // Check whether this type of event is one we want to throttle, and if not then send it
+        if (!mEventThrottleDelays.containsKey(eventType)) {
+            mClient.dispatchEvent(virtualViewId, eventType);
+            return;
+        }
+
+        // Check when we last fired an event of |eventType| for this |virtualViewId|. If we have not
+        // fired an event of this type for this id, or the last time was longer ago than the delay
+        // for this eventType as per |mEventThrottleDelays|, then we allow this event to be sent
+        // immediately and record the time and clear any lingering callbacks.
+        long now = Calendar.getInstance().getTimeInMillis();
+        long uuid = uuid(virtualViewId, eventType);
+        if (mEventLastFiredTimes.get(uuid) == null
+                || now - mEventLastFiredTimes.get(uuid) >= mEventThrottleDelays.get(eventType)) {
+            // Attempt to dispatch an event, can fail and return false if node is invalid etc.
+            if (mClient.dispatchEvent(virtualViewId, eventType)) {
+                // Record time of last fired event if the dispatch was successful.
+                mEventLastFiredTimes.put(uuid, now);
+            }
+
+            // Remove any lingering callbacks and pending events regardless of success.
+            mClient.removeRunnable(mPendingEvents.get(uuid));
+            mPendingEvents.remove(uuid);
+        } else {
+            // We have fired an event of |eventType| for this |virtualViewId| within our
+            // |mEventThrottleDelays| delay window. Store this event, replacing any events in
+            // |mPendingEvents| of the same |uuid|, and set a delay equal.
+            mClient.removeRunnable(mPendingEvents.get(uuid));
+
+            Runnable myRunnable = () -> {
+                // We have delayed firing this event, so accessibility may not be enabled or the
+                // node may be invalid, in which case dispatch will return false.
+                if (mClient.dispatchEvent(virtualViewId, eventType)) {
+                    // After sending event, record time it was sent
+                    mEventLastFiredTimes.put(uuid, Calendar.getInstance().getTimeInMillis());
+                }
+
+                // Remove any lingering callbacks and pending events regardless of success.
+                mClient.removeRunnable(mPendingEvents.get(uuid));
+                mPendingEvents.remove(uuid);
+            };
+
+            mClient.postRunnable(myRunnable,
+                    (mEventLastFiredTimes.get(uuid) + mEventThrottleDelays.get(eventType)) - now);
+            mPendingEvents.put(uuid, myRunnable);
+        }
+    }
+
+    /**
+     * Calculates a unique identifier for a given |virtualViewId| and |eventType| pairing. This is
+     * used to replace the need for a Pair<> or wrapper object to hold two simple ints. We shift the
+     * |virtualViewId| 32 bits left, and bitwise OR with |eventType|, creating a 64-bit unique long.
+     *
+     * @param virtualViewId     This virtualViewId for the view trying to send an event.
+     * @param eventType         The AccessibilityEvent type.
+     * @return uuid             The uuid (universal unique id) for this pairing.
+     */
+    private long uuid(int virtualViewId, int eventType) {
+        return ((long) virtualViewId << 32) | ((long) eventType);
+    }
+}
\ No newline at end of file

content/public/android/java/src/org/chromium/content/browser/accessibility/WebContentsAccessibilityImpl.java

@@ -18,7 +18,6 @@ import android.provider.Settings;
 import android.text.SpannableString;
 import android.text.style.URLSpan;
 import android.util.SparseArray;
-import android.util.SparseLongArray;
 import android.view.MotionEvent;
 import android.view.View;
 import android.view.ViewGroup;
@@ -51,11 +50,8 @@ import org.chromium.content_public.browser.WebContentsAccessibility;
 import org.chromium.ui.base.WindowAndroid;
 
 import java.util.ArrayList;
-import java.util.Calendar;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
-import java.util.Set;
 
 /**
  * Implementation of {@link WebContentsAccessibility} interface.
@@ -118,9 +114,6 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
     // Constant for no granularity selected.
     private static final int NO_GRANULARITY_SELECTED = 0;
 
-    // Constant for throttling delay of successive accessibility events in milliseconds.
-    private static final int ACCESSIBILITY_EVENT_DELAY = 100;
-
     private final WebContentsImpl mWebContents;
     protected AccessibilityManager mAccessibilityManager;
     protected final Context mContext;
@@ -165,27 +158,13 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
     // Accessibility touch exploration state.
     private boolean mTouchExplorationEnabled;
 
-    // Unordered list of event types we will throttle if multiple events of the given type are sent
-    // in quick succession.
-    // TODO (mschillaci) - No event types added in M83 to disable all throttling.
-    private Set<Integer> mEventsToThrottle = new HashSet<Integer>();
-
-    // For events being throttled (see: |mEventsToThrottle|), this array will map the eventType
-    // to the last time (long in milliseconds) such an event has been sent.
-    private SparseLongArray mEventLastFiredTimes = new SparseLongArray();
-
-    // For events being throttled (see: |mEventsToThrottle|), this array will map the eventType
-    // to a single Runnable that will send an event after some delay.
-    private SparseArray<Runnable> mPendingEvents = new SparseArray<>();
-
     // This array maps a given virtualViewId to an |AccessibilityNodeInfo| for that view. We use
     // this to update a node quickly rather than building from one scratch each time.
-    // TODO (mschillaci) - Nothing is added to the cache in M83, it is disabled.
     private SparseArray<AccessibilityNodeInfo> mNodeInfoCache = new SparseArray<>();
 
-    // TODO (mschillaci) - Revert this change to re-enable throttling.
-    private Runnable mSendWindowContentChangedRunnable;
-    private static final int WINDOW_CONTENT_CHANGED_DELAY_MS = 500;
+    // This handles the dispatching of accessibility events. It acts as an intermediary where we can
+    // apply throttling rules, delay event construction, etc.
+    private AccessibilityEventDispatcher mEventDispatcher;
 
     /**
      * Create a WebContentsAccessibilityImpl object.
@@ -227,6 +206,29 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
         mCaptioningController = new CaptioningController(mWebContents);
         WindowEventObserverManager.from(mWebContents).addObserver(this);
 
+        mEventDispatcher =
+                new AccessibilityEventDispatcher(new AccessibilityEventDispatcher.Client() {
+                    @Override
+                    public void postRunnable(Runnable toPost, long delayInMilliseconds) {
+                        mView.postDelayed(toPost, delayInMilliseconds);
+                    }
+
+                    @Override
+                    public void removeRunnable(Runnable toRemove) {
+                        mView.removeCallbacks(toRemove);
+                    }
+
+                    @Override
+                    public boolean dispatchEvent(int virtualViewId, int eventType) {
+                        AccessibilityEvent event =
+                                buildAccessibilityEvent(virtualViewId, eventType);
+                        if (event == null) return false;
+
+                        mView.requestSendAccessibilityEvent(mView, event);
+                        return true;
+                    }
+                });
+
         // Native is initialized lazily, when node provider is actually requested.
     }
 
@@ -378,8 +380,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
             if (WebContentsAccessibilityImplJni.get().populateAccessibilityNodeInfo(
                         mNativeObj, WebContentsAccessibilityImpl.this, info, virtualViewId)) {
                 // After successfully populating this node, add it to our cache then return.
-                // TODO (mschillaci) - Uncomment this line to re-enable caching.
-                // mNodeInfoCache.put(virtualViewId, AccessibilityNodeInfo.obtain(info));
+                mNodeInfoCache.put(virtualViewId, AccessibilityNodeInfo.obtain(info));
                 return info;
             } else {
                 info.recycle();
@@ -417,9 +418,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
     public void setObscuredByAnotherView(boolean isObscured) {
         if (isObscured != mIsObscuredByAnotherView) {
             mIsObscuredByAnotherView = isObscured;
-            // TODO (mschillaci) - Revert this change to re-enable throttling.
-            // sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
-            mView.sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
+            sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
         }
     }
 
@@ -799,9 +798,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
 
         // Invalidate the container view, since the chrome accessibility tree is now
         // ready and listed as the child of the container view.
-        // TODO (mschillaci) - Revert this change to re-enable throttling.
-        // sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
-        sendWindowContentChangedOnView();
+        sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
 
         // (Re-) focus focused element, since we weren't able to create an
         // AccessibilityNodeInfo for this element before.
@@ -1070,30 +1067,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
      */
     @CalledByNative
     private void sendDelayedWindowContentChangedEvent() {
-        // TODO (mschillaci) - Revert this change to re-enable throttling.
-        if (mSendWindowContentChangedRunnable != null) return;
-
-        mSendWindowContentChangedRunnable = new Runnable() {
-            @Override
-            public void run() {
-                sendWindowContentChangedOnView();
-            }
-        };
-
-        mView.postDelayed(mSendWindowContentChangedRunnable, WINDOW_CONTENT_CHANGED_DELAY_MS);
-    }
-
-    private void sendWindowContentChangedOnView() {
-        if (mSendWindowContentChangedRunnable != null) {
-            mView.removeCallbacks(mSendWindowContentChangedRunnable);
-            mSendWindowContentChangedRunnable = null;
-        }
-        mView.sendAccessibilityEvent(AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
-    }
-
-    private void sendWindowContentChangedOnVirtualView(int virtualViewId) {
-        sendAccessibilityEvent(virtualViewId, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
-        // sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
+        sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
     }
 
     private void sendAccessibilityEvent(int virtualViewId, int eventType) {
@@ -1111,54 +1085,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
             return;
         }
 
-        // Check whether this type of event is one we want to throttle, and if not then send it
-        // TODO (mschillaci) - Will always return true until we enable throttling (see line: 171).
-        if (!mEventsToThrottle.contains(eventType)) {
-            AccessibilityEvent event = buildAccessibilityEvent(virtualViewId, eventType);
-            if (event == null) return;
-
-            mView.requestSendAccessibilityEvent(mView, event);
-            return;
-        }
-
-        // Check when we last fired an event. If we have not fired an event of this type, or the
-        // last time was longer than |ACCESSIBILITY_EVENT_DELAY| ago, then we allow this event
-        // to be sent immediately and record the time and clear any lingering callbacks.
-        long now = Calendar.getInstance().getTimeInMillis();
-        if (now - mEventLastFiredTimes.get(eventType, 0) >= ACCESSIBILITY_EVENT_DELAY) {
-            AccessibilityEvent event = buildAccessibilityEvent(virtualViewId, eventType);
-            // If accessibility is disabled or the node is invalid, returned event will be null.
-            if (event == null) return;
-
-            mView.requestSendAccessibilityEvent(mView, event);
-            mView.removeCallbacks(mPendingEvents.get(eventType));
-            mPendingEvents.remove(eventType);
-            mEventLastFiredTimes.put(eventType, now);
-        } else {
-            // We have fired an event within our |ACCESSIBILITY_EVENT_DELAY| delay window.
-            // Store this event, replacing any events in |mPendingEvents| of the same type,
-            // and set a delay equal to remaining time in our delay.
-            mView.removeCallbacks(mPendingEvents.get(eventType));
-
-            Runnable myRunnable = () -> {
-                // We have delayed firing this event, so accessibility may not be enabled or the
-                // node may be invalid, in which case build will return a null event.
-                AccessibilityEvent event = buildAccessibilityEvent(virtualViewId, eventType);
-                if (event != null) {
-                    mView.requestSendAccessibilityEvent(mView, event);
-                    // After sending event, record time it was sent
-                    mEventLastFiredTimes.put(eventType, Calendar.getInstance().getTimeInMillis());
-                }
-
-                // Remove callbacks and pending event
-                mView.removeCallbacks(mPendingEvents.get(eventType));
-                mPendingEvents.remove(eventType);
-            };
-
-            mView.postDelayed(myRunnable,
-                    (mEventLastFiredTimes.get(eventType, 0) + ACCESSIBILITY_EVENT_DELAY) - now);
-            mPendingEvents.put(eventType, myRunnable);
-        }
+        mEventDispatcher.enqueueEvent(virtualViewId, eventType);
     }
 
     private AccessibilityEvent buildAccessibilityEvent(int virtualViewId, int eventType) {
@@ -1316,13 +1243,9 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
                 mNativeObj, WebContentsAccessibilityImpl.this);
         if (rootId != mCurrentRootId) {
             mCurrentRootId = rootId;
-            // TODO (mschillaci) - Revert this change to re-enable throttling.
-            // sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
-            sendWindowContentChangedOnView();
+            sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
         } else {
-            // TODO (mschillaci) - Revert this change to re-enable throttling.
-            // sendAccessibilityEvent(id, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
-            sendWindowContentChangedOnVirtualView(id);
+            sendAccessibilityEvent(id, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
         }
     }
 
@@ -1332,9 +1255,7 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
         mAccessibilityFocusRect = null;
         mUserHasTouchExplored = false;
         // Invalidate the host, since its child is now gone.
-        // TODO (mschillaci) - Revert this change to re-enable throttling.
-        // sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
-        sendWindowContentChangedOnView();
+        sendAccessibilityEvent(View.NO_ID, AccessibilityEvent.TYPE_WINDOW_CONTENT_CHANGED);
     }
 
     @CalledByNative