Fix WindowState and DialogState savers to not crash on unspecified wi…

…ndow/dialog size. (#1394)
JetBrains · Jun 10, 2024 · 2d2b9e1 · 2d2b9e1
1 parent 7a13310
commit 2d2b9e1
Show file tree

Hide file tree

Showing 4 changed files with 95 additions and 20 deletions.
diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/DialogState.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/DialogState.desktop.kt
@@ -27,6 +27,8 @@ import androidx.compose.ui.Alignment
 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.DpSize
 import androidx.compose.ui.unit.dp
+import androidx.compose.ui.unit.isSpecified
+import androidx.compose.ui.unit.takeOrElse
 
 /**
  * Creates a [DialogState] that is remembered across compositions.
@@ -173,8 +175,9 @@ private class DialogStateImpl(
                     it.position.isSpecified,
                     it.position.x.value,
                     it.position.y.value,
-                    it.size.width.value,
-                    it.size.height.value,
+                    it.size.takeOrElse { DpSize.Zero }.width.value,
+                    it.size.takeOrElse { DpSize.Zero }.height.value,
+                    it.size.isSpecified,
                 )
             },
             restore = { state ->
@@ -184,7 +187,11 @@ private class DialogStateImpl(
                     } else {
                         unspecifiedPosition
                     },
-                    size = DpSize((state[3] as Float).dp, (state[4] as Float).dp),
+                    size = if (state.getOrNull(5) != false) {
+                        DpSize((state[3] as Float).dp, (state[4] as Float).dp)
+                    } else {
+                        DpSize.Unspecified
+                    },
                 )
             }
         )

diff --git a/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/WindowState.desktop.kt b/compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/window/WindowState.desktop.kt
@@ -26,6 +26,8 @@ import androidx.compose.runtime.setValue
 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.DpSize
 import androidx.compose.ui.unit.dp
+import androidx.compose.ui.unit.isSpecified
+import androidx.compose.ui.unit.takeOrElse
 
 /**
  * Creates a [WindowState] that is remembered across compositions.
@@ -227,20 +229,25 @@ private class WindowStateImpl(
                     it.position.isSpecified,
                     it.position.x.value,
                     it.position.y.value,
-                    it.size.width.value,
-                    it.size.height.value,
+                    it.size.takeOrElse { DpSize.Zero }.width.value,
+                    it.size.takeOrElse { DpSize.Zero }.height.value,
+                    it.size.isSpecified,
                 )
             },
             restore = { state ->
                 WindowStateImpl(
-                    placement = WindowPlacement.values()[state[0] as Int],
+                    placement = WindowPlacement.entries[state[0] as Int],
                     isMinimized = state[1] as Boolean,
                     position = if (state[2] as Boolean) {
                         WindowPosition((state[3] as Float).dp, (state[4] as Float).dp)
                     } else {
                         unspecifiedPosition
                     },
-                    size = DpSize((state[5] as Float).dp, (state[6] as Float).dp),
+                    size = if (state.getOrNull(7) != false) {
+                        DpSize((state[5] as Float).dp, (state[6] as Float).dp)
+                    } else {
+                        DpSize.Unspecified
+                    },
                 )
             }
         )

diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/DialogWindowTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/DialogWindowTest.kt
@@ -26,8 +26,10 @@ import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.compositionLocalOf
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
+import androidx.compose.runtime.saveable.rememberSaveableStateHolder
 import androidx.compose.runtime.setValue
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.awt.ComposeDialog
@@ -41,6 +43,8 @@ import androidx.compose.ui.input.key.onKeyEvent
 import androidx.compose.ui.input.key.onPreviewKeyEvent
 import androidx.compose.ui.platform.LocalLayoutDirection
 import androidx.compose.ui.sendKeyEvent
+import androidx.compose.ui.test.ExperimentalTestApi
+import androidx.compose.ui.test.runComposeUiTest
 import androidx.compose.ui.text.drawText
 import androidx.compose.ui.text.rememberTextMeasurer
 import androidx.compose.ui.toInt
@@ -666,4 +670,32 @@ class DialogWindowTest {
         dialog?.dispatchEvent(WindowEvent(dialog, WindowEvent.WINDOW_CLOSING))
     }
 
+    @OptIn(ExperimentalTestApi::class)
+    @Test
+    fun `can save Unspecified dialog size`() = runComposeUiTest {
+        val expectedState = DialogState(size = DpSize.Unspecified)
+        lateinit var restoredState: DialogState
+        var index by mutableIntStateOf(0)
+        setContent {
+            val saveableStateHolder = rememberSaveableStateHolder()
+            saveableStateHolder.SaveableStateProvider(index) {
+                val state = rememberDialogState(size = DpSize.Unspecified)
+                if (index == 0) {
+                    restoredState = state
+                }
+            }
+        }
+
+        index = 1
+        waitForIdle()
+        index = 0
+        waitForIdle()
+
+        assertDialogStateEquals(expectedState, restoredState)
+    }
+}
+
+private fun assertDialogStateEquals(expected: DialogState, actual: DialogState) {
+    assertEquals(expected.size, actual.size, "size differs")
+    assertEquals(expected.position, actual.position, "position differs")
 }
diff --git a/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/WindowStateTest.kt b/compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/WindowStateTest.kt
@@ -20,15 +20,21 @@ import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.width
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableIntStateOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.saveable.rememberSaveableStateHolder
 import androidx.compose.runtime.setValue
-import androidx.compose.ui.*
+import androidx.compose.ui.Alignment
+import androidx.compose.ui.Modifier
 import androidx.compose.ui.awt.ComposeWindow
 import androidx.compose.ui.isLinux
 import androidx.compose.ui.isMacOs
 import androidx.compose.ui.platform.LocalWindowInfo
 import androidx.compose.ui.platform.WindowInfo
+import androidx.compose.ui.test.ExperimentalTestApi
+import androidx.compose.ui.test.runComposeUiTest
+import androidx.compose.ui.toDpSize
+import androidx.compose.ui.toWindowPosition
 import androidx.compose.ui.unit.Dp
 import androidx.compose.ui.unit.DpSize
 import androidx.compose.ui.unit.dp
@@ -48,6 +54,7 @@ import javax.swing.JFrame
 import kotlin.math.abs
 import kotlin.math.absoluteValue
 import kotlin.math.max
+import kotlin.test.assertEquals
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.channels.consumeEach
 import org.junit.Assume.assumeTrue
@@ -601,28 +608,43 @@ class WindowStateTest {
         }
 
         awaitIdle()
-        assertThat(lastRecomposedState.placement).isEqualTo(initialState.placement)
-        assertThat(lastRecomposedState.isMinimized).isEqualTo(initialState.isMinimized)
-        assertThat(lastRecomposedState.size).isEqualTo(initialState.size)
-        assertThat(lastRecomposedState.position).isEqualTo(initialState.position)
+        assertWindowStateEquals(lastRecomposedState, initialState)
         lastRecomposedState.placement = newState.placement
         lastRecomposedState.isMinimized = newState.isMinimized
         lastRecomposedState.size = newState.size
         lastRecomposedState.position = newState.position
 
         index = 1
         awaitIdle()
-        assertThat(lastRecomposedState.placement).isEqualTo(initialState.placement)
-        assertThat(lastRecomposedState.isMinimized).isEqualTo(initialState.isMinimized)
-        assertThat(lastRecomposedState.size).isEqualTo(initialState.size)
-        assertThat(lastRecomposedState.position).isEqualTo(initialState.position)
+        assertWindowStateEquals(lastRecomposedState, initialState)
 
         index = 0
         awaitIdle()
-        assertThat(lastRecomposedState.placement).isEqualTo(newState.placement)
-        assertThat(lastRecomposedState.isMinimized).isEqualTo(newState.isMinimized)
-        assertThat(lastRecomposedState.size).isEqualTo(newState.size)
-        assertThat(lastRecomposedState.position).isEqualTo(newState.position)
+        assertWindowStateEquals(lastRecomposedState, newState)
+    }
+
+    @OptIn(ExperimentalTestApi::class)
+    @Test
+    fun `can save Unspecified window size`() = runComposeUiTest {
+        val expectedState = WindowState(size = DpSize.Unspecified)
+        lateinit var restoredState: WindowState
+        var index by mutableIntStateOf(0)
+        setContent {
+            val saveableStateHolder = rememberSaveableStateHolder()
+            saveableStateHolder.SaveableStateProvider(index) {
+                val state = rememberWindowState(size = DpSize.Unspecified)
+                if (index == 0) {
+                    restoredState = state
+                }
+            }
+        }
+
+        index = 1
+        waitForIdle()
+        index = 0
+        waitForIdle()
+
+        assertWindowStateEquals(expectedState, restoredState)
     }
 
     @Test
@@ -908,3 +930,10 @@ private fun assertSizesNotApproximatelyEqual(
         )
     }
 }
+
+private fun assertWindowStateEquals(expected: WindowState, actual: WindowState) {
+    assertEquals(expected.placement, actual.placement, "Placement differs")
+    assertEquals(expected.isMinimized, actual.isMinimized, "isMinimized differs")
+    assertEquals(expected.size, actual.size, "size differs")
+    assertEquals(expected.position, actual.position, "position differs")
+}