diff --git a/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt b/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt index 92ae21f419..c3f187e2dd 100644 --- a/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt +++ b/app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt @@ -1307,7 +1307,7 @@ open class EditorHandlerActivity : // If there are NO unsaved files, just perform the close action directly. // The 'manualFinish' is false because this action doesn't exit the activity by itself. - performCloseAllFiles(manualFinish = false) + performCloseAllFiles() runAfter() } @@ -1907,7 +1907,7 @@ open class EditorHandlerActivity : confirmProjectClose() } - private fun performCloseAllFiles(manualFinish: Boolean) { + private fun performCloseAllFiles() { val pluginManager = IDEApplication.getPluginManager() val fileCount = editorViewModel.getOpenedFileCount() for (i in 0 until fileCount) { @@ -1929,8 +1929,12 @@ open class EditorHandlerActivity : tabs.removeAllTabs() editorContainer.removeAllViews() } + } - if (manualFinish) { + private fun closeProject(saveFloatingFiles: Boolean) { + performCloseAllFiles() + lifecycleScope.launch { + floatingTabController.closeAll(save = saveFloatingFiles) finish() } } @@ -1951,7 +1955,7 @@ open class EditorHandlerActivity : (content.editorContainer.getChildAt(i) as? CodeEditorView)?.editor?.markUnmodified() } - performCloseAllFiles(manualFinish = true) + closeProject(saveFloatingFiles = false) } // OPTION 2: Save and close @@ -1961,7 +1965,7 @@ open class EditorHandlerActivity : saveAllAsync(notify = false) { runOnUiThread { if (contentOrNull == null) return@runOnUiThread - performCloseAllFiles(manualFinish = true) + closeProject(saveFloatingFiles = true) } recentProjectsViewModel.updateProjectModifiedDate( editorViewModel.getProjectName(), diff --git a/app/src/main/java/com/itsaky/androidide/editor/floating/EditorPanelDockableContent.kt b/app/src/main/java/com/itsaky/androidide/editor/floating/EditorPanelDockableContent.kt index fa1765a30c..cee0b2ad74 100644 --- a/app/src/main/java/com/itsaky/androidide/editor/floating/EditorPanelDockableContent.kt +++ b/app/src/main/java/com/itsaky/androidide/editor/floating/EditorPanelDockableContent.kt @@ -19,11 +19,11 @@ import com.itsaky.androidide.models.Position import com.itsaky.androidide.models.Range import com.itsaky.androidide.projects.builder.BuildService import com.itsaky.androidide.ui.CodeEditorView -import java.io.File import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.withContext +import java.io.File import com.itsaky.androidide.resources.R as ResR /** @@ -118,6 +118,9 @@ class EditorPanelDockableContent( } } + val isModified: Boolean + get() = editorView?.isModified == true + suspend fun save(): Boolean = editorView?.save() ?: false fun release() { diff --git a/app/src/main/java/com/itsaky/androidide/editor/floating/IdeFloatingTabController.kt b/app/src/main/java/com/itsaky/androidide/editor/floating/IdeFloatingTabController.kt index 917b2139fb..69a6b95210 100644 --- a/app/src/main/java/com/itsaky/androidide/editor/floating/IdeFloatingTabController.kt +++ b/app/src/main/java/com/itsaky/androidide/editor/floating/IdeFloatingTabController.kt @@ -12,7 +12,9 @@ import com.itsaky.androidide.floating.permission.OverlayPermission import com.itsaky.androidide.floating.service.FloatingTabService import com.itsaky.androidide.floating.window.InitialBounds import com.itsaky.androidide.resources.R +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.launch +import org.slf4j.LoggerFactory /** * Bridges the editor activity to the floating-window system: turns an editor file tab into a @@ -77,9 +79,44 @@ class IdeFloatingTabController( FloatingTabService.ensureRunning(activity.applicationContext) } + /** + * Tears down every floating window because the project is closing: a docked plugin tab or file + * panel is closed with the project, and an undocked one is the same tab in another window. + * + * File panels are saved (when [save] is set, i.e. the user chose "save and close") and released + * inline, not through [onEvent]: that coroutine dies with the finishing activity, and it saves + * unconditionally, which would defeat "close without saving". Hence [DockingManager.remove] + * rather than [DockingManager.close] - the teardown is done, no listener should redo it. + */ + suspend fun closeAll(save: Boolean) { + for (tab in DockingManager.windows.value) { + val panel = tab.content as? EditorPanelDockableContent + if (save && panel != null && panel.isModified && !savePanel(panel)) { + Toast + .makeText( + activity, + activity.getString(R.string.msg_floating_close_save_failed, panel.title), + Toast.LENGTH_LONG, + ).show() + } + DockingManager.remove(tab.id) + panel?.release() + } + } + + private suspend fun savePanel(panel: EditorPanelDockableContent): Boolean = + try { + panel.save() + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + log.error("Failed to save floating panel '{}' while closing the project", panel.title, e) + false + } + private fun onEvent(event: DockingEvent) { when (val content = event.content) { - is EditorPanelDockableContent -> + is EditorPanelDockableContent -> { activity.lifecycleScope.launch { content.save() content.release() @@ -88,12 +125,14 @@ class IdeFloatingTabController( activity.openFile(content.file, null) } } + } - is PluginTabDockableContent -> + is PluginTabDockableContent -> { if (event is DockingEvent.Redock) { bringIdeToFront() activity.selectPluginTabById(content.tabId) } + } } } @@ -106,4 +145,8 @@ class IdeFloatingTabController( .addFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT), ) } + + private companion object { + private val log = LoggerFactory.getLogger(IdeFloatingTabController::class.java) + } } diff --git a/app/src/test/java/com/itsaky/androidide/editor/floating/FloatingWindowProjectCloseTest.kt b/app/src/test/java/com/itsaky/androidide/editor/floating/FloatingWindowProjectCloseTest.kt new file mode 100644 index 0000000000..65f16d11ef --- /dev/null +++ b/app/src/test/java/com/itsaky/androidide/editor/floating/FloatingWindowProjectCloseTest.kt @@ -0,0 +1,173 @@ +/* + * 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 . + */ + +package com.itsaky.androidide.editor.floating + +import com.google.common.truth.Truth.assertThat +import com.itsaky.androidide.activities.editor.EditorHandlerActivity +import com.itsaky.androidide.app.BaseApplication +import com.itsaky.androidide.floating.model.DockingEvent +import com.itsaky.androidide.floating.model.DockingManager +import com.itsaky.androidide.floating.window.WindowBounds +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.coVerifyOrder +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.Robolectric +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import java.io.File + +/** + * ADFA-4501: closing the project must leave no floating window behind. A docked plugin tab or file + * panel is closed with the project; an undocked one is the same tab living in another window, and + * used to keep running over other apps against a project that no longer exists. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(application = FloatingWindowProjectCloseTest.TestApp::class) +class FloatingWindowProjectCloseTest { + open class TestApp : BaseApplication() + + private lateinit var controller: IdeFloatingTabController + + @Before + fun setUp() { + val activity = Robolectric.buildActivity(EditorHandlerActivity::class.java).get() + controller = IdeFloatingTabController(activity) + } + + @After + fun tearDown() { + DockingManager.windows.value.forEach { DockingManager.close(it.id) } + } + + @Test + fun `closing the project tears down every floating window`() = + runTest { + DockingManager.undock(PluginTabDockableContent("keygen.tab", "Keystore Generator"), BOUNDS) + DockingManager.undock(EditorPanelDockableContent(File("/tmp/adfa4501/Main.kt")), BOUNDS) + assertThat(DockingManager.windows.value).hasSize(2) + + controller.closeAll(save = false) + + assertThat(DockingManager.windows.value).isEmpty() + } + + /** + * The teardown saves and releases file panels itself, according to the choice the user made in + * the close dialog. A [DockingEvent.Close] would hand the same panel to a listener that saves + * unconditionally, overriding "close without saving". + */ + @Test + fun `teardown emits no docking event`() = + runTest { + val events = mutableListOf() + backgroundScope.launch(UnconfinedTestDispatcher(testScheduler)) { + DockingManager.events.collect(events::add) + } + DockingManager.undock(EditorPanelDockableContent(File("/tmp/adfa4501/Notes.md")), BOUNDS) + + controller.closeAll(save = false) + + assertThat(events).isEmpty() + assertThat(DockingManager.windows.value).isEmpty() + } + + @Test + fun `tearing down with no floating windows is a no-op`() = + runTest { + controller.closeAll(save = true) + + assertThat(DockingManager.windows.value).isEmpty() + } + + @Test + fun `save and close writes a modified panel out before releasing it`() = + runTest { + val panel = floatingPanel("/tmp/adfa4501/Main.kt", modified = true) + coEvery { panel.save() } returns true + DockingManager.undock(panel, BOUNDS) + + controller.closeAll(save = true) + + coVerifyOrder { + panel.save() + panel.release() + } + assertThat(DockingManager.windows.value).isEmpty() + } + + /** "Close without saving" must reach the panel too, not just the docked editors. */ + @Test + fun `close without saving discards a modified panel`() = + runTest { + val panel = floatingPanel("/tmp/adfa4501/Draft.kt", modified = true) + DockingManager.undock(panel, BOUNDS) + + controller.closeAll(save = false) + + coVerify(exactly = 0) { panel.save() } + verify { panel.release() } + assertThat(DockingManager.windows.value).isEmpty() + } + + /** + * A file that went away under its panel (deleted directory, revoked permission) makes + * `writeTo` throw. The project is closing: that must not crash the teardown, nor strand the + * windows queued behind the bad one - which is the very state this ticket exists to prevent. + */ + @Test + fun `a panel whose save throws does not strand the windows behind it`() = + runTest { + val failing = floatingPanel("/tmp/adfa4501/Gone.kt", modified = true) + coEvery { failing.save() } throws RuntimeException("parent directory deleted") + val queued = floatingPanel("/tmp/adfa4501/Queued.kt", modified = false) + DockingManager.undock(failing, BOUNDS) + DockingManager.undock(queued, BOUNDS) + + controller.closeAll(save = true) + + assertThat(DockingManager.windows.value).isEmpty() + verify { failing.release() } + verify { queued.release() } + } + + private fun floatingPanel( + path: String, + modified: Boolean, + ): EditorPanelDockableContent = + mockk(relaxed = true).also { panel -> + every { panel.id } returns path + every { panel.title } returns File(path).name + every { panel.isModified } returns modified + } + + private companion object { + private val BOUNDS = WindowBounds(x = 0, y = 0, width = 600, height = 400) + } +} diff --git a/floating-window/src/main/java/com/itsaky/androidide/floating/model/DockingManager.kt b/floating-window/src/main/java/com/itsaky/androidide/floating/model/DockingManager.kt index 0099de19f2..c4699e05f6 100644 --- a/floating-window/src/main/java/com/itsaky/androidide/floating/model/DockingManager.kt +++ b/floating-window/src/main/java/com/itsaky/androidide/floating/model/DockingManager.kt @@ -46,18 +46,18 @@ object DockingManager { } /** Remove the floating window for [id] and signal that the tab should return to the dock. */ - fun dock(id: String): DockableContent? { - val tab = find(id) ?: return null - _windows.update { current -> current.filterNot { it.id == id } } - _events.tryEmit(DockingEvent.Redock(tab.content)) - return tab.content - } + fun dock(id: String): DockableContent? = remove(id)?.also { _events.tryEmit(DockingEvent.Redock(it)) } /** Remove the floating window for [id] and signal that the tab should be closed entirely. */ - fun close(id: String): DockableContent? { + fun close(id: String): DockableContent? = remove(id)?.also { _events.tryEmit(DockingEvent.Close(it)) } + + /** + * Remove the floating window for [id] without emitting a [DockingEvent]. For a caller that has + * already torn the tab down itself and must not have a listener act on it a second time. + */ + fun remove(id: String): DockableContent? { val tab = find(id) ?: return null _windows.update { current -> current.filterNot { it.id == id } } - _events.tryEmit(DockingEvent.Close(tab.content)) return tab.content } @@ -74,15 +74,17 @@ object DockingManager { ) { mutate(id) { state -> when (mode) { - WindowMode.MAXIMIZED, WindowMode.MINIMIZED -> + WindowMode.MAXIMIZED, WindowMode.MINIMIZED -> { if (state.mode == WindowMode.NORMAL) { state.copy(mode = mode, restoreBounds = state.bounds) } else { state.copy(mode = mode) } + } - WindowMode.NORMAL -> + WindowMode.NORMAL -> { state.copy(mode = mode, bounds = state.restoreBounds) + } } } } diff --git a/resources/src/main/res/values/strings.xml b/resources/src/main/res/values/strings.xml index dbc02a3bb3..581cffcb83 100644 --- a/resources/src/main/res/values/strings.xml +++ b/resources/src/main/res/values/strings.xml @@ -808,6 +808,7 @@ Could not save the font file. Please try again. A file save operation is already in progress! Couldn\'t save “%1$s”. Undock cancelled. + Couldn\'t save “%1$s”. Its unsaved changes were discarded with the project. Install Development Tools \nTo install the tools needed to build Android projects, tap the button at the bottom right of the screen\n