Merge branch 'stage' into fix/ADFA-4387

Author: itsaky-adfa

app/src/main/java/com/itsaky/androidide/fragments/onboarding/PermissionsFragment.kt

@@ -32,6 +32,7 @@ import androidx.core.net.toUri
 import androidx.fragment.app.viewModels
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.repeatOnLifecycle
+import androidx.lifecycle.withResumed
 import androidx.recyclerview.widget.RecyclerView
 import com.github.appintro.SlidePolicy
 import com.github.appintro.SlideSelectionListener
@@ -45,6 +46,7 @@ import com.itsaky.androidide.databinding.LayoutOnboardingPermissionsBinding
 import com.itsaky.androidide.events.InstallationEvent
 import com.itsaky.androidide.preferences.internal.prefManager
 import com.itsaky.androidide.tasks.doAsyncWithProgress
+import com.itsaky.androidide.utils.OverlayPermissionGuide
 import com.itsaky.androidide.utils.PermissionsHelper
 import com.itsaky.androidide.utils.flashError
 import com.itsaky.androidide.utils.isTestMode
@@ -73,7 +75,6 @@ class PermissionsFragment :
 	private var recyclerView: RecyclerView? = null
 	private var finishButton: MaterialButton? = null
     private lateinit var pulseAnimation: Animation
