diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorImpl.kt index 342c4085163..12cadf3448a 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorImpl.kt @@ -33,14 +33,13 @@ class BitwardenAccessibilityProcessorImpl( ) { // Only process the event if the tile was clicked val accessibilityAction = accessibilityAutofillManager.accessibilityAction ?: return - val eventNode = event.source ?: return // Prevent clearing the action until we receive a processable event in case unprocessable // events are still being received from the device. This can happen on slower devices or if // screen transitions are still being performed. - if (!eventNode.shouldProcessEvent(rootAccessibilityNodeInfoProvider)) { - return - } + val eventNode = event + .getValidNode(rootAccessibilityNodeInfoProvider = rootAccessibilityNodeInfoProvider) + ?: return // Clear the action since we are now acting on a supported node. accessibilityAutofillManager.accessibilityAction = null @@ -53,35 +52,43 @@ class BitwardenAccessibilityProcessorImpl( } } - private fun AccessibilityNodeInfo.shouldProcessEvent( + private fun AccessibilityEvent.getValidNode( rootAccessibilityNodeInfoProvider: () -> AccessibilityNodeInfo?, - ): Boolean { + ): AccessibilityNodeInfo? { + val eventNode = this + .source + ?: run { + Timber.w("Accessibility event source is null, attempting root node") + // We only call for the root node once, after verifying that the there is an action + // to be filled. This is because it is a significant performance hit. + // Additionally, we do not use the root node if it does not have the same package + // as the triggering event. + rootAccessibilityNodeInfoProvider()?.takeIf { it.packageName == this.packageName } + } + ?: run { + Timber.w("Root node was also null, skipping this event") + return null + } + // Ignore the event when the phone is inactive. - if (!powerManager.isInteractive) return false + if (!powerManager.isInteractive) return null // We skip if the system package. - if (this.isSystemPackage) { - Timber.d("Skipping autofill for system package $packageName.") - return false + if (eventNode.isSystemPackage) { + Timber.d("Skipping autofill for system package ${eventNode.packageName}.") + return null } // We skip any package that is explicitly blocked. - if (this.shouldSkipPackage) { - Timber.d("Skipping autofill on block-listed package $packageName.") - return false + if (eventNode.shouldSkipPackage) { + Timber.d("Skipping autofill on block-listed package ${eventNode.packageName}.") + return null } // We skip any package that is a launcher. - if (launcherPackageNameManager.launcherPackages.any { it == this.packageName }) { - Timber.d("Skipping autofill on launcher package $packageName.") - return false - } - - // We only call for the root node once, after all other checks have passed, because it is a - // significant performance hit. - if (rootAccessibilityNodeInfoProvider()?.packageName != this.packageName) { - Timber.d("Skipping autofill due to package name mismatch.") - return false + if (launcherPackageNameManager.launcherPackages.any { it == eventNode.packageName }) { + Timber.d("Skipping autofill on launcher package ${eventNode.packageName}.") + return null } - return true + return eventNode } private fun handleAttemptParseUri(rootNode: AccessibilityNodeInfo) { diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorTest.kt index abd1d4f001c..1151e9b6da2 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/accessibility/processor/BitwardenAccessibilityProcessorTest.kt @@ -77,7 +77,7 @@ class BitwardenAccessibilityProcessorTest { } @Test - fun `processAccessibilityEvent with null event source should return`() { + fun `processAccessibilityEvent with null event source and root node should return`() { val event = mockk { every { source } returns null } @@ -89,6 +89,41 @@ class BitwardenAccessibilityProcessorTest { } } + @Test + fun `processAccessibilityEvent with null event source and invalid root node should return`() { + val event = mockk { + every { source } returns null + every { packageName } returns "event_package" + } + val rootNode = mockk { + every { packageName } returns "other_package" + } + + bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { rootNode } + + verify(exactly = 0) { + powerManager.isInteractive + } + } + + @Test + fun `processAccessibilityEvent with null event source and valid root node should continue`() { + val event = mockk { + every { source } returns null + every { packageName } returns "package" + } + val rootNode = mockk { + every { packageName } returns "package" + } + every { powerManager.isInteractive } returns false + + bitwardenAccessibilityProcessor.processAccessibilityEvent(event = event) { rootNode } + + verify(exactly = 1) { + powerManager.isInteractive + } + } + @Test fun `processAccessibilityEvent with powerManager not interactive should return`() { val event = mockk { @@ -184,40 +219,6 @@ class BitwardenAccessibilityProcessorTest { } } - @Test - fun `processAccessibilityEvent with mismatched package name should return`() { - val testPackageName = "com.android.chrome" - val rootNode = mockk { - every { packageName } returns "other.package.name" - } - val node = mockk { - every { isSystemPackage } returns false - every { shouldSkipPackage } returns false - every { packageName } returns testPackageName - } - val event = mockk { - 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 - } - } - @Test fun `processAccessibilityEvent with AttemptParseUri and a invalid uri should show a toast`() { val testPackageName = "com.android.chrome"