Change UIViewLazyList so cells can be removed and re-added without re…

…use (#2242)

* Change UIViewLazyList so cells can be removed and re-added without reuse

We observed in some test suites that table cells would observe a sequence
of calls like this:

LazyListContainerCell.createView()
LazyListContainerCell.willMoveToSuperview(non-null)
LazyListContainerCell.willMoveToSuperview(null)
LazyListContainerCell.willMoveToSuperview(non-null)

Note that we didn't receive a call to prepareForReuse() between
the code being removed from its parent and added again. The code
wasn't prepared for this sequence of calls.

* apiDump

CHANGELOG.md

@@ -9,6 +9,7 @@ New:
 
 Changed:
 - `ProtocolFactory` interface is now sealed as arbitrary subtypes were never supported. Only schema-generated subtypes should be used.
+- `UIViewLazyList` doesn't crash with a `NullPointerException` if cells are added, removed, and re-added without being reused.
 
 Fixed:
 - Nothing yet!

redwood-lazylayout-uiview/src/commonMain/kotlin/app/cash/redwood/lazylayout/uiview/UIViewLazyList.kt

@@ -301,14 +301,13 @@ internal class LazyListContainerCell(
     }
 
     // Confirm the cell is bound when it's about to be displayed.
-    if (superview == null && newSuperview != null) {
-      require(binding!!.isBound) { "about to display a cell that isn't bound!" }
+    if (superview == null && newSuperview != null && !binding!!.isBound) {
+      binding!!.bind(this)
     }
 
     // Unbind the cell when its view is detached from the table.
     if (superview != null && newSuperview == null) {
       binding?.unbind()
-      binding = null
     }
   }
 

redwood-lazylayout-widget/api/redwood-lazylayout-widget.api

@@ -42,6 +42,7 @@ public abstract class app/cash/redwood/lazylayout/widget/LazyListUpdateProcessor
 }
 
 public final class app/cash/redwood/lazylayout/widget/LazyListUpdateProcessor$Binding {
+	public final fun bind (Ljava/lang/Object;)V
 	public final fun getView ()Ljava/lang/Object;
 	public final fun isBound ()Z
 	public final fun unbind ()V

redwood-lazylayout-widget/api/redwood-lazylayout-widget.klib.api

@@ -87,6 +87,7 @@ abstract class <#A: kotlin/Any, #B: kotlin/Any> app.cash.redwood.lazylayout.widg
         final var view // app.cash.redwood.lazylayout.widget/LazyListUpdateProcessor.Binding.view|{}view[0]
             final fun <get-view>(): #A1? // app.cash.redwood.lazylayout.widget/LazyListUpdateProcessor.Binding.view.<get-view>|<get-view>(){}[0]
 
+        final fun bind(#A1) // app.cash.redwood.lazylayout.widget/LazyListUpdateProcessor.Binding.bind|bind(1:0){}[0]
         final fun unbind() // app.cash.redwood.lazylayout.widget/LazyListUpdateProcessor.Binding.unbind|unbind(){}[0]
     }
 }

redwood-lazylayout-widget/src/commonMain/kotlin/app/cash/redwood/lazylayout/widget/LazyListUpdateProcessor.kt

@@ -314,22 +314,24 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
   }
 
   private fun placeholderToLoaded(
-    placeholder: Binding<V, W>?,
+    binding: Binding<V, W>?,
     loadedContent: Widget<W>,
   ): Binding<V, W> {
     // No binding for this index. Create one.
-    if (placeholder == null) {
+    if (binding == null) {
       return Binding(this).apply {
         setContentAndModifier(loadedContent)
       }
     }
 
     // We have a binding. Give it loaded content.
-    require(placeholder.isPlaceholder)
-    recyclePlaceholder(placeholder.content)
-    placeholder.isPlaceholder = false
-    placeholder.setContentAndModifier(loadedContent)
-    return placeholder
+    require(binding.isPlaceholder)
+    if (binding.isBound) {
+      recyclePlaceholder(binding.content!!)
+    }
+    binding.isPlaceholder = false
+    binding.setContentAndModifier(loadedContent)
+    return binding
   }
 
   private fun loadedToPlaceholder(loaded: Binding<V, W>): Binding<V, W>? {
@@ -438,8 +440,13 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
     public var view: V? = null
       private set
 
-    /** The content of this binding; either a loaded widget or a placeholder. */
-    internal lateinit var content: W
+    /**
+     * The content of this binding; either a loaded widget, a placeholder, or null.
+     *
+     * This may be null if its content is a placeholder that is not bound to a view. That way we
+     * don't take placeholders from the pool until we need them.
+     */
+    internal var content: W? = null
       private set
     private var modifier: Modifier = Modifier
 
@@ -458,9 +465,13 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
     public val isBound: Boolean
       get() = view != null
 
-    internal fun bind(view: V) {
+    public fun bind(view: V) {
       require(this.view == null) { "already bound" }
 
+      if (isPlaceholder && this.content == null) {
+        this.content = processor.takePlaceholder()
+      }
+
       this.view = view
       processor.setContent(view, content, modifier)
     }
@@ -481,7 +492,8 @@ public abstract class LazyListUpdateProcessor<V : Any, W : Any> {
         if (itemsAfterIndex != -1) processor.itemsAfter.set(itemsAfterIndex, null)
 
         // When a placeholder is reused, recycle its widget.
-        processor.recyclePlaceholder(content)
+        processor.recyclePlaceholder(content!!)
+        this.content = null
       }
     }
   }