diff --git a/app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt b/app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt index 5b6d8779ff..c2a99585d7 100644 --- a/app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt +++ b/app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt @@ -30,85 +30,95 @@ import com.itsaky.androidide.idetooltips.TooltipCategory import com.itsaky.androidide.idetooltips.TooltipManager import com.itsaky.androidide.idetooltips.TooltipTag -class ShowTooltipAction(private val context: Context, override val order: Int) : - BaseEditorAction() { - - companion object { - const val ID = "ide.editor.code.text.show_tooltip" - } - - override val id: String = ID - override var location: ActionItem.Location = ActionItem.Location.EDITOR_TEXT_ACTIONS - - init { - label = context.getString(R.string.title_show_tooltip) - val drawable = ContextCompat.getDrawable(context, R.drawable.ic_action_help_outlined) - icon = drawable?.let { tintDrawable(context, it) } - } - - override fun prepare(data: ActionData) { - super.prepare(data) - if (!visible) return - - val target = getTextTarget(data) - visible = target != null - enabled = visible - } - - override suspend fun execAction(data: ActionData): Boolean { - val target = getTextTarget(data) ?: return false - val anchorView = target.getAnchorView() ?: return false - val editor = getEditor(data) - - val categoryAndTag = - if (editor != null) { - val category = tooltipCategoryForExtension(editor.file?.extension) - resolveTooltipTag( - category = category, - selectedText = target.getSelectedText(), - editorTag = editor.tag?.toString(), - isXmlAttribute = category == TooltipCategory.CATEGORY_XML && editor.isXmlAttribute(), - ).let { tag -> category to tag } - } else { - TooltipCategory.CATEGORY_IDE to TooltipTag.DIALOG_FIND_IN_PROJECT - } - val (category, tag) = categoryAndTag - - if (tag.isEmpty()) return false - - TooltipManager.showTooltip( - context = anchorView.context, - anchorView = anchorView, - category = category, - tag = tag, - ) - - return true - } - - override fun retrieveTooltipTag(isAlternateContext: Boolean) = TooltipTag.EDITOR_TOOLBAR_HELP +/** + * Editor text action that shows the tooltip for the current selection. + * + * EDITOR_TEXT_ACTIONS is never cleared (see EditorActivityActions.clear), so the static + * ActionsRegistry outlives every editor activity and an action holding a Context leaks it. + * [execAction] uses the anchor view's context instead. + * + * @param context initializes [label] and [icon] only; deliberately not retained. + */ +class ShowTooltipAction( + context: Context, + override val order: Int, +) : BaseEditorAction() { + companion object { + const val ID = "ide.editor.code.text.show_tooltip" + } + + override val id: String = ID + override var location: ActionItem.Location = ActionItem.Location.EDITOR_TEXT_ACTIONS + + init { + label = context.getString(R.string.title_show_tooltip) + val drawable = ContextCompat.getDrawable(context, R.drawable.ic_action_help_outlined) + icon = drawable?.let { tintDrawable(context, it) } + } + + override fun prepare(data: ActionData) { + super.prepare(data) + if (!visible) return + + val target = getTextTarget(data) + visible = target != null + enabled = visible + } + + override suspend fun execAction(data: ActionData): Boolean { + val target = getTextTarget(data) ?: return false + val anchorView = target.getAnchorView() ?: return false + val editor = getEditor(data) + + val categoryAndTag = + if (editor != null) { + val category = tooltipCategoryForExtension(editor.file?.extension) + resolveTooltipTag( + category = category, + selectedText = target.getSelectedText(), + editorTag = editor.tag?.toString(), + isXmlAttribute = category == TooltipCategory.CATEGORY_XML && editor.isXmlAttribute(), + ).let { tag -> category to tag } + } else { + TooltipCategory.CATEGORY_IDE to TooltipTag.DIALOG_FIND_IN_PROJECT + } + val (category, tag) = categoryAndTag + + if (tag.isEmpty()) return false + + TooltipManager.showTooltip( + context = anchorView.context, + anchorView = anchorView, + category = category, + tag = tag, + ) + + return true + } + + override fun retrieveTooltipTag(isAlternateContext: Boolean) = TooltipTag.EDITOR_TOOLBAR_HELP } internal fun tooltipCategoryForExtension(extension: String?): String = - when (extension) { - "java" -> TooltipCategory.CATEGORY_JAVA - "kt" -> TooltipCategory.CATEGORY_KOTLIN - "xml" -> TooltipCategory.CATEGORY_XML - else -> TooltipCategory.CATEGORY_IDE - } + when (extension) { + "java" -> TooltipCategory.CATEGORY_JAVA + "kt" -> TooltipCategory.CATEGORY_KOTLIN + "xml" -> TooltipCategory.CATEGORY_XML + else -> TooltipCategory.CATEGORY_IDE + } internal fun resolveTooltipTag( - category: String, - selectedText: String?, - editorTag: String?, - isXmlAttribute: Boolean, + category: String, + selectedText: String?, + editorTag: String?, + isXmlAttribute: Boolean, ): String { - val textToUse = selectedText ?: "" - return when { - !editorTag.isNullOrEmpty() -> editorTag - category == TooltipCategory.CATEGORY_XML && isXmlAttribute -> textToUse.substringAfterLast(":") - category == TooltipCategory.CATEGORY_KOTLIN && isKotlinOperatorToken(textToUse) -> "kotlin.operator.$textToUse" - category == TooltipCategory.CATEGORY_JAVA && isJavaOperatorToken(textToUse) -> "java.operator.$textToUse" - else -> textToUse - } -} \ No newline at end of file + val textToUse = selectedText ?: "" + return when { + !editorTag.isNullOrEmpty() -> editorTag + category == TooltipCategory.CATEGORY_XML && isXmlAttribute -> textToUse.substringAfterLast(":") + category == TooltipCategory.CATEGORY_KOTLIN && isKotlinOperatorToken(textToUse) -> "kotlin.operator.$textToUse" + category == TooltipCategory.CATEGORY_JAVA && isJavaOperatorToken(textToUse) -> "java.operator.$textToUse" + else -> textToUse + } +} diff --git a/app/src/main/java/com/itsaky/androidide/viewmodel/DebuggerViewModel.kt b/app/src/main/java/com/itsaky/androidide/viewmodel/DebuggerViewModel.kt index 731cc9a9bf..65962c38e1 100644 --- a/app/src/main/java/com/itsaky/androidide/viewmodel/DebuggerViewModel.kt +++ b/app/src/main/java/com/itsaky/androidide/viewmodel/DebuggerViewModel.kt @@ -113,77 +113,90 @@ class DebuggerViewModel : ViewModel() { _debugeePackage.update { value } } - val allThreads = state.map { - logger.debug("Updating all threads") - it.threads - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = emptyList(), - ) - - val selectedThread = state - .map { state -> - state.selectedThread to state.threadIndex - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = null to -1, - ) + val allThreads = + state + .map { + logger.debug("Updating all threads") + it.threads + }.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = emptyList(), + ) - val allFrames = selectedThread - .map { (thread, _) -> - thread?.getFrames() ?: emptyList() - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = emptyList(), - ) + val selectedThread = + state + .map { state -> + state.selectedThread to state.threadIndex + }.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = null to -1, + ) - val selectedFrame = state - .map { state -> - state.selectedFrame() to state.frameIndex - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = null to -1, - ) + val allFrames = + selectedThread + .map { (thread, _) -> + thread?.getFrames() ?: emptyList() + }.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = emptyList(), + ) - val selectedFrameVariables = selectedFrame - .map { (frame, _) -> - frame?.getVariables() ?: emptyList() - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = emptyList(), - ) + val selectedFrame = + state + .map { state -> + state.selectedFrame() to state.frameIndex + }.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = null to -1, + ) - val variablesTree = state - .map { state -> - state.variablesTree - }.stateIn( - scope = viewModelScope, - started = SharingStarted.Eagerly, - initialValue = DebuggerState.DEFAULT.variablesTree, - ) + val selectedFrameVariables = + selectedFrame + .map { (frame, _) -> + frame?.getVariables() ?: emptyList() + }.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = emptyList(), + ) + + val variablesTree = + state + .map { state -> + state.variablesTree + }.stateIn( + scope = viewModelScope, + started = SharingStarted.Eagerly, + initialValue = DebuggerState.DEFAULT.variablesTree, + ) override fun onCleared() { super.onCleared() Lookup.getDefault().unregister(IDEDebugClientImpl::class.java) + // The client registered itself with EventBus in its init block; without this the default + // EventBus keeps it, and through it this view model, for the life of the process. + debugClient.unregister() } fun setConnectionState(state: DebuggerConnectionState) { _connectionState.update { state } } - private fun setDebuggerState(newState: DebuggerState, because: String) { + private fun setDebuggerState( + newState: DebuggerState, + because: String, + ) { logger.debug("Updating debugger state because {}: {}", because, newState) state.update { newState } } private suspend inline fun setDebuggerState( because: String, - crossinline newState: suspend (DebuggerState) -> DebuggerState + crossinline newState: suspend (DebuggerState) -> DebuggerState, ) { val currentState = state.value val newState = newState(currentState) @@ -229,13 +242,13 @@ class DebuggerViewModel : ViewModel() { suspend fun setThreads( threads: List, selectedThreadIndex: Int = -1, - selectedFrameIndex: Int = -1 + selectedFrameIndex: Int = -1, ) { logger.debug( "setThreads(selectedThreadIndex={}, selectedFrameIndex={}, threads={})", selectedThreadIndex, selectedFrameIndex, - threads + threads, ) withContext(Dispatchers.IO) { @@ -270,11 +283,16 @@ class DebuggerViewModel : ViewModel() { var frameIndex = selectedFrameIndex if (frameIndex < 0) { - frameIndex = if (resolvableThreads - .getOrNull(threadIndex) - ?.getFrames() - ?.firstOrNull() != null - ) 0 else -1 + frameIndex = + if (resolvableThreads + .getOrNull(threadIndex) + ?.getFrames() + ?.firstOrNull() != null + ) { + 0 + } else { + -1 + } } DebuggerState( @@ -343,38 +361,39 @@ class DebuggerViewModel : ViewModel() { } } - suspend fun setSelectedThreadIndex(index: Int) = withContext(Dispatchers.IO) { - setDebuggerState(because = "selected thread index changed") { current -> - check(index in 0.. + check(index in 0.. - check(index in 0..<(current.selectedThread?.getFrames()?.size ?: 0)) { - "Invalid frame index: $index" - } + suspend fun setSelectedFrameIndex(index: Int) = + withContext(Dispatchers.IO) { + setDebuggerState(because = "selected frame index changed") { current -> + check(index in 0..<(current.selectedThread?.getFrames()?.size ?: 0)) { + "Invalid frame index: $index" + } - current.copy( - frameIndex = index, - variablesTree = - createVariablesTree( - current.threads, - current.threadIndex, - index, - ), - ) + current.copy( + frameIndex = index, + variablesTree = + createVariablesTree( + current.threads, + current.threadIndex, + index, + ), + ) + } } - } @OptIn(ExperimentalStdlibApi::class) fun observeLatestSelectedFrame( diff --git a/app/src/main/java/com/itsaky/androidide/viewmodel/WADBConnectionViewModel.kt b/app/src/main/java/com/itsaky/androidide/viewmodel/WADBConnectionViewModel.kt index 398b413df0..095d74c71e 100644 --- a/app/src/main/java/com/itsaky/androidide/viewmodel/WADBConnectionViewModel.kt +++ b/app/src/main/java/com/itsaky/androidide/viewmodel/WADBConnectionViewModel.kt @@ -191,6 +191,7 @@ class WADBConnectionViewModel : ViewModel() { AdbPairingService.ACTION_PAIR_STARTED -> { _status.update { ConnectionStatus.Pairing } } + AdbPairingService.ACTION_PAIR_SUCCEEDED -> { _status.update { ConnectionStatus.Paired } diff --git a/app/src/test/java/com/itsaky/androidide/actions/EditorTextActionContextTest.kt b/app/src/test/java/com/itsaky/androidide/actions/EditorTextActionContextTest.kt new file mode 100644 index 0000000000..2c54417ded --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/actions/EditorTextActionContextTest.kt @@ -0,0 +1,89 @@ +package com.itsaky.androidide.actions + +import android.content.Context +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.actions.editor.CopyAction +import com.itsaky.androidide.actions.editor.CutAction +import com.itsaky.androidide.actions.editor.ExpandSelectionAction +import com.itsaky.androidide.actions.editor.LongSelectAction +import com.itsaky.androidide.actions.editor.PasteAction +import com.itsaky.androidide.actions.editor.SelectAllAction +import com.itsaky.androidide.actions.file.FormatCodeAction +import com.itsaky.androidide.actions.file.ShowTooltipAction +import com.itsaky.androidide.actions.locations.CodeActionsMenu +import org.junit.Test +import java.lang.reflect.Field + +/** + * `ActionsRegistry` is a process-lifetime singleton and EDITOR_TEXT_ACTIONS is deliberately never + * cleared, so language-server actions survive - see `EditorActivityActions.clear`. An action in that + * bucket that stores its `Context` therefore keeps one editor activity alive for the whole process, + * which is what `ShowTooltipAction` did. + * + * Reflection rather than a heap assertion: it names the mistake directly and costs nothing. + * + * What it does not cover, so nobody reads more into a green run than is there: + * - the list is hand-maintained, so a newly registered action is uncovered until it is added here; + * - the LSP actions that `LSPEditorActions` nests under [CodeActionsMenu.children] at runtime; + * - a `Context` reached indirectly - held by a companion object, or captured by a lambda stored in + * a field - since those live on synthetic classes, not on a field of the action typed `Context`. + */ +class EditorTextActionContextTest { + private val editorTextActions = + listOf( + ExpandSelectionAction::class.java, + SelectAllAction::class.java, + LongSelectAction::class.java, + CutAction::class.java, + CopyAction::class.java, + PasteAction::class.java, + FormatCodeAction::class.java, + ShowTooltipAction::class.java, + // A process-lifetime `object`, so the most dangerous entry in the bucket rather than one + // to leave out. + CodeActionsMenu::class.java, + ) + + @Test + fun givenTheEditorTextActions_whenInspected_thenNoneDeclaresAContextField() { + val offenders = + editorTextActions + .flatMap { action -> + action + .fieldsIncludingInherited() + .filter { Context::class.java.isAssignableFrom(it.type) } + .map { "${action.simpleName} (from ${it.declaringClass.simpleName}).${it.name}" } + } + + assertThat(offenders).isEmpty() + } + + @Test + fun givenAContextFieldOnASuperclass_whenInspected_thenItIsStillFound() { + // Pins the reason this walks the hierarchy: with declaredFields alone this finds nothing, so + // an action inheriting a Context would have passed the test above. + val found = + ActionInheritingAContext::class.java + .fieldsIncludingInherited() + .filter { Context::class.java.isAssignableFrom(it.type) } + + assertThat(found).isNotEmpty() + } + + private open class BaseHoldingAContext { + @Suppress("unused") + private val context: Context? = null + } + + private class ActionInheritingAContext : BaseHoldingAContext() + + /** + * Declared fields of this class and of every superclass. [Class.getDeclaredFields] stops at the + * class itself, so a Context held by a shared base class - the more likely place for one to hide + * than in each action - would pass unnoticed. + */ + private fun Class<*>.fieldsIncludingInherited(): List = + generateSequence(this) { it.superclass } + .flatMap { it.declaredFields.asSequence() } + .toList() +} diff --git a/subprojects/shizuku-manager/src/main/java/moe/shizuku/manager/adb/AdbMdns.kt b/subprojects/shizuku-manager/src/main/java/moe/shizuku/manager/adb/AdbMdns.kt index 9aac9a38a1..e4719d3464 100644 --- a/subprojects/shizuku-manager/src/main/java/moe/shizuku/manager/adb/AdbMdns.kt +++ b/subprojects/shizuku-manager/src/main/java/moe/shizuku/manager/adb/AdbMdns.kt @@ -18,9 +18,22 @@ class AdbMdns( private val serviceType: String, private val observer: Consumer, ) { + /** + * Takes an NsdManager that neither retains [context] nor is shared with another [AdbMdns]. + * + * getSystemService caches NsdManager per Context, and the manager holds that Context for its + * own lifetime while the framework keeps the manager alive, so one taken from an Activity or a + * Service retains it for the whole process; stopServiceDiscovery() does not release it. A + * fresh attribution context off the application context avoids that, and since the cache is + * per-ContextImpl it also keeps each instance a separate NsdService client - which matters + * below Android 13, where a second concurrent resolveService fails with FAILURE_ALREADY_ACTIVE. + */ constructor(context: Context, serviceType: String, observer: Consumer) : this( - nsdManager = context.getSystemService(NsdManager::class.java), + nsdManager = + context.applicationContext + .createAttributionContext(null) + .getSystemService(NsdManager::class.java), serviceType = serviceType, observer = observer, ) @@ -141,8 +154,12 @@ class AdbMdns( ) : NsdManager.ResolveListener { override fun onResolveFailed( nsdServiceInfo: NsdServiceInfo, - i: Int, - ) {} + errorCode: Int, + ) { + // An empty body here hid every resolve failure: no port is delivered and the caller + // just times out. FAILURE_ALREADY_ACTIVE (3) means two resolves raced on one manager. + Log.w(TAG, "onResolveFailed: ${nsdServiceInfo.serviceName}, $errorCode") + } override fun onServiceResolved(nsdServiceInfo: NsdServiceInfo) { adbMdns.onServiceResolved(nsdServiceInfo)