-	private var awaitingOverlayGrantResult = false
 
 	private val storagePermissionRequestLauncher =
 		registerForActivityResult(
@@ -97,6 +98,8 @@ class PermissionsFragment :
 		private val logger = LoggerFactory.getLogger(PermissionsFragment::class.java)
 		private const val KEY_PRIVACY_DISCLOSURE_SHOWN = "privacy.disclosure.shown"
 
+		private var awaitingOverlayGrantResult = false
+
 		@JvmStatic
 		fun newInstance(context: Context): PermissionsFragment =
 			PermissionsFragment().apply {
@@ -231,12 +234,13 @@ class PermissionsFragment :
        }
        awaitingOverlayGrantResult = false
 
-       if (PermissionsHelper.canDrawOverlays(requireContext())) {
-          return
+       viewLifecycleScope.launch {
+          viewLifecycleOwner.withResumed {
+             if (!PermissionsHelper.canDrawOverlays(requireContext())) {
+                OverlayPermissionGuide.showRestrictedSettingsDialog(requireContext())
+             }
+          }
        }
-
-       flashError(getString(R.string.permission_overlay_restricted_settings_hint))
-       requestSettingsTogglePermission(Settings.ACTION_APPLICATION_DETAILS_SETTINGS)
     }
 
 	private fun startIdeSetup() {

app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt

@@ -90,12 +90,20 @@ class BuildOutputFragment : NonEditableEditorFragment() {
 		super.onDestroyView()
 	}
 
+	/** Clears the build output, guarding against access while the fragment is detached. */
 	override fun clearOutput() {
+		// Avoid forcing the activityViewModels lazy init (which calls requireActivity())
+		// when the fragment is detached, otherwise an IllegalStateException is thrown.
+		if (!isAdded || activity == null) return
 		buildOutputViewModel.clear()
 		super.clearOutput()
 	}
 
+	/** Returns the shareable build output, or an empty string when the fragment is detached. */
 	override fun getShareableContent(): String {
+		// Same guard as clearOutput(): touching buildOutputViewModel while detached
+		// triggers requireActivity() via activityViewModels and crashes.
+		if (!isAdded || activity == null) return ""
 		val snapshot = buildOutputViewModel.getCachedContentSnapshot()
 		return if (snapshot.isEmpty()) "" else BuildInfoUtils.BASIC_INFO + System.lineSeparator() + snapshot
 	}

app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt

@@ -452,7 +452,7 @@ constructor(
 		suppressedGradleWarnings.any { msg.contains(it) }
 
 	fun clearBuildOutput() {
-		pagerAdapter.buildOutputFragment?.clearOutput()
+		pagerAdapter.buildOutputFragment?.takeIf { it.isAdded }?.clearOutput()
 	}
 
 	fun handleDiagnosticsResultVisibility(errorVisible: Boolean) {

app/src/main/java/com/itsaky/androidide/utils/OverlayPermissionGuide.kt

@@ -0,0 +1,55 @@
+package com.itsaky.androidide.utils
+
+import android.app.Activity
+import android.content.ActivityNotFoundException
+import android.content.Context
+import android.content.Intent
+import android.net.Uri
+import android.provider.Settings
+import android.text.SpannableStringBuilder
+import com.itsaky.androidide.resources.R
+import org.slf4j.LoggerFactory
+
+object OverlayPermissionGuide {
+
+	private val log = LoggerFactory.getLogger(OverlayPermissionGuide::class.java)
+
+	fun showRestrictedSettingsDialog(context: Context) {
+		val message = SpannableStringBuilder()
+		message.append(context.getString(R.string.permission_overlay_restricted_dialog_message))
+		message.append("\n\n")
+		message.appendOrderedList(*context.resources.getStringArray(R.array.overlay_restricted_settings_steps))
+
+		DialogUtils
+			.newMaterialDialogBuilder(context)
+			.setTitle(R.string.permission_overlay_restricted_dialog_title)
+			.setMessage(message)
+			.setPositiveButton(R.string.permission_overlay_open_app_info) { dialog, _ ->
+				dialog.dismiss()
+				openAppInfo(context)
+			}
+			.setNegativeButton(android.R.string.cancel, null)
+			.show()
+	}
+
+	private fun openAppInfo(context: Context) {
+		val intent = Intent(
+			Settings.ACTION_APPLICATION_DETAILS_SETTINGS,
+			Uri.fromParts("package", context.packageName, null),
+		)
+		if (context !is Activity) {
+			intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
+		}
+		try {
+			context.startActivity(intent)
+		} catch (e: ActivityNotFoundException) {
+			log.error("Failed to open App info settings", e)
+			flashError(
+				context.getString(
+					R.string.err_no_activity_to_handle_action,
+					Settings.ACTION_APPLICATION_DETAILS_SETTINGS,
+				),
+			)
+		}
+	}
+}

app/src/test/java/com/itsaky/androidide/fragments/output/BuildOutputFragmentDetachedTest.kt

@@ -0,0 +1,72 @@
+/*
+ *  This file is part of AndroidIDE.
+ *
+ *  AndroidIDE is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  AndroidIDE is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *   along with AndroidIDE.  If not, see <https://www.gnu.org/licenses/>.
+ */
+
+package com.itsaky.androidide.fragments.output
+
+import com.google.common.truth.Truth.assertThat
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * Regression test for ADFA-3472.
+ *
+ * [BuildOutputFragment.clearOutput] and [BuildOutputFragment.getShareableContent] touch
+ * `buildOutputViewModel`, which is created via `by activityViewModels()`. Forcing that lazy
+ * delegate while the fragment is detached calls `requireActivity()`, which throws an
+ * [IllegalStateException] ("not attached to an activity"). The run-tasks dialog / config-change
+ * path can invoke these methods on a detached fragment, crashing the app (Sentry ADFA-3472).
+ *
+ * The fix guards both methods with `if (!isAdded || activity == null) return`. These tests
+ * assert that a detached fragment does NOT crash and returns the safe no-op values.
+ *
+ * Mutation-mindset: on the pre-fix code (no guard), both calls force the activityViewModels
+ * delegate -> requireActivity() -> IllegalStateException, so each test goes RED.
+ */
+@RunWith(RobolectricTestRunner::class)
+class BuildOutputFragmentDetachedTest {
+
+  /** Verifies clearOutput() is a safe no-op on a detached fragment instead of crashing. */
+  @Test
+  fun `clearOutput on a detached fragment does not crash`() {
+    // A freshly-constructed fragment that was never added to an activity is "detached":
+    // isAdded == false and activity == null, exactly the run-tasks / config-change state
+    // in which the Sentry crash was observed.
+    val fragment = BuildOutputFragment()
+
+    assertThat(fragment.isAdded).isFalse()
+
+    // Pre-fix: this forces the `by activityViewModels()` delegate, which calls
+    // requireActivity() on a detached fragment and throws IllegalStateException.
+    // Post-fix: the guard returns early, no exception.
+    fragment.clearOutput()
+  }
+
+  /** Verifies getShareableContent() returns an empty string on a detached fragment instead of crashing. */
+  @Test
+  fun `getShareableContent on a detached fragment returns empty without crashing`() {
+    val fragment = BuildOutputFragment()
+
+    assertThat(fragment.isAdded).isFalse()
+
+    // Pre-fix: forces the activityViewModels delegate -> requireActivity() -> ISE.
+    // Post-fix: guard returns "" without touching the view model.
+    val content = fragment.getShareableContent()
+
+    assertThat(content).isEmpty()
+  }
+}

common/build.gradle.kts

@@ -43,6 +43,8 @@ dependencies {
 	implementation(libs.monitor)
 
 	testImplementation(projects.testing.common)
+	testImplementation(libs.tests.kotlinx.coroutines)
+	testImplementation(libs.tests.google.truth)
 	androidTestImplementation(projects.testing.android)
 
 	// brotli4j

common/src/main/java/com/itsaky/androidide/utils/KeyedDebouncingAction.kt

@@ -2,11 +2,13 @@ package com.itsaky.androidide.utils
 
 import com.itsaky.androidide.progress.ICancelChecker
 import com.itsaky.androidide.tasks.JobCancelChecker
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.channels.Channel
+import kotlinx.coroutines.channels.ClosedReceiveChannelException
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.ensureActive
 import kotlinx.coroutines.isActive
@@ -29,9 +31,13 @@ class KeyedDebouncingAction<T: Any>(
 		val channel: Channel<T>,
 		val job: Job,
 	) {
+		/** Cancels this entry's worker job and closes its channel, in that order. */
 		fun cancel() {
-			channel.close()
+			// Cancel the job FIRST, then close the channel. Closing the channel first
+			// wakes a parked receive() with a ClosedReceiveChannelException before the
+			// job is cancelled, which can crash a worker that has no exception handling.
 			job.cancel()
+			channel.close()
 		}
 	}
 
@@ -50,38 +56,50 @@ class KeyedDebouncingAction<T: Any>(
 		entry.channel.trySend(key)
 	}
 
+	/**
+	 * Creates a new [ActionEntry]: a CONFLATED channel plus a worker coroutine that debounces
+	 * incoming keys and runs [action] for the latest one, stopping cleanly when the channel is closed.
+	 */
 	@OptIn(ExperimentalCoroutinesApi::class)
 	private fun createEntry(): ActionEntry<T> {
 		val channel = Channel<T>(Channel.CONFLATED)
 		val job = scope.launch(actionContext) {
 			while (isActive) {
-				var latestKey = channel.receive()
-				var debouncing = true
-				while (debouncing) {
-					debouncing = select {
-						onTimeout(debounceDuration) { false }
-						channel.onReceive { newKey ->
-							latestKey = newKey
-							true
+				try {
+					var latestKey = channel.receive()
+					var debouncing = true
+					while (debouncing) {
+						debouncing = select {
+							onTimeout(debounceDuration) { false }
+							channel.onReceive { newKey ->
+								latestKey = newKey
+								true
+							}
 						}
 					}
-				}
 
-				ensureActive()
-				val actionJob = launch {
-					val cancelChecker = JobCancelChecker(currentCoroutineContext()[Job])
-					action(latestKey, cancelChecker)
-				}
+					ensureActive()
+					val actionJob = launch {
+						val cancelChecker = JobCancelChecker(currentCoroutineContext()[Job])
+						action(latestKey, cancelChecker)
+					}
 
-				select<Unit> {
-					actionJob.onJoin {}
-					channel.onReceive { newerKey ->
-						actionJob.cancel()
-						channel.trySend(newerKey)
+					select<Unit> {
+						actionJob.onJoin {}
+						channel.onReceive { newerKey ->
+							actionJob.cancel()
+							channel.trySend(newerKey)
+						}
 					}
-				}
 
-				actionJob.join()
+					actionJob.join()
+				} catch (e: ClosedReceiveChannelException) {
+					// The channel was closed (entry cancelled). Stop the worker cleanly
+					// instead of letting the exception propagate to an uncaught handler.
+					break
+				} catch (e: CancellationException) {
+					throw e
+				}
 			}
 		}
 

common/src/test/java/com/itsaky/androidide/utils/KeyedDebouncingActionCancelTest.kt

@@ -0,0 +1,66 @@
+package com.itsaky.androidide.utils
+
+import com.google.common.truth.Truth.assertThat
+import kotlinx.coroutines.CoroutineExceptionHandler
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.channels.ClosedReceiveChannelException
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.runBlocking
+import org.junit.Test
+import java.util.concurrent.atomic.AtomicReference
+import kotlin.coroutines.CoroutineContext
+import kotlin.time.Duration.Companion.milliseconds
+
+/**
+ * Repro for ADFA-4328: cancelling a [KeyedDebouncingAction] entry whose worker is
+ * parked on `channel.receive()` must NOT let a [ClosedReceiveChannelException]
+ * escape to the scope's uncaught-exception handler.
+ *
+ * On the pre-fix baseline, `ActionEntry.cancel()` did `channel.close()` BEFORE
+ * `job.cancel()`. Closing the channel wakes the parked `receive()` with a
+ * [ClosedReceiveChannelException] (NOT a CancellationException), which propagates
+ * uncaught to the [CoroutineExceptionHandler] -> the Sentry crash this ticket fixes.
+ *
+ * The fix swaps the order (job.cancel() first) AND wraps the worker loop in a
+ * try/catch that swallows ClosedReceiveChannelException, so no uncaught exception fires.
+ */
+class KeyedDebouncingActionCancelTest {
+
+  /** Cancelling an entry whose worker is parked on receive() must not surface an uncaught exception. */
+  @Test
+  fun `cancelling a parked worker does not leak a ClosedReceiveChannelException`() = runBlocking {
+    val uncaught = AtomicReference<Throwable?>(null)
+    // A plain Job (not Supervisor of the worker) + a handler that records anything
+    // that escapes the debounce worker coroutine.
+    val handler = CoroutineExceptionHandler { _, t -> uncaught.set(t) }
+    val scope = CoroutineScope(SupervisorJob() + handler)
+
+    val ctx: CoroutineContext = scope.coroutineContext
+
+    val debouncer = KeyedDebouncingAction<String>(
+      scope = scope,
+      debounceDuration = 50.milliseconds,
+      actionContext = ctx,
+      action = { _, _ -> /* never invoked: we cancel while parked on receive */ },
+    )
+
+    // schedule() creates the entry + launches the worker. With a CONFLATED channel and
+    // no further sends, the worker debounces the single key, runs the (empty) action,
+    // then loops back and parks on channel.receive() waiting for the next key.
+    debouncer.schedule("k")
+
+    // Give the worker time to: receive "k", run the empty action, loop, and PARK on
+    // the next channel.receive(). 200ms >> 50ms debounce window.
+    delay(200)
+
+    // Cancel the entry while the worker is parked on receive().
+    debouncer.cancelPending("k")
+
+    // Let any uncaught exception propagate to the handler.
+    delay(200)
+
+    val leaked = uncaught.get()
+    assertThat(leaked).isNull()
+  }
+}