Skip to content

Commit

Permalink
WIP: Break more cycles in widget bindings
Browse files Browse the repository at this point in the history
The ProtocolNode is a Kotlin object and the Widget is potentially
a Swift object. This introduces a potential retain cycle problem.
  • Loading branch information
squarejesse committed May 16, 2024
1 parent 3964f1c commit 2b3860d
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ import platform.UIKit.item
import platform.darwin.NSInteger
import platform.darwin.NSObject

internal open class UIViewLazyList() : LazyList<UIView>, ChangeListener {
internal open class UIViewLazyList : LazyList<UIView>, ChangeListener {
internal val tableView: UITableView = object : UITableView(
CGRectZero.readValue(),
UITableViewStyle.UITableViewStylePlain,
Expand Down Expand Up @@ -94,6 +94,10 @@ internal open class UIViewLazyList() : LazyList<UIView>, ChangeListener {
}
}

override fun isSizeOnlyPlaceholder(placeholder: UIView): Boolean {
return false // TODO
}

override fun insertRows(index: Int, count: Int) {
// TODO(jwilson): pass a range somehow when 'count' is large?
tableView.beginUpdates()
Expand All @@ -118,7 +122,7 @@ internal open class UIViewLazyList() : LazyList<UIView>, ChangeListener {
tableView.endUpdates()
}

override fun setContent(view: LazyListContainerCell, content: Widget<UIView>?) {
override fun setContent(view: LazyListContainerCell, content: UIView?) {
view.content = content
}
}
Expand Down Expand Up @@ -269,13 +273,13 @@ internal class LazyListContainerCell(
reuseIdentifier: String?,
) : UITableViewCell(style, reuseIdentifier) {
internal var binding: Binding<LazyListContainerCell, UIView>? = null
internal var content: Widget<UIView>? = null
internal var content: UIView? = null
set(value) {
field = value

removeAllSubviews()
if (value != null) {
contentView.addSubview(value.value)
contentView.addSubview(value)
}
setNeedsLayout()
}
Expand Down Expand Up @@ -319,12 +323,12 @@ internal class LazyListContainerCell(
super.layoutSubviews()

val content = this.content ?: return
content.value.setFrame(bounds)
content.setFrame(bounds)
contentView.setFrame(bounds)
}

override fun sizeThatFits(size: CValue<CGSize>): CValue<CGSize> {
return content?.value?.sizeThatFits(size) ?: return super.sizeThatFits(size)
return content?.sizeThatFits(size) ?: return super.sizeThatFits(size)
}

private fun removeAllSubviews() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
get() = itemsBefore.nonNullElements + loadedItems + itemsAfter.nonNullElements

/** Pool of placeholder widgets. */
private val placeholdersQueue = ArrayDeque<Widget<W>>()
private val placeholdersQueue = ArrayDeque<W>()

/**
* The first placeholder ever returned. We use it to choose measured dimensions for created
Expand All @@ -69,7 +69,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
override fun insert(index: Int, widget: Widget<W>) {
_widgets += widget
if (firstPlaceholder == null) firstPlaceholder = widget
placeholdersQueue += widget
placeholdersQueue += widget.value
}

override fun move(fromIndex: Int, toIndex: Int, count: Int) {
Expand All @@ -82,6 +82,10 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {

override fun onModifierUpdated(index: Int, widget: Widget<W>) {
}

override fun disconnect() {
_widgets.clear()
}
}

/** Changes to this list are collected and processed in batch once all changes are received. */
Expand Down Expand Up @@ -120,6 +124,10 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
}
}

override fun disconnect() {
_widgets.clear()
}

override fun onModifierUpdated(index: Int, widget: Widget<W>) {
}
}
Expand Down Expand Up @@ -168,7 +176,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
val toPromoteCount = minOf(edit.widgets.size, itemsBefore.size - newItemsBefore)
for (i in edit.widgets.size - 1 downTo edit.widgets.size - toPromoteCount) {
val placeholder = itemsBefore.removeLast()
loadedItems.add(0, placeholderToLoaded(placeholder, edit.widgets[i]))
loadedItems.add(0, placeholderToLoaded(placeholder, edit.widgets[i].value))
edit.widgets.removeAt(i)
}
} else if (
Expand Down Expand Up @@ -197,7 +205,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
for (i in 0 until toPromoteCount) {
val widget = edit.widgets.removeFirst()
val placeholder = itemsAfter.removeAt(0)
loadedItems.add(placeholderToLoaded(placeholder, widget))
loadedItems.add(placeholderToLoaded(placeholder, widget.value))
edit.index++
}
} else if (
Expand All @@ -220,7 +228,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
for (i in 0 until edit.widgets.size) {
val index = itemsBefore.size + edit.index + i
val binding = Binding(this).apply {
content = edit.widgets[i]
content = edit.widgets[i].value
}
loadedItems.add(edit.index + i, binding)

Expand Down Expand Up @@ -307,7 +315,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {

private fun placeholderToLoaded(
placeholder: Binding<V, W>?,
loadedContent: Widget<W>,
loadedContent: W,
): Binding<V, W> {
// No binding for this index. Create one.
if (placeholder == null) {
Expand Down Expand Up @@ -377,18 +385,18 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
}
}

private fun takePlaceholder(): Widget<W> {
private fun takePlaceholder(): W {
val result = placeholdersQueue.removeFirstOrNull()
if (result != null) return result

val created = createPlaceholder(firstPlaceholder!!.value)
return createPlaceholder(firstPlaceholder!!.value)
?: throw IllegalStateException("no more placeholders!")

return SizeOnlyPlaceholder(created)
}

private fun recyclePlaceholder(placeholder: Widget<W>) {
if (placeholder !is SizeOnlyPlaceholder) {
protected abstract fun isSizeOnlyPlaceholder(placeholder: W): Boolean

private fun recyclePlaceholder(placeholder: W) {
if (!isSizeOnlyPlaceholder(placeholder)) {
placeholdersQueue += placeholder
}
}
Expand All @@ -411,7 +419,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {

protected abstract fun deleteRows(index: Int, count: Int)

protected abstract fun setContent(view: V, content: Widget<W>?)
protected abstract fun setContent(view: V, content: W?)

/**
* Binds a UI-managed view to model-managed content.
Expand Down Expand Up @@ -440,7 +448,7 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
* The content of this binding; either a loaded widget or a placeholder. This should be
* `lateinit`, but it can't be because of the side-effect in set().
*/
internal var content: Widget<W>? = null
internal var content: W? = null
set(value) {
field = value
val view = this.view
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,13 @@ public class ProtocolBridge<W : Any>(
*/
public fun close() {
closed = true
for (node in nodes.values) {
for (childrenTag in factory.widgetChildren(node.widgetTag)) {
val children = node.children(childrenTag) ?: continue
children.nodes.clear()
children.children.disconnect()
}
}
nodes.clear()
pool.clear()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,8 @@ public class MutableListChildren<W : Any>(
override fun onModifierUpdated(index: Int, widget: Widget<W>) {
modifierUpdated()
}

override fun disconnect() {
container.clear()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,8 @@ public interface Widget<W : Any> {

/** Indicates that [Modifier]s for the child widgets have changed. */
public fun onModifierUpdated(index: Int, widget: Widget<W>)

public fun disconnect() {
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ public class UIViewChildren(
private fun invalidate() {
(container.superview ?: container).setNeedsLayout()
}

override fun disconnect() {
_widgets.clear()
}
}

private fun List<UIView>.remove(index: Int, count: Int): Array<UIView> {
Expand Down

0 comments on commit 2b3860d

Please sign in to comment.