From 60016e69f5456378c3a5b9a146daf0025eea207f Mon Sep 17 00:00:00 2001
From: Luke De Feo <lukedefeo@meta.com>
Date: Wed, 26 Jul 2023 04:58:43 -0700
Subject: [PATCH] Remove Node id default impl

Summary: This is never identity hash code for declarative framework so this default impl is just a source of bugs, including this one.

Reviewed By: lblasa

Differential Revision: D47754625

fbshipit-source-id: 470aab084c82fa847f25116342021a79d52b7c67
---
 .../jetpackcompose/descriptors/ComposeInnerViewDescriptor.kt  | 4 ++++
 .../jetpackcompose/descriptors/ComposeNodeDescriptor.kt       | 3 +++
 .../plugins/uidebugger/descriptors/ChainedDescriptor.kt       | 2 ++
 .../flipper/plugins/uidebugger/descriptors/NodeDescriptor.kt  | 2 +-
 .../plugins/uidebugger/descriptors/ObjectDescriptor.kt        | 2 ++
 5 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeInnerViewDescriptor.kt b/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeInnerViewDescriptor.kt
index 4e49937bb..8e430d89a 100644
--- a/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeInnerViewDescriptor.kt
+++ b/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeInnerViewDescriptor.kt
@@ -10,6 +10,7 @@ package com.facebook.flipper.plugins.jetpackcompose.descriptors
 import android.graphics.Bitmap
 import android.view.ViewGroup
 import com.facebook.flipper.plugins.jetpackcompose.model.ComposeInnerViewNode
+import com.facebook.flipper.plugins.uidebugger.descriptors.Id
 import com.facebook.flipper.plugins.uidebugger.descriptors.NodeDescriptor
 import com.facebook.flipper.plugins.uidebugger.descriptors.ViewDescriptor
 import com.facebook.flipper.plugins.uidebugger.descriptors.ViewGroupDescriptor
@@ -17,9 +18,12 @@ import com.facebook.flipper.plugins.uidebugger.model.Bounds
 import com.facebook.flipper.plugins.uidebugger.model.InspectableObject
 import com.facebook.flipper.plugins.uidebugger.model.MetadataId
 import com.facebook.flipper.plugins.uidebugger.util.MaybeDeferred
+import java.lang.System
 
 object ComposeInnerViewDescriptor : NodeDescriptor<ComposeInnerViewNode> {
 
+  override fun getId(node: ComposeInnerViewNode): Id = System.identityHashCode(node.view)
+
   override fun getBounds(node: ComposeInnerViewNode): Bounds {
     return node.bounds
   }
diff --git a/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeNodeDescriptor.kt b/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeNodeDescriptor.kt
index 16e294e58..22b5e0046 100644
--- a/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeNodeDescriptor.kt
+++ b/android/plugins/jetpack-compose/src/main/java/com/facebook/flipper/plugins/jetpackcompose/descriptors/ComposeNodeDescriptor.kt
@@ -10,6 +10,7 @@ package com.facebook.flipper.plugins.jetpackcompose.descriptors
 import android.graphics.Bitmap
 import com.facebook.flipper.plugins.jetpackcompose.model.ComposeNode
 import com.facebook.flipper.plugins.uidebugger.descriptors.BaseTags
+import com.facebook.flipper.plugins.uidebugger.descriptors.Id
 import com.facebook.flipper.plugins.uidebugger.descriptors.MetadataRegister
 import com.facebook.flipper.plugins.uidebugger.descriptors.NodeDescriptor
 import com.facebook.flipper.plugins.uidebugger.model.Bounds
@@ -134,4 +135,6 @@ object ComposeNodeDescriptor : NodeDescriptor<ComposeNode> {
   override fun getActiveChild(node: ComposeNode): Any? = null
 
   override fun getTags(node: ComposeNode): Set<String> = setOf(BaseTags.Android, "Compose")
+
+  override fun getId(node: ComposeNode): Id = node.inspectorNode.id.toInt()
 }
diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ChainedDescriptor.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ChainedDescriptor.kt
index 194778089..53078f374 100644
--- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ChainedDescriptor.kt
+++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ChainedDescriptor.kt
@@ -26,6 +26,8 @@ import com.facebook.flipper.plugins.uidebugger.util.MaybeDeferred
 abstract class ChainedDescriptor<T> : NodeDescriptor<T> {
   private var mSuper: ChainedDescriptor<T>? = null
 
+  override fun getId(node: T): Id = System.identityHashCode(node)
+
   fun setSuper(superDescriptor: ChainedDescriptor<T>) {
     if (superDescriptor !== mSuper) {
       check(mSuper == null)
diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/NodeDescriptor.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/NodeDescriptor.kt
index 413e4086c..d7b569c9d 100644
--- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/NodeDescriptor.kt
+++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/NodeDescriptor.kt
@@ -38,7 +38,7 @@ typealias Id = Int
 interface NodeDescriptor<T> {
 
   /** Used to detect the same node across traversals */
-  fun getId(node: T): Id = System.identityHashCode(node)
+  fun getId(node: T): Id
 
   /** Should be w.r.t the direct parent */
   fun getBounds(node: T): Bounds
diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ObjectDescriptor.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ObjectDescriptor.kt
index 78d4ea80f..f60127995 100644
--- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ObjectDescriptor.kt
+++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/ObjectDescriptor.kt
@@ -15,6 +15,8 @@ import com.facebook.flipper.plugins.uidebugger.util.Immediate
 
 object ObjectDescriptor : NodeDescriptor<Any> {
 
+  override fun getId(node: Any): Id = System.identityHashCode(node)
+
   override fun getActiveChild(node: Any): Any? = null
 
   override fun getName(node: Any): String {