Skip to content

Commit

Permalink
Switch back to MutableMap to avoid scatter map bug
Browse files Browse the repository at this point in the history
Sometimes the scatter-based maps can return values from other keys when inserting a new key. See https://issuetracker.google.com/issues/378077858.
  • Loading branch information
JakeWharton committed Nov 8, 2024
1 parent e3e6281 commit 2052c85
Showing 1 changed file with 9 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import app.cash.redwood.protocol.RedwoodVersion
import app.cash.redwood.protocol.WidgetTag
import app.cash.redwood.widget.ChangeListener
import app.cash.redwood.widget.Widget
import kotlin.collections.mutableMapOf
import kotlin.native.ObjCName

/**
Expand All @@ -61,7 +62,7 @@ public class HostProtocolAdapter<W : Any>(
}

private val nodes =
mutableIntObjectMapOf<ProtocolNode<W>>(Id.Root.value, RootProtocolNode(container))
mutableMapOf<Int, ProtocolNode<W>>(Id.Root.value to RootProtocolNode(container))

private val removeNodeById = IdVisitor { nodes.remove(it.value) }

Expand Down Expand Up @@ -173,7 +174,7 @@ public class HostProtocolAdapter<W : Any>(
public fun close() {
closed = true

nodes.forEachValue { node ->
nodes.values.forEach { node ->
node.detach()
}
nodes.clear()
Expand Down Expand Up @@ -235,9 +236,10 @@ public class HostProtocolAdapter<W : Any>(
// Must have a Create node that precedes it.
if (lastCreatedId != change.id) continue

idToNode[change.id.value] = ReuseNode(
widgetId = change.id,
childrenTag = ChildrenTag.Root,
idToNode[lastCreatedId.value] = ReuseNode(
widgetId = lastCreatedId,
// This is the root of the reuse tree, so it will never have a children tag from a parent.
childrenTag = ChildrenTag(-1),
)
}

Expand Down Expand Up @@ -359,19 +361,17 @@ public class HostProtocolAdapter<W : Any>(
* descendants into the nodes map.
*/
fun assignPooledNodeRecursive(
nodes: MutableIntObjectMap<ProtocolNode<W>>,
nodes: MutableMap<Int, ProtocolNode<W>>,
changesAndNulls: Array<Change?>,
pooled: ProtocolNode<W>,
) {
// Reuse the node.
pooled.id = widgetId
val old = nodes.put(widgetId.value, pooled)
require(old == null) {
"Insert attempted to replace existing widget with ID ${widgetId.value}"
}

val skippedCreate = changesAndNulls[changeIndexForCreate] as Create
pooled.id = skippedCreate.id

// Remove the corresponding changes that we avoided by node reuse. We don't clear the 'Add'
// that adds the node to its new parent.
changesAndNulls[changeIndexForCreate] = null
Expand Down

0 comments on commit 2052c85

Please sign in to comment.