Automation: Improve reliability one Android 14 devices (OnePlus & Redmi)

Stop using the root from accessibility events as fallback. Delayed event emission can lead to an out-dated (and later recycled) root being used by the step-process. Error behavior: * Click event from app 1 is emitted when SD Maid starts processing app 2 * windowRoot of app 2 is not ready yet * SD Maid starts using fallback root from the last event (the click event from app 2) * The fallback root is no longer valid, as it corresponds to the settings screen from app 1 * SD Maids keeps retrying and the fallback root is at some point recycled * SD Maid keeps trying to crawl an empty root nodef Fixes #1124 Might fix #1016
d4rken-org · May 1, 2024 · 7bfe1db · 7bfe1db
1 parent 0149cd4
commit 7bfe1db
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 46 deletions.
diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/AutomationExtensions.kt b/app/src/main/java/eu/darken/sdmse/automation/core/AutomationExtensions.kt
@@ -1,5 +1,27 @@
 package eu.darken.sdmse.automation.core
 
+import android.view.accessibility.AccessibilityNodeInfo
+import eu.darken.sdmse.common.debug.Bugs
+import eu.darken.sdmse.common.debug.logging.Logging.Priority.VERBOSE
+import eu.darken.sdmse.common.debug.logging.log
+import kotlinx.coroutines.CancellationException
+import kotlinx.coroutines.currentCoroutineContext
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.isActive
 
-suspend fun AutomationManager.canUseAcsNow(): Boolean = useAcs.first()
+suspend fun AutomationManager.canUseAcsNow(): Boolean = useAcs.first()
+
+suspend fun AutomationHost.waitForWindowRoot(delayMs: Long = 200): AccessibilityNodeInfo {
+    var root: AccessibilityNodeInfo? = null
+
+    while (currentCoroutineContext().isActive) {
+        root = windowRoot()
+        if (root != null) break
+
+        if (Bugs.isDebug) log(VERBOSE) { "Waiting for windowRoot..." }
+        delay(delayMs)
+    }
+
+    return root ?: throw CancellationException("Cancelled while waiting for windowRoot")
+}
diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/AutomationHost.kt b/app/src/main/java/eu/darken/sdmse/automation/core/AutomationHost.kt
@@ -18,7 +18,7 @@ interface AutomationHost : Progress.Client {
 
     val scope: CoroutineScope
 
-    suspend fun windowRoot(): AccessibilityNodeInfo
+    suspend fun windowRoot(): AccessibilityNodeInfo?
 
     suspend fun changeOptions(action: (Options) -> Options)
 

diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/AutomationService.kt b/app/src/main/java/eu/darken/sdmse/automation/core/AutomationService.kt
@@ -14,8 +14,7 @@ import androidx.annotation.Keep
 import androidx.appcompat.view.ContextThemeWrapper
 import dagger.hilt.android.AndroidEntryPoint
 import eu.darken.sdmse.R
-import eu.darken.sdmse.automation.core.common.AutomationException
-import eu.darken.sdmse.automation.core.common.getRoot
+import eu.darken.sdmse.automation.core.common.toStringShort
 import eu.darken.sdmse.automation.core.errors.AutomationNoConsentException
 import eu.darken.sdmse.automation.core.errors.UserCancelledAutomationException
 import eu.darken.sdmse.automation.ui.AutomationControlView
@@ -225,25 +224,6 @@ class AutomationService : AccessibilityService(), AutomationHost, Progress.Host,
 
         if (Bugs.isDebug) log(TAG, VERBOSE) { "New automation event: $eventCopy" }
 
-        serviceScope.launch {
-            try {
-                eventCopy.source
-                    ?.getRoot(maxNesting = Int.MAX_VALUE)
-                    ?.let {
-                        fallbackMutex.withLock {
-                            if (!hasApiLevel(30)) {
-                                @Suppress("DEPRECATION")
-                                fallbackRoot?.recycle()
-                            }
-                            fallbackRoot = it
-                        }
-                    }
-                    .also { log(TAG, VERBOSE) { "Fallback root was $fallbackRoot, now is $it" } }
-            } catch (e: Exception) {
-                log(TAG, ERROR) { "Failed to get fallbackRoot from $event: $e" }
-            }
-        }
-
         serviceScope.launch {
             // If we need fallbackRoot, don't race it
             delay(50)
@@ -252,16 +232,10 @@ class AutomationService : AccessibilityService(), AutomationHost, Progress.Host,
         }
     }
 
