Improve accessibility autofill performance (#4276)

app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/BitwardenAccessibilityService.kt

@@ -22,8 +22,7 @@ class BitwardenAccessibilityService : AccessibilityService() {
     lateinit var processor: BitwardenAccessibilityProcessor
 
     override fun onAccessibilityEvent(event: AccessibilityEvent) {
-        if (rootInActiveWindow?.packageName != event.packageName) return
+        processor.processAccessibilityEvent(event = event) { rootInActiveWindow }
-        processor.processAccessibilityEvent(rootAccessibilityNodeInfo = event.source)
     }
 
     override fun onInterrupt() = Unit

app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessor.kt

@@ -1,5 +1,6 @@
 package com.x8bit.bitwarden.data.autofill.accessibility.processor
 
+import android.view.accessibility.AccessibilityEvent
 import android.view.accessibility.AccessibilityNodeInfo
 
 /**
@@ -7,7 +8,12 @@ import android.view.accessibility.AccessibilityNodeInfo
  */
 interface BitwardenAccessibilityProcessor {
     /**
-     * Processes the [AccessibilityNodeInfo] for autofill options.
+     * Processes the [AccessibilityEvent] for autofill options and grant access to the current
+     * [AccessibilityNodeInfo] via the [rootAccessibilityNodeInfoProvider] (note that calling the
+     * `rootAccessibilityNodeInfoProvider` is expensive).
      */
-    fun processAccessibilityEvent(rootAccessibilityNodeInfo: AccessibilityNodeInfo?)
+    fun processAccessibilityEvent(
+        event: AccessibilityEvent,
+        rootAccessibilityNodeInfoProvider: () -> AccessibilityNodeInfo?,
+    )
 }

app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorImpl.kt

@@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.autofill.accessibility.processor
 
 import android.content.Context
 import android.os.PowerManager
+import android.view.accessibility.AccessibilityEvent
 import android.view.accessibility.AccessibilityNodeInfo
 import android.widget.Toast
 import com.x8bit.bitwarden.R
@@ -25,15 +26,18 @@ class BitwardenAccessibilityProcessorImpl(
     private val launcherPackageNameManager: LauncherPackageNameManager,
     private val powerManager: PowerManager,
 ) : BitwardenAccessibilityProcessor {
-    override fun processAccessibilityEvent(rootAccessibilityNodeInfo: AccessibilityNodeInfo?) {
+    override fun processAccessibilityEvent(
-        val rootNode = rootAccessibilityNodeInfo ?: return
+        event: AccessibilityEvent,
+        rootAccessibilityNodeInfoProvider: () -> AccessibilityNodeInfo?,
+    ) {
+        val eventNode = event.source ?: return
         // Ignore the event when the phone is inactive
         if (!powerManager.isInteractive) return
         // We skip if the system package
-        if (rootNode.isSystemPackage) return
+        if (eventNode.isSystemPackage) return
         // We skip any package that is a launcher or unsupported
-        if (rootNode.shouldSkipPackage ||
+        if (eventNode.shouldSkipPackage ||
-            launcherPackageNameManager.launcherPackages.any { it == rootNode.packageName }
+            launcherPackageNameManager.launcherPackages.any { it == eventNode.packageName }
         ) {
             // Clear the action since this event needs to be ignored completely
             accessibilityAutofillManager.accessibilityAction = null
@@ -42,14 +46,19 @@ class BitwardenAccessibilityProcessorImpl(
 
         // Only process the event if the tile was clicked
         val accessibilityAction = accessibilityAutofillManager.accessibilityAction ?: return
+        // We only call for the root node once after all other checks
+        // have passed because it is significant performance hit
+        if (rootAccessibilityNodeInfoProvider()?.packageName != event.packageName) return
+
+        // Clear the action since we are now acting on it
         accessibilityAutofillManager.accessibilityAction = null
 
         when (accessibilityAction) {
             is AccessibilityAction.AttemptFill -> {
-                handleAttemptFill(rootNode = rootNode, attemptFill = accessibilityAction)
+                handleAttemptFill(rootNode = eventNode, attemptFill = accessibilityAction)
             }
 
-            AccessibilityAction.AttemptParseUri -> handleAttemptParseUri(rootNode = rootNode)
+            AccessibilityAction.AttemptParseUri -> handleAttemptParseUri(rootNode = eventNode)
         }
     }
 

app/src/test/java/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorTest.kt

@@ -3,6 +3,7 @@ package com.x8bit.bitwarden.data.autofill.accessibility.processor
 import android.content.Context
 import android.net.Uri
 import android.os.PowerManager
+import android.view.accessibility.AccessibilityEvent
 import android.view.accessibility.AccessibilityNodeInfo
 import android.widget.Toast
 import com.bitwarden.vault.CipherView
@@ -74,10 +75,12 @@ class BitwardenAccessibilityProcessorTest {
     }
 
     @Test
-    fun `processAccessibilityEvent with null rootNode should return`() {
+    fun `processAccessibilityEvent with null event source should return`() {
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        val event = mockk<AccessibilityEvent> {
-            rootAccessibilityNodeInfo = null,
+            every { source } returns null
-        )
+        }
+
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { null }
 
         verify(exactly = 0) {
             powerManager.isInteractive
@@ -86,12 +89,12 @@ class BitwardenAccessibilityProcessorTest {
 
     @Test
     fun `processAccessibilityEvent with powerManager not interactive should return`() {
-        val rootNode = mockk<AccessibilityNodeInfo>()
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns mockk()
+        }
         every { powerManager.isInteractive } returns false
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { null }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
@@ -100,38 +103,40 @@ class BitwardenAccessibilityProcessorTest {
 
     @Test
     fun `processAccessibilityEvent with system package should return`() {
-        val rootNode = mockk<AccessibilityNodeInfo> {
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns true
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+        }
         every { powerManager.isInteractive } returns true
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { null }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.isSystemPackage
+            node.isSystemPackage
         }
     }
 
     @Test
     fun `processAccessibilityEvent with skippable package should return`() {
-        val rootNode = mockk<AccessibilityNodeInfo> {
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns false
             every { shouldSkipPackage } returns true
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+        }
         every { powerManager.isInteractive } returns true
         every { accessibilityAutofillManager.accessibilityAction = null } just runs
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { null }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.isSystemPackage
+            node.isSystemPackage
-            rootNode.shouldSkipPackage
+            node.shouldSkipPackage
             accessibilityAutofillManager.accessibilityAction = null
         }
     }
@@ -139,23 +144,24 @@ class BitwardenAccessibilityProcessorTest {
     @Test
     fun `processAccessibilityEvent with launcher package should return`() {
         val testPackageName = "com.google.android.launcher"
-        val rootNode = mockk<AccessibilityNodeInfo> {
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns false
             every { shouldSkipPackage } returns false
             every { packageName } returns testPackageName
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+        }
         every { launcherPackageNameManager.launcherPackages } returns listOf(testPackageName)
         every { powerManager.isInteractive } returns true
         every { accessibilityAutofillManager.accessibilityAction = null } just runs
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { null }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.isSystemPackage
+            node.isSystemPackage
-            rootNode.shouldSkipPackage
+            node.shouldSkipPackage
             launcherPackageNameManager.launcherPackages
             accessibilityAutofillManager.accessibilityAction = null
         }
@@ -164,23 +170,58 @@ class BitwardenAccessibilityProcessorTest {
     @Test
     fun `processAccessibilityEvent without accessibility action should return`() {
         val testPackageName = "com.android.chrome"
-        val rootNode = mockk<AccessibilityNodeInfo> {
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns false
             every { shouldSkipPackage } returns false
             every { packageName } returns testPackageName
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+        }
         every { launcherPackageNameManager.launcherPackages } returns emptyList()
         every { accessibilityAutofillManager.accessibilityAction } returns null
         every { powerManager.isInteractive } returns true
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { null }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.shouldSkipPackage
+            node.shouldSkipPackage
-            rootNode.isSystemPackage
+            node.isSystemPackage
+            launcherPackageNameManager.launcherPackages
+            accessibilityAutofillManager.accessibilityAction
+        }
+    }
+
+    @Test
+    fun `processAccessibilityEvent with mismatched package name should return`() {
+        val testPackageName = "com.android.chrome"
+        val rootNode = mockk<AccessibilityNodeInfo> {
+            every { packageName } returns "other.package.name"
+        }
+        val node = mockk<AccessibilityNodeInfo> {
+            every { isSystemPackage } returns false
+            every { shouldSkipPackage } returns false
+            every { packageName } returns testPackageName
+        }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+            every { packageName } returns testPackageName
+        }
+        every { powerManager.isInteractive } returns true
+        every { launcherPackageNameManager.launcherPackages } returns emptyList()
+        every {
+            accessibilityAutofillManager.accessibilityAction
+        } returns AccessibilityAction.AttemptParseUri
+        every { accessibilityAutofillManager.accessibilityAction = null } just runs
+        every { accessibilityParser.parseForUriOrPackageName(rootNode = node) } returns null
+
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { rootNode }
+
+        verify(exactly = 1) {
+            powerManager.isInteractive
+            node.isSystemPackage
+            node.shouldSkipPackage
             launcherPackageNameManager.launcherPackages
             accessibilityAutofillManager.accessibilityAction
         }
@@ -190,30 +231,35 @@ class BitwardenAccessibilityProcessorTest {
     fun `processAccessibilityEvent with AttemptParseUri and a invalid uri should show a toast`() {
         val testPackageName = "com.android.chrome"
         val rootNode = mockk<AccessibilityNodeInfo> {
+            every { packageName } returns testPackageName
+        }
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns false
             every { shouldSkipPackage } returns false
             every { packageName } returns testPackageName
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+            every { packageName } returns testPackageName
+        }
         every { powerManager.isInteractive } returns true
         every { launcherPackageNameManager.launcherPackages } returns emptyList()
         every {
             accessibilityAutofillManager.accessibilityAction
         } returns AccessibilityAction.AttemptParseUri
         every { accessibilityAutofillManager.accessibilityAction = null } just runs
-        every { accessibilityParser.parseForUriOrPackageName(rootNode = rootNode) } returns null
+        every { accessibilityParser.parseForUriOrPackageName(rootNode = node) } returns null
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { rootNode }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.isSystemPackage
+            node.isSystemPackage
-            rootNode.shouldSkipPackage
+            node.shouldSkipPackage
             launcherPackageNameManager.launcherPackages
             accessibilityAutofillManager.accessibilityAction
             accessibilityAutofillManager.accessibilityAction = null
-            accessibilityParser.parseForUriOrPackageName(rootNode = rootNode)
+            accessibilityParser.parseForUriOrPackageName(rootNode = node)
             Toast
                 .makeText(context, R.string.autofill_tile_uri_not_found, Toast.LENGTH_LONG)
                 .show()
@@ -225,10 +271,17 @@ class BitwardenAccessibilityProcessorTest {
     fun `processAccessibilityEvent with AttemptParseUri and a valid uri should start the main activity`() {
         val testPackageName = "com.android.chrome"
         val rootNode = mockk<AccessibilityNodeInfo> {
+            every { packageName } returns testPackageName
+        }
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns false
             every { shouldSkipPackage } returns false
             every { packageName } returns testPackageName
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+            every { packageName } returns testPackageName
+        }
         every { powerManager.isInteractive } returns true
         every { launcherPackageNameManager.launcherPackages } returns emptyList()
         every {
@@ -243,20 +296,18 @@ class BitwardenAccessibilityProcessorTest {
                 uri = any(),
             )
         } returns mockk()
-        every { accessibilityParser.parseForUriOrPackageName(rootNode = rootNode) } returns mockk()
+        every { accessibilityParser.parseForUriOrPackageName(rootNode = node) } returns mockk()
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { rootNode }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.isSystemPackage
+            node.isSystemPackage
-            rootNode.shouldSkipPackage
+            node.shouldSkipPackage
             launcherPackageNameManager.launcherPackages
             accessibilityAutofillManager.accessibilityAction
             accessibilityAutofillManager.accessibilityAction = null
-            accessibilityParser.parseForUriOrPackageName(rootNode = rootNode)
+            accessibilityParser.parseForUriOrPackageName(rootNode = node)
             createAutofillSelectionIntent(
                 context = context,
                 framework = AutofillSelectionData.Framework.ACCESSIBILITY,
@@ -276,23 +327,28 @@ class BitwardenAccessibilityProcessorTest {
         val uri = mockk<Uri>()
         val attemptFill = AccessibilityAction.AttemptFill(cipherView = cipherView, uri = uri)
         val rootNode = mockk<AccessibilityNodeInfo> {
+            every { packageName } returns testPackageName
+        }
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns false
             every { shouldSkipPackage } returns false
             every { packageName } returns testPackageName
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+            every { packageName } returns testPackageName
+        }
         every { powerManager.isInteractive } returns true
         every { launcherPackageNameManager.launcherPackages } returns emptyList()
         every { accessibilityAutofillManager.accessibilityAction } returns attemptFill
         every { accessibilityAutofillManager.accessibilityAction = null } just runs
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { rootNode }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.isSystemPackage
+            node.isSystemPackage
-            rootNode.shouldSkipPackage
+            node.shouldSkipPackage
             launcherPackageNameManager.launcherPackages
             accessibilityAutofillManager.accessibilityAction
             accessibilityAutofillManager.accessibilityAction = null
@@ -325,31 +381,36 @@ class BitwardenAccessibilityProcessorTest {
         val uri = mockk<Uri>()
         val attemptFill = AccessibilityAction.AttemptFill(cipherView = cipherView, uri = uri)
         val rootNode = mockk<AccessibilityNodeInfo> {
+            every { packageName } returns testPackageName
+        }
+        val node = mockk<AccessibilityNodeInfo> {
             every { isSystemPackage } returns false
             every { shouldSkipPackage } returns false
             every { packageName } returns testPackageName
         }
+        val event = mockk<AccessibilityEvent> {
+            every { source } returns node
+            every { packageName } returns testPackageName
+        }
         every { powerManager.isInteractive } returns true
         every { launcherPackageNameManager.launcherPackages } returns emptyList()
         every { accessibilityAutofillManager.accessibilityAction } returns attemptFill
         every { accessibilityAutofillManager.accessibilityAction = null } just runs
         every {
-            accessibilityParser.parseForFillableFields(rootNode = rootNode, uri = uri)
+            accessibilityParser.parseForFillableFields(rootNode = node, uri = uri)
         } returns fillableFields
 
-        bitwardenAccessibilityProcessor.processAccessibilityEvent(
+        bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { rootNode }
-            rootAccessibilityNodeInfo = rootNode,
-        )
 
         verify(exactly = 1) {
             powerManager.isInteractive
-            rootNode.isSystemPackage
+            node.isSystemPackage
-            rootNode.shouldSkipPackage
+            node.shouldSkipPackage
             launcherPackageNameManager.launcherPackages
             accessibilityAutofillManager.accessibilityAction
             accessibilityAutofillManager.accessibilityAction = null
             cipherView.login
-            accessibilityParser.parseForFillableFields(rootNode = rootNode, uri = uri)
+            accessibilityParser.parseForFillableFields(rootNode = node, uri = uri)
             mockUsernameField.fillTextField(testUsername)
             mockPasswordField.fillTextField(testPassword)
         }