From 0ebedc9c493137e12714898883a83c7e0359f5d5 Mon Sep 17 00:00:00 2001 From: Luke De Feo Date: Wed, 16 Nov 2022 10:38:23 -0800 Subject: [PATCH] Add Id to node descriptor interface Summary: There were 2 situations where the UI hadn't changed but we were sending a nodes with a different Id accross traversals which confuses things on the desktop 1. By removing the litho observer we are repeatidly scanning the entire hierachy on scroll or when a video plays. The debug component that represents litho views are not stable, they are generated each time even if the underlying component is the same 2. Offset child is generated by us each time and the old id refered to the generated offset child not the underlying object This diff addresses these 2 cases by allowing a descriptor to choose the ID. The nodeId convience method was used in a lot of places. For the places where the id ended up in the protocol we have migrated to the descriptors getID. In some other places it is only used internally or for logging. In this case I have renamed the method and changed the return type from Id to int so it cant be accidently used in the protocol Reviewed By: lblasa Differential Revision: D41307239 fbshipit-source-id: 655b230180a6d02d66888e86b2293eead3385455 --- .../plugins/uidebugger/litho/LithoObserver.kt | 13 +++++++------ .../litho/descriptors/DebugComponentDescriptor.kt | 6 ++++++ .../plugins/uidebugger/UIDebuggerFlipperPlugin.kt | 6 ++++-- .../plugins/uidebugger/core/NativeScanScheduler.kt | 7 +++++-- .../uidebugger/descriptors/NodeDescriptor.kt | 11 +++++------ .../uidebugger/descriptors/OffsetChildDescriptor.kt | 2 ++ .../plugins/uidebugger/observers/TreeObserver.kt | 12 ++++++------ .../uidebugger/traversal/PartialLayoutTraversal.kt | 13 ++++++++----- .../plugins/uidebugger/util/ObjectIdentity.kt | 12 ++++++++++++ 9 files changed, 55 insertions(+), 27 deletions(-) create mode 100644 android/src/main/java/com/facebook/flipper/plugins/uidebugger/util/ObjectIdentity.kt diff --git a/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/LithoObserver.kt b/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/LithoObserver.kt index 1a209d319..5700b4f9a 100644 --- a/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/LithoObserver.kt +++ b/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/LithoObserver.kt @@ -13,7 +13,6 @@ import android.view.ViewTreeObserver import com.facebook.flipper.plugins.uidebugger.LogTag import com.facebook.flipper.plugins.uidebugger.core.Context import com.facebook.flipper.plugins.uidebugger.descriptors.ViewDescriptor -import com.facebook.flipper.plugins.uidebugger.descriptors.nodeId import com.facebook.flipper.plugins.uidebugger.litho.descriptors.LithoViewDescriptor import com.facebook.flipper.plugins.uidebugger.model.Bounds import com.facebook.flipper.plugins.uidebugger.model.Coordinate @@ -21,6 +20,7 @@ import com.facebook.flipper.plugins.uidebugger.observers.CoordinateUpdate import com.facebook.flipper.plugins.uidebugger.observers.TreeObserver import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverBuilder import com.facebook.flipper.plugins.uidebugger.scheduler.throttleLatest +import com.facebook.flipper.plugins.uidebugger.util.objectIdentity import com.facebook.litho.LithoView import com.facebook.rendercore.extensions.ExtensionState import com.facebook.rendercore.extensions.MountExtension @@ -56,10 +56,10 @@ class LithoViewTreeObserver(val context: Context) : TreeObserver() { @SuppressLint("PrivateApi") override fun subscribe(node: Any) { - Log.d(LogTag, "Subscribing to litho view ${node.nodeId()}") - nodeRef = node as LithoView + Log.d(LogTag, "Subscribing to litho view ${nodeRef?.objectIdentity()}") + val lithoDebuggerExtension = LithoDebuggerExtension(this) node.registerUIDebugger(lithoDebuggerExtension) @@ -69,7 +69,8 @@ class LithoViewTreeObserver(val context: Context) : TreeObserver() { val bounds = ViewDescriptor.onGetBounds(node) if (bounds != lastBounds) { context.treeObserverManager.enqueueUpdate( - CoordinateUpdate(this.type, node.nodeId(), Coordinate(bounds.x, bounds.y))) + CoordinateUpdate( + this.type, LithoViewDescriptor.getId(node), Coordinate(bounds.x, bounds.y))) lastBounds = bounds } } @@ -89,7 +90,7 @@ class LithoViewTreeObserver(val context: Context) : TreeObserver() { } override fun unsubscribe() { - Log.d(LogTag, "Unsubscribing from litho view ${nodeRef?.nodeId()}") + Log.d(LogTag, "Unsubscribing from litho view ${nodeRef?.objectIdentity()}") nodeRef?.viewTreeObserver?.removeOnPreDrawListener(preDrawListener) nodeRef?.unregisterUIDebugger() nodeRef = null @@ -107,7 +108,7 @@ class LithoDebuggerExtension(val observer: LithoViewTreeObserver) : MountExtensi * mounting includes adding updating or removing views from the heriachy */ override fun afterMount(state: ExtensionState) { - Log.i(LogTag, "After mount called for litho view ${observer.nodeRef?.nodeId()}") + Log.i(LogTag, "After mount called for litho view ${observer.nodeRef?.objectIdentity()}") observer.lastBounds = ViewDescriptor.onGetBounds(state.rootHost) observer.processUpdate(observer.context, state.rootHost as Any) } diff --git a/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/DebugComponentDescriptor.kt b/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/DebugComponentDescriptor.kt index 3614697e3..d88df74e4 100644 --- a/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/DebugComponentDescriptor.kt +++ b/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/DebugComponentDescriptor.kt @@ -20,6 +20,12 @@ import com.facebook.litho.DebugComponent class DebugComponentDescriptor(val register: DescriptorRegister) : NodeDescriptor { private val NAMESPACE = "DebugComponent" + /* + * Debug component is generated on the fly so use the underlying component instance which is + * immutable + */ + override fun getId(node: DebugComponent): Id = System.identityHashCode(node.component) + override fun getName(node: DebugComponent): String = node.component.simpleName override fun getQualifiedName(node: com.facebook.litho.DebugComponent): String = diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/UIDebuggerFlipperPlugin.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/UIDebuggerFlipperPlugin.kt index fc76db82d..b5bf9ae54 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/UIDebuggerFlipperPlugin.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/UIDebuggerFlipperPlugin.kt @@ -12,9 +12,9 @@ import android.util.Log import com.facebook.flipper.core.FlipperConnection import com.facebook.flipper.core.FlipperPlugin import com.facebook.flipper.plugins.uidebugger.core.* +import com.facebook.flipper.plugins.uidebugger.descriptors.ApplicationRefDescriptor import com.facebook.flipper.plugins.uidebugger.descriptors.DescriptorRegister import com.facebook.flipper.plugins.uidebugger.descriptors.MetadataRegister -import com.facebook.flipper.plugins.uidebugger.descriptors.nodeId import com.facebook.flipper.plugins.uidebugger.model.InitEvent import com.facebook.flipper.plugins.uidebugger.model.MetadataUpdateEvent import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverFactory @@ -51,7 +51,9 @@ class UIDebuggerFlipperPlugin( connection.send( InitEvent.name, - Json.encodeToString(InitEvent.serializer(), InitEvent(context.applicationRef.nodeId()))) + Json.encodeToString( + InitEvent.serializer(), + InitEvent(ApplicationRefDescriptor.getId(context.applicationRef)))) connection.send( MetadataUpdateEvent.name, diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/NativeScanScheduler.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/NativeScanScheduler.kt index 9276d93a6..582353bba 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/NativeScanScheduler.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/core/NativeScanScheduler.kt @@ -10,8 +10,8 @@ package com.facebook.flipper.plugins.uidebugger.core import android.os.Looper import android.util.Log import com.facebook.flipper.plugins.uidebugger.LogTag +import com.facebook.flipper.plugins.uidebugger.descriptors.ApplicationRefDescriptor import com.facebook.flipper.plugins.uidebugger.descriptors.MetadataRegister -import com.facebook.flipper.plugins.uidebugger.descriptors.nodeId import com.facebook.flipper.plugins.uidebugger.model.MetadataUpdateEvent import com.facebook.flipper.plugins.uidebugger.model.Node import com.facebook.flipper.plugins.uidebugger.model.PerfStatsEvent @@ -66,7 +66,10 @@ class NativeScanScheduler(val context: Context) : Scheduler.Task { Json.encodeToString( SubtreeUpdateEvent.serializer(), SubtreeUpdateEvent( - input.txId, observerType, context.applicationRef.nodeId(), input.nodes)) + input.txId, + observerType, + ApplicationRefDescriptor.getId(context.applicationRef), + input.nodes)) val serializationEnd = System.currentTimeMillis() context.connectionRef.connection?.send( 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 1fda161ee..ac075a36f 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 @@ -31,8 +31,13 @@ object BaseTags { val AccessibleNativeAndroid = setOf(Android, Native, Accessibility) } +typealias Id = Int + interface NodeDescriptor { + /** Used to detect the same node across traversals */ + fun getId(node: T): Id = System.identityHashCode(node) + /** Should be w.r.t the direct parent */ fun getBounds(node: T): Bounds @@ -76,9 +81,3 @@ interface NodeDescriptor { */ fun getTags(node: T): Set } - -typealias Id = Int - -fun Any.nodeId(): Id { - return System.identityHashCode(this) -} diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/OffsetChildDescriptor.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/OffsetChildDescriptor.kt index a606787b5..04f57c17e 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/OffsetChildDescriptor.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/OffsetChildDescriptor.kt @@ -21,6 +21,8 @@ class OffsetChild(val child: Any, val descriptor: NodeDescriptor, val x: In object OffsetChildDescriptor : NodeDescriptor { + override fun getId(node: OffsetChild): Id = node.descriptor.getId(node.child) + override fun getBounds(node: OffsetChild): Bounds { val bounds = node.descriptor.getBounds(node.child) return Bounds(node.x, node.y, bounds.width, bounds.height) diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt index 687295026..f8aad5695 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserver.kt @@ -13,7 +13,7 @@ import com.facebook.flipper.plugins.uidebugger.common.BitmapPool import com.facebook.flipper.plugins.uidebugger.core.Context import com.facebook.flipper.plugins.uidebugger.descriptors.Id import com.facebook.flipper.plugins.uidebugger.descriptors.NodeDescriptor -import com.facebook.flipper.plugins.uidebugger.descriptors.nodeId +import com.facebook.flipper.plugins.uidebugger.util.objectIdentity /* * Represents a stateful observer that manages some subtree in the UI Hierarchy. @@ -49,19 +49,19 @@ abstract class TreeObserver { // Add any new observers observableRoots.forEach { observable -> - if (!children.containsKey(observable.nodeId())) { + if (!children.containsKey(observable.objectIdentity())) { context.observerFactory.createObserver(observable, context)?.let { observer -> Log.d( LogTag, - "Observer ${this.type} discovered new child of type ${observer.type} Node ID ${observable.nodeId()}") + "Observer ${this.type} discovered new child of type ${observer.type} Node ID ${observable.objectIdentity()}") observer.subscribe(observable) - children[observable.nodeId()] = observer + children[observable.objectIdentity()] = observer } } } // Remove any old observers - val observableRootsIdentifiers = observableRoots.map { it.nodeId() } + val observableRootsIdentifiers = observableRoots.map { it.objectIdentity() } val removables = mutableListOf() children.keys.forEach { key -> if (!observableRootsIdentifiers.contains(key)) { @@ -92,7 +92,7 @@ abstract class TreeObserver { context.treeObserverManager.enqueueUpdate( SubtreeUpdate( type, - root.nodeId(), + root.objectIdentity(), visitedNodes, startTimestamp, traversalCompleteTime, diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/traversal/PartialLayoutTraversal.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/traversal/PartialLayoutTraversal.kt index 3626c4d12..26fd41dfc 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/traversal/PartialLayoutTraversal.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/traversal/PartialLayoutTraversal.kt @@ -12,7 +12,6 @@ import com.facebook.flipper.plugins.uidebugger.LogTag import com.facebook.flipper.plugins.uidebugger.descriptors.DescriptorRegister import com.facebook.flipper.plugins.uidebugger.descriptors.Id import com.facebook.flipper.plugins.uidebugger.descriptors.NodeDescriptor -import com.facebook.flipper.plugins.uidebugger.descriptors.nodeId import com.facebook.flipper.plugins.uidebugger.model.Node import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverFactory @@ -55,7 +54,7 @@ class PartialLayoutTraversal( if (shallow.contains(node)) { visited.add( Node( - node.nodeId(), + descriptor.getId(node), descriptor.getQualifiedName(node), descriptor.getName(node), emptyMap(), @@ -71,14 +70,18 @@ class PartialLayoutTraversal( val children = descriptor.getChildren(node) val activeChild = descriptor.getActiveChild(node) + var activeChildId: Id? = null if (activeChild != null) { - activeChildId = activeChild.nodeId() + val activeChildDescriptor = + descriptorRegister.descriptorForClassUnsafe(activeChild.javaClass) + activeChildId = activeChildDescriptor.getId(activeChild) } val childrenIds = mutableListOf() children.forEach { child -> - childrenIds.add(child.nodeId()) + val childDescriptor = descriptorRegister.descriptorForClassUnsafe(child.javaClass) + childrenIds.add(childDescriptor.getId(child)) stack.add(child) // If there is an active child then don't traverse it if (activeChild != null && activeChild != child) { @@ -91,7 +94,7 @@ class PartialLayoutTraversal( val tags = descriptor.getTags(node) visited.add( Node( - node.nodeId(), + descriptor.getId(node), descriptor.getQualifiedName(node), descriptor.getName(node), attributes, diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/util/ObjectIdentity.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/util/ObjectIdentity.kt new file mode 100644 index 000000000..5961a34c0 --- /dev/null +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/util/ObjectIdentity.kt @@ -0,0 +1,12 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.flipper.plugins.uidebugger.util + +fun Any.objectIdentity(): Int { + return System.identityHashCode(this) +}