Fix races with ImageComposeScene

ImageComposeScene will be introduced in the next commit. It performs in another thread, and have Unconfined coroutine dispatcher. This means all callbacks dispatched to that dispatcher will be performed in the caller thread.

For example, when GlobalSnapshotManager will sendApplyNotifications, all callbacks will be performed in Swing thread, and will race with `render`, which performs in the test thread.

To avoid that, we don't dispatch callback to coroutine dispatcher, instead we use our own dispatching inside ComposeScene via invalidate/render.

Fixes JetBrains/compose-multiplatform#1392

Change-Id: I732b37dd2fa5bb03e7e8f6a001f518199360c9a1
Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true

compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/GlobalSnapshotManager.desktop.kt

@@ -45,7 +45,10 @@ internal actual object GlobalSnapshotManager {
             val channel = Channel<Unit>(Channel.CONFLATED)
             CoroutineScope(Dispatchers.Swing).launch {
                 channel.consumeEach {
-                    Snapshot.sendApplyNotifications()
+                    // TODO(https://github.com/JetBrains/compose-jb/issues/1854) get rid of synchronized
+                    synchronized(GlobalSnapshotManager) {
+                        Snapshot.sendApplyNotifications()
+                    }
                 }
             }
             Snapshot.registerGlobalWriteObserver {

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/CommandList.desktop.kt

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.compose.ui
+
+import androidx.compose.ui.platform.synchronized
+
+/**
+ * Allows postponing execution of some code (command), adding it to the list via [add],
+ * and performing all added commands in some time in the future via [perform]
+ */
+internal class CommandList(
+    private var onNewCommand: () -> Unit
+) {
+    private val list = mutableListOf<() -> Unit>()
+    private val listCopy = mutableListOf<() -> Unit>()
+
+    /**
+     * true if there are any commands added.
+     *
+     * Can be called concurrently from multiple threads.
+     */
+    val hasCommands: Boolean get() = synchronized(list) {
+        list.isNotEmpty()
+    }
+
+    /**
+     * Add command to the list, and notify observer via [onNewCommand].
+     *
+     * Can be called concurrently from multiple threads.
+     */
+    fun add(command: () -> Unit) {
+        synchronized(list) {
+            list.add(command)
+        }
+        onNewCommand()
+    }
+
+    /**
+     * Clear added commands and perform them.
+     *
+     * Doesn't support multiple [perform]'s from different threads. But does support concurrent [perform]
+     * and concurrent [add].
+     */
+    fun perform() {
+        synchronized(list) {
+            listCopy.addAll(list)
+            list.clear()
+        }
+        listCopy.forEach { it.invoke() }
+        listCopy.clear()
+    }
+}

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/ComposeScene.skiko.kt

@@ -43,6 +43,7 @@ import androidx.compose.ui.platform.FlushCoroutineDispatcher
 import androidx.compose.ui.platform.GlobalSnapshotManager
 import androidx.compose.ui.platform.setContent
 import androidx.compose.ui.node.RootForTest
+import androidx.compose.ui.platform.synchronized
 import androidx.compose.ui.unit.Constraints
 import androidx.compose.ui.unit.Density
 import androidx.compose.ui.unit.IntSize
@@ -107,8 +108,15 @@ class ComposeScene internal constructor(
     @Volatile
     private var hasPendingDraws = true
     private inline fun <T> postponeInvalidation(block: () -> T): T {
+        check(!isClosed) { "ComposeScene is closed" }
         isInvalidationDisabled = true
         val result = try {
+            // We must see the actual state before we will do [block]
+            // TODO(https://github.com/JetBrains/compose-jb/issues/1854) get rid of synchronized.
+            synchronized(GlobalSnapshotManager) {
+                Snapshot.sendApplyNotifications()
+            }
+            snapshotChanges.perform()
             block()
         } finally {
             isInvalidationDisabled = false
@@ -119,7 +127,7 @@ class ComposeScene internal constructor(
 
     private fun invalidateIfNeeded() {
         hasPendingDraws = frameClock.hasAwaiters || needLayout || needDraw ||
-            list.any(SkiaBasedOwner::needRender)
+            list.any(SkiaBasedOwner::needRender) || snapshotChanges.hasCommands
         if (hasPendingDraws && !isInvalidationDisabled && !isClosed) {
             invalidate()
         }
@@ -213,11 +221,7 @@ class ComposeScene internal constructor(
         isClosed = true
     }
 
-    private fun dispatchCommand(command: () -> Unit) {
-        coroutineScope.launch {
-            command()
-        }
-    }
+    private val snapshotChanges = CommandList(::invalidateIfNeeded)
 
     /**
      * Returns true if there are pending recompositions, renders or dispatched tasks.
@@ -234,7 +238,7 @@ class ComposeScene internal constructor(
         owner.onNeedRender = ::invalidateIfNeeded
         owner.requestLayout = ::requestLayout
         owner.requestDraw = ::requestDraw
-        owner.onDispatchCommand = ::dispatchCommand
+        owner.dispatchSnapshotChanges = snapshotChanges::add
         owner.constraints = constraints
         owner.accessibilityController = makeAccessibilityController(
             owner,
@@ -249,7 +253,7 @@ class ComposeScene internal constructor(
     internal fun detach(owner: SkiaBasedOwner) {
         check(!isClosed) { "ComposeScene is closed" }
         list.remove(owner)
-        owner.onDispatchCommand = null
+        owner.dispatchSnapshotChanges = null
         owner.requestDraw = null
         owner.requestLayout = null
         owner.onNeedRender = null
@@ -313,7 +317,7 @@ class ComposeScene internal constructor(
         }
         this.mainOwner = mainOwner
 
-        // to perform all pending work synchronously. to start LaunchedEffect for example
+        // to perform all pending work synchronously
         recomposeDispatcher.flush()
     }
 
@@ -344,9 +348,6 @@ class ComposeScene internal constructor(
      * animations in the content (or any other code, which uses [withFrameNanos]
      */
     fun render(canvas: Canvas, nanoTime: Long): Unit = postponeInvalidation {
-        check(!isClosed) { "ComposeScene is closed" }
-        // We must see the actual state before we will render the frame
-        Snapshot.sendApplyNotifications()
         recomposeDispatcher.flush()
         frameClock.sendFrame(nanoTime)
         needLayout = false
@@ -372,8 +373,7 @@ class ComposeScene internal constructor(
         targetOwner: SkiaBasedOwner?
     ) = list.indexOf(this) > list.indexOf(targetOwner)
 
-    // TODO(demin): return Boolean (when it is consumed).
-    //  see ComposeLayer todo about AWTDebounceEventQueue
+    // TODO(demin): return Boolean (when it is consumed)
     /**
      * Send pointer event to the content.
      *
@@ -400,7 +400,6 @@ class ComposeScene internal constructor(
         keyboardModifiers: PointerKeyboardModifiers? = null,
         nativeEvent: Any? = null,
     ): Unit = postponeInvalidation {
-        check(!isClosed) { "ComposeScene is closed" }
         defaultPointerStateTracker.onPointerEvent(eventType)
 
         val actualButtons = buttons ?: defaultPointerStateTracker.buttons

compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/platform/SkiaBasedOwner.skiko.kt

@@ -193,7 +193,7 @@ internal class SkiaBasedOwner(
     override val rootForTest = this
 
     override val snapshotObserver = OwnerSnapshotObserver { command ->
-        onDispatchCommand?.invoke(command)
+        dispatchSnapshotChanges?.invoke(command)
     }
     private val pointerInputEventProcessor = PointerInputEventProcessor(root)
     private val measureAndLayoutDelegate = MeasureAndLayoutDelegate(root)
@@ -258,7 +258,7 @@ internal class SkiaBasedOwner(
     var onNeedRender: (() -> Unit)? = null
     var requestLayout: (() -> Unit)? = null
     var requestDraw: (() -> Unit)? = null
-    var onDispatchCommand: ((Command) -> Unit)? = null
+    var dispatchSnapshotChanges: ((Command) -> Unit)? = null
 
     private var needClearObservations = false