[Refactor] Eliminate some JNI trips for a11y Android code

This CL includes some small refactors to JNI calls for a11y
Android code.

We eliminate some trips through the JNI with the following updates:

 - announceLiveRegion checks empty strings before JNI call
 - Combine 4 populate AccessibilityEvent JNI calls to 1
 - Combine 2 populate AccessibilityNodeInfo JNI calls to 1
 - Update setting AccessibilityNodeInfo child ids to pass a
   vector once, rather than calling through JNI for each child

AX-Relnotes: N/A
Change-Id: Ic6c46b8316ebc954a20084844f5f072149a218d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2569852Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Mark Schillaci <mschillaci@google.com>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Cr-Commit-Position: refs/heads/master@{#833454}

content/browser/accessibility/web_contents_accessibility_android.cc

@@ -500,6 +500,11 @@ void WebContentsAccessibilityAndroid::AnnounceLiveRegionText(
   ScopedJavaLocalRef<jobject> obj = java_ref_.get(env);
   if (obj.is_null())
     return;
+
+  // Do not announce empty text.
+  if (text.empty())
+    return;
+
   Java_WebContentsAccessibilityImpl_announceLiveRegionText(
       env, obj, base::android::ConvertUTF16ToJavaString(env, text));
 }
@@ -750,19 +755,28 @@ jboolean WebContentsAccessibilityAndroid::PopulateAccessibilityNodeInfo(
     Java_WebContentsAccessibilityImpl_setAccessibilityNodeInfoParent(
         env, obj, info, android_node->unique_id());
   }
+
+  // Build a vector of child ids
+  std::vector<int> child_ids;
   for (BrowserAccessibility::PlatformChildIterator it =
            node->PlatformChildrenBegin();
        it != node->PlatformChildrenEnd(); ++it) {
     auto* android_node = static_cast<BrowserAccessibilityAndroid*>(it.get());
-    Java_WebContentsAccessibilityImpl_addAccessibilityNodeInfoChild(
-        env, obj, info, android_node->unique_id());
+    child_ids.push_back(android_node->unique_id());
+  }
+  if (child_ids.size()) {
+    Java_WebContentsAccessibilityImpl_addAccessibilityNodeInfoChildren(
+        env, obj, info,
+        base::android::ToJavaIntArray(env, child_ids.data(), child_ids.size()));
   }
+
   Java_WebContentsAccessibilityImpl_setAccessibilityNodeInfoBooleanAttributes(
       env, obj, info, unique_id, node->IsReportingCheckable(),
       node->IsChecked(), node->IsClickable(), node->IsContentInvalid(),
       node->IsEnabled(), node->IsFocusable(), node->IsFocused(),
       node->HasImage(), node->IsPasswordField(), node->IsScrollable(),
       node->IsSelected(), node->IsVisibleToUser());
+
   Java_WebContentsAccessibilityImpl_addAccessibilityNodeInfoActions(
       env, obj, info, unique_id, node->CanScrollForward(),
       node->CanScrollBackward(), node->CanScrollUp(), node->CanScrollDown(),
@@ -778,7 +792,11 @@ jboolean WebContentsAccessibilityAndroid::PopulateAccessibilityNodeInfo(
       base::android::ConvertUTF8ToJavaString(env, node->GetRoleString()),
       base::android::ConvertUTF16ToJavaString(env, node->GetRoleDescription()),
       base::android::ConvertUTF16ToJavaString(env, node->GetHint()),
-      base::android::ConvertUTF16ToJavaString(env, node->GetTargetUrl()));
+      base::android::ConvertUTF16ToJavaString(env, node->GetTargetUrl()),
+      node->CanOpenPopup(), node->IsDismissable(), node->IsMultiLine(),
+      node->AndroidInputType(), node->AndroidLiveRegionType(),
+      base::android::ConvertUTF16ToJavaString(
+          env, node->GetContentInvalidErrorMessage()));
 
   ScopedJavaLocalRef<jintArray> suggestion_starts_java;
   ScopedJavaLocalRef<jintArray> suggestion_ends_java;
@@ -819,13 +837,6 @@ jboolean WebContentsAccessibilityAndroid::PopulateAccessibilityNodeInfo(
 
   UpdateAccessibilityNodeInfoBoundsRect(env, obj, info, unique_id, node);
 
-  Java_WebContentsAccessibilityImpl_setAccessibilityNodeInfoAttributes(
-      env, obj, info, node->CanOpenPopup(), node->IsDismissable(),
-      node->IsMultiLine(), node->AndroidInputType(),
-      node->AndroidLiveRegionType(),
-      base::android::ConvertUTF16ToJavaString(
-          env, node->GetContentInvalidErrorMessage()));
-
   Java_WebContentsAccessibilityImpl_setAccessibilityNodeInfoOAttributes(
       env, obj, info, node->HasCharacterLocations(),
       base::android::ConvertUTF16ToJavaString(env, node->GetHint()));
@@ -870,17 +881,13 @@ jboolean WebContentsAccessibilityAndroid::PopulateAccessibilityEvent(
   if (!node)
     return false;
 
-  Java_WebContentsAccessibilityImpl_setAccessibilityEventBooleanAttributes(
+  // We will always set boolean, classname, list and scroll attributes.
+  Java_WebContentsAccessibilityImpl_setAccessibilityEventBaseAttributes(
       env, obj, event, node->IsChecked(), node->IsEnabled(),
-      node->IsPasswordField(), node->IsScrollable());
-  Java_WebContentsAccessibilityImpl_setAccessibilityEventClassName(
-      env, obj, event,
+      node->IsPasswordField(), node->IsScrollable(), node->GetItemIndex(),
+      node->GetItemCount(), node->GetScrollX(), node->GetScrollY(),
+      node->GetMaxScrollX(), node->GetMaxScrollY(),
       base::android::ConvertUTF8ToJavaString(env, node->GetClassName()));
-  Java_WebContentsAccessibilityImpl_setAccessibilityEventListAttributes(
-      env, obj, event, node->GetItemIndex(), node->GetItemCount());
-  Java_WebContentsAccessibilityImpl_setAccessibilityEventScrollAttributes(
-      env, obj, event, node->GetScrollX(), node->GetScrollY(),
-      node->GetMaxScrollX(), node->GetMaxScrollY());
 
   switch (event_type) {
     case ANDROID_ACCESSIBILITY_EVENT_TEXT_CHANGED: {

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

@@ -1369,8 +1369,10 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
     }
 
     @CalledByNative
-    private void addAccessibilityNodeInfoChild(AccessibilityNodeInfo node, int childId) {
-        node.addChild(mView, childId);
+    private void addAccessibilityNodeInfoChildren(AccessibilityNodeInfo node, int[] childIds) {
+        for (int childId : childIds) {
+            node.addChild(mView, childId);
+        }
     }
 
     @CalledByNative
@@ -1535,7 +1537,9 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
 
     @CalledByNative
     private void setAccessibilityNodeInfoBaseAttributes(AccessibilityNodeInfo node, boolean isRoot,
-            String className, String role, String roleDescription, String hint, String targetUrl) {
+            String className, String role, String roleDescription, String hint, String targetUrl,
+            boolean canOpenPopup, boolean dismissable, boolean multiLine, int inputType,
+            int liveRegion, String errorMessage) {
         node.setClassName(className);
 
         Bundle bundle = node.getExtras();
@@ -1549,6 +1553,22 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
             bundle.putCharSequence(
                     "ACTION_ARGUMENT_HTML_ELEMENT_STRING_VALUES", mSupportedHtmlElementTypes);
         }
+
+        node.setCanOpenPopup(canOpenPopup);
+        node.setDismissable(dismissable);
+        node.setMultiLine(multiLine);
+        node.setInputType(inputType);
+
+        // Deliberately don't call setLiveRegion because TalkBack speaks
+        // the entire region anytime it changes. Instead Chrome will
+        // call announceLiveRegionText() only on the nodes that change.
+        // node.setLiveRegion(liveRegion);
+
+        // We only apply the |errorMessage| if {@link setAccessibilityNodeInfoBooleanAttributes}
+        // set |contentInvalid| to true based on throttle delay.
+        if (node.isContentInvalid()) {
+            node.setError(errorMessage);
+        }
     }
 
     @SuppressLint("NewApi")
@@ -1696,27 +1716,6 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
         }
     }
 
-    @CalledByNative
-    protected void setAccessibilityNodeInfoAttributes(AccessibilityNodeInfo node,
-            boolean canOpenPopup, boolean dismissable, boolean multiLine, int inputType,
-            int liveRegion, String errorMessage) {
-        node.setCanOpenPopup(canOpenPopup);
-        node.setDismissable(dismissable);
-        node.setMultiLine(multiLine);
-        node.setInputType(inputType);
-
-        // Deliberately don't call setLiveRegion because TalkBack speaks
-        // the entire region anytime it changes. Instead Chrome will
-        // call announceLiveRegionText() only on the nodes that change.
-        // node.setLiveRegion(liveRegion);
-
-        // We only apply the |errorMessage| if {@link setAccessibilityNodeInfoBooleanAttributes}
-        // set |contentInvalid| to true based on throttle delay.
-        if (node.isContentInvalid()) {
-            node.setError(errorMessage);
-        }
-    }
-
     @CalledByNative
     protected void setAccessibilityNodeInfoCollectionInfo(
             AccessibilityNodeInfo node, int rowCount, int columnCount, boolean hierarchical) {
@@ -1762,33 +1761,21 @@ public class WebContentsAccessibilityImpl extends AccessibilityNodeProvider
     }
 
     @CalledByNative
-    private void setAccessibilityEventBooleanAttributes(AccessibilityEvent event, boolean checked,
-            boolean enabled, boolean password, boolean scrollable) {
+    private void setAccessibilityEventBaseAttributes(AccessibilityEvent event, boolean checked,
+            boolean enabled, boolean password, boolean scrollable, int currentItemIndex,
+            int itemCount, int scrollX, int scrollY, int maxScrollX, int maxScrollY,
+            String className) {
         event.setChecked(checked);
         event.setEnabled(enabled);
         event.setPassword(password);
         event.setScrollable(scrollable);
-    }
-
-    @CalledByNative
-    private void setAccessibilityEventClassName(AccessibilityEvent event, String className) {
-        event.setClassName(className);
-    }
-
-    @CalledByNative
-    private void setAccessibilityEventListAttributes(
-            AccessibilityEvent event, int currentItemIndex, int itemCount) {
         event.setCurrentItemIndex(currentItemIndex);
         event.setItemCount(itemCount);
-    }
-
-    @CalledByNative
-    private void setAccessibilityEventScrollAttributes(
-            AccessibilityEvent event, int scrollX, int scrollY, int maxScrollX, int maxScrollY) {
         event.setScrollX(scrollX);
         event.setScrollY(scrollY);
         event.setMaxScrollX(maxScrollX);
         event.setMaxScrollY(maxScrollY);
+        event.setClassName(className);
     }
 
     @CalledByNative