Use after pooling fields state to identify changes on fields state be…

…fore release. Summary: In order for a more accurate verification of the improper release of items, we are adding extra checks. We are now recording the state of the fields right when the item is created/pooled. This state is then compared to the one at the moment of the item release back into the pool. This allow us to ignore situations where there is a field/listener that is already set from the initial moment (as part of custom view for example). Reviewed By: adityasharat Differential Revision: D60234841 fbshipit-source-id: 7f4cd8a7d5a8d2f09365e8b90663b503c9fedf66
facebook · Jul 29, 2024 · 7dca42a · 7dca42a
1 parent 71bc4cb
commit 7dca42a
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 29 deletions.
diff --git a/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemPoolsReleaseValidator.kt b/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemPoolsReleaseValidator.kt
@@ -24,6 +24,7 @@ import android.widget.TextView
 import androidx.core.view.children
 import com.facebook.kotlin.compilerplugins.dataclassgenerate.annotation.DataClassGenerate
 import java.lang.reflect.Field
+import java.util.WeakHashMap
 
 /**
  * Inspects a [View] to understand if it has any view properties that were not cleaned up before the
@@ -43,6 +44,8 @@ internal class MountItemPoolsReleaseValidator(
     extraFields: List<FieldExtractionDefinition> = emptyList()
 ) {
 
+  private val pooledViewsToInitialState = WeakHashMap<View, Set<FieldState>>()
+
   private val fields =
       setOf(
           FieldExtractionDefinition("touchListener") {
@@ -87,6 +90,15 @@ internal class MountItemPoolsReleaseValidator(
             }
           }) + extraFields
 
+  fun registerAcquiredViewState(view: View) {
+    pooledViewsToInitialState[view] =
+        fields.map { field -> FieldState(field.id, field.extractor(view)) }.toSet()
+
+    if (view is ViewGroup) {
+      view.children.forEach { child -> registerAcquiredViewState(child) }
+    }
+  }
+
   fun assertValidRelease(view: View, hierarchyIdentifiers: List<String>) {
     if (!BuildConfig.DEBUG) {
       return
@@ -108,25 +120,34 @@ internal class MountItemPoolsReleaseValidator(
       return
     }
 
-    val unreleasedFields = fields.filter { field -> field.extractor(view) != null }
-    if (unreleasedFields.isNotEmpty()) {
-      val result = buildString {
-        append("Improper release detected: ${currentHierarchyIdentifier}\n")
-        unreleasedFields.forEach { field -> append("- ${field.id} | ${field.extractor(view)}\n") }
-
-        if (view is TextView) {
-          append("- text=${view.text}\n")
+    val beforeReleaseFieldsState =
+        fields.map { field -> FieldState(field.id, field.extractor(view)) }.toSet()
+
+    val afterPoolFieldsState = pooledViewsToInitialState.remove(view)
+    if (beforeReleaseFieldsState != afterPoolFieldsState) {
+      val differentFieldsState =
+          if (afterPoolFieldsState == null) beforeReleaseFieldsState
+          else beforeReleaseFieldsState.minus(afterPoolFieldsState)
+
+      val unreleasedFields = differentFieldsState.filter { field -> field.value != null }
+      if (unreleasedFields.isNotEmpty()) {
+        val result = buildString {
+          append("Improper release detected: ${currentHierarchyIdentifier}\n")
+          unreleasedFields.forEach { field -> append("- ${field.id} | ${field.value}\n") }
+
+          if (view is TextView) {
+            append("- text=${view.text}\n")
+          }
+          append("\n")
         }
-        append("\n")
-      }
 
-      onInvalidRelease?.invoke(InvalidReleaseToMountPoolException(result))
+        onInvalidRelease?.invoke(InvalidReleaseToMountPoolException(result))
 
-      if (failOnDetection) {
-        assert(false) { result }
-      } else {
-        Log.d(TAG, currentHierarchyIdentifier)
-        Log.d(TAG, result)
+        if (failOnDetection) {
+          assert(false) { result }
+        } else {
+          Log.d(TAG, result)
+        }
       }
     }
   }
@@ -170,6 +191,8 @@ internal class MountItemPoolsReleaseValidator(
 
   @DataClassGenerate
   data class FieldExtractionDefinition(val id: String, val extractor: (View) -> Any?)
+
+  @DataClassGenerate data class FieldState(val id: String, val value: Any?)
 }
 
 private const val TAG = "MountReleaseValidator"

diff --git a/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemsPool.kt b/litho-rendercore/src/main/java/com/facebook/rendercore/MountItemsPool.kt
@@ -92,19 +92,25 @@ object MountItemsPool {
         }
 
     return if (content != null) {
-      content
-    } else {
-      val isTracing = RenderCoreSystrace.isTracing()
-      if (isTracing) {
-        RenderCoreSystrace.beginSection(
-            "MountItemsPool:createMountContent ${poolableMountContent.getPoolableContentType().simpleName}")
-      }
-      val content = poolableMountContent.createPoolableContent(context)
-      if (isTracing) {
-        RenderCoreSystrace.endSection()
-      }
-      content
-    }
+          content
+        } else {
+          val isTracing = RenderCoreSystrace.isTracing()
+          if (isTracing) {
+            RenderCoreSystrace.beginSection(
+                "MountItemsPool:createMountContent ${poolableMountContent.getPoolableContentType().simpleName}")
+          }
+          val content = poolableMountContent.createPoolableContent(context)
+          if (isTracing) {
+            RenderCoreSystrace.endSection()
+          }
+
+          content
+        }
+        .also { content ->
+          if (content is View) {
+            mountItemPoolsReleaseValidator?.registerAcquiredViewState(content)
+          }
+        }
   }
 
   @JvmStatic