-    private val fallbackMutex = Mutex()
-    private var fallbackRoot: AccessibilityNodeInfo? = null
-
-    override suspend fun windowRoot(): AccessibilityNodeInfo = suspendCancellableCoroutine {
-        val maybeRootNode: AccessibilityNodeInfo? = rootInActiveWindow ?: fallbackRoot?.also {
-            log(TAG, WARN) { "Using fallback rootNode: $it" }
-        }
-
-        log(TAG, VERBOSE) { "Providing window root: $maybeRootNode" }
-        it.resume(maybeRootNode ?: throw AutomationException("Root node is currently null"))
+    override suspend fun windowRoot(): AccessibilityNodeInfo? = suspendCancellableCoroutine {
+        val rootNode: AccessibilityNodeInfo? = rootInActiveWindow
+        log(TAG, VERBOSE) { "Providing windowRoot: ${rootNode?.toStringShort()}" }
+        it.resume(rootNode)
     }
 
     private val controlLp: WindowManager.LayoutParams = WindowManager.LayoutParams().apply {

diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/common/AccessibilityNodeExtensions.kt b/app/src/main/java/eu/darken/sdmse/automation/core/common/AccessibilityNodeExtensions.kt
@@ -14,8 +14,10 @@ import java.util.concurrent.LinkedBlockingDeque
 
 private val TAG: String = logTag("Automation", "Crawler", "Common")
 
-fun AccessibilityNodeInfo.toStringShort() =
-    "className=${this.className}, text='${this.text}', isClickable=${this.isClickable}, isEnabled=${this.isEnabled}, viewIdResourceName=${this.viewIdResourceName}, pkgName=${this.packageName}"
+fun AccessibilityNodeInfo.toStringShort(): String {
+    val identity = Integer.toHexString(System.identityHashCode(this))
+    return "className=${this.className}, text='${this.text}', isClickable=${this.isClickable}, isEnabled=${this.isEnabled}, viewIdResourceName=${this.viewIdResourceName}, pkgName=${this.packageName}, identity=$identity"
+}
 
 val AccessibilityNodeInfo.textVariants: Set<String>
     get() {

diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/common/StepProcessor.kt b/app/src/main/java/eu/darken/sdmse/automation/core/common/StepProcessor.kt
@@ -13,6 +13,7 @@ import eu.darken.sdmse.automation.core.errors.AutomationException
 import eu.darken.sdmse.automation.core.errors.PlanAbortException
 import eu.darken.sdmse.automation.core.errors.ScreenUnavailableException
 import eu.darken.sdmse.automation.core.errors.StepAbortException
+import eu.darken.sdmse.automation.core.waitForWindowRoot
 import eu.darken.sdmse.common.R
 import eu.darken.sdmse.common.ca.CaDrawable
 import eu.darken.sdmse.common.ca.CaString
@@ -140,7 +141,7 @@ class StepProcessor @AssistedInject constructor(
             var currentRoot: AccessibilityNodeInfo? = null
 
             while (step.windowNodeTest != null && currentCoroutineContext().isActive) {
-                currentRoot = host.windowRoot().apply {
+                currentRoot = host.waitForWindowRoot().apply {
                     if (Bugs.isDebug) {
                         log(TAG, VERBOSE) { "Looking for viable window root, current nodes:" }
                         crawl().forEach { log(TAG, VERBOSE) { it.infoShort } }
@@ -151,11 +152,11 @@ class StepProcessor @AssistedInject constructor(
                     break
                 } else {
                     log(TAG) { "Not a viable root node: $currentRoot (spec=$step)" }
-                    delay(200)
+                    delay(500)
                 }
             }
 
-            currentRoot ?: host.windowRoot()
+            currentRoot ?: host.waitForWindowRoot()
         }
         log(TAG, VERBOSE) { "Current window root node is ${targetWindowRoot.toStringShort()}" }
 
@@ -189,12 +190,12 @@ class StepProcessor @AssistedInject constructor(
                         delay(100)
                     }
                     // Let's try a new one
-                    currentRootNode = host.windowRoot()
+                    currentRootNode = host.waitForWindowRoot()
                 }
                 target!!
             }
 
-            else -> host.windowRoot()
+            else -> host.waitForWindowRoot()
         }
         log(TAG, VERBOSE) { "Target node is ${targetNode.toStringShort()}" }
 

diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/debug/DebugTaskModule.kt b/app/src/main/java/eu/darken/sdmse/automation/core/debug/DebugTaskModule.kt
@@ -15,6 +15,7 @@ import eu.darken.sdmse.automation.core.AutomationHost
 import eu.darken.sdmse.automation.core.AutomationModule
 import eu.darken.sdmse.automation.core.AutomationTask
 import eu.darken.sdmse.automation.core.common.crawl
+import eu.darken.sdmse.automation.core.waitForWindowRoot
 import eu.darken.sdmse.common.ca.caString
 import eu.darken.sdmse.common.debug.logging.log
 import eu.darken.sdmse.common.debug.logging.logTag
@@ -60,7 +61,7 @@ class DebugTaskModule @AssistedInject constructor(
         val eventJob = host.events
             .onEach {
                 log(TAG) { "Event: $it" }
-                val crawled = host.windowRoot().crawl(debug = true).toList()
+                val crawled = host.waitForWindowRoot().crawl(debug = true).toList()
                 updateProgressSecondary("Event: ${it.eventType} (depth: ${crawled.last().level})")
             }
             .launchIn(moduleScope)

diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/errors/AutomationException.kt b/app/src/main/java/eu/darken/sdmse/automation/core/errors/AutomationException.kt
@@ -1,5 +1,8 @@
 package eu.darken.sdmse.automation.core.errors
 
-open class AutomationException(message: String?, cause: Throwable?) : Exception(message, cause) {
+open class AutomationException(
+    message: String? = null,
+    cause: Throwable? = null
+) : Exception(message, cause) {
     constructor(message: String?) : this(message, null)
 }
diff --git a/app/src/main/java/eu/darken/sdmse/automation/core/specs/AutomationExplorer.kt b/app/src/main/java/eu/darken/sdmse/automation/core/specs/AutomationExplorer.kt
@@ -1,6 +1,5 @@
 package eu.darken.sdmse.automation.core.specs
 
-import android.accessibilityservice.AccessibilityService
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
@@ -55,7 +54,7 @@ class AutomationExplorer @AssistedInject constructor(
             override val attempts: Int
                 get() = attempts
 
-            override val service: AccessibilityService = host.service
+            override val host: AutomationHost = this@AutomationExplorer.host
 
             override val stepper: StepProcessor = stepProcessorFactory.create(host)
         }
@@ -86,10 +85,10 @@ class AutomationExplorer @AssistedInject constructor(
 
         val attempts: Int
 
-        val service: AccessibilityService
+        val host: AutomationHost
 
         val androidContext: android.content.Context
-            get() = service
+            get() = host.service
 
         val stepper: StepProcessor
     }