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
This commit is contained in:
committed by
Facebook GitHub Bot
parent
f6060d0046
commit
0ebedc9c49
@@ -13,7 +13,6 @@ import android.view.ViewTreeObserver
|
|||||||
import com.facebook.flipper.plugins.uidebugger.LogTag
|
import com.facebook.flipper.plugins.uidebugger.LogTag
|
||||||
import com.facebook.flipper.plugins.uidebugger.core.Context
|
import com.facebook.flipper.plugins.uidebugger.core.Context
|
||||||
import com.facebook.flipper.plugins.uidebugger.descriptors.ViewDescriptor
|
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.litho.descriptors.LithoViewDescriptor
|
||||||
import com.facebook.flipper.plugins.uidebugger.model.Bounds
|
import com.facebook.flipper.plugins.uidebugger.model.Bounds
|
||||||
import com.facebook.flipper.plugins.uidebugger.model.Coordinate
|
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.TreeObserver
|
||||||
import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverBuilder
|
import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverBuilder
|
||||||
import com.facebook.flipper.plugins.uidebugger.scheduler.throttleLatest
|
import com.facebook.flipper.plugins.uidebugger.scheduler.throttleLatest
|
||||||
|
import com.facebook.flipper.plugins.uidebugger.util.objectIdentity
|
||||||
import com.facebook.litho.LithoView
|
import com.facebook.litho.LithoView
|
||||||
import com.facebook.rendercore.extensions.ExtensionState
|
import com.facebook.rendercore.extensions.ExtensionState
|
||||||
import com.facebook.rendercore.extensions.MountExtension
|
import com.facebook.rendercore.extensions.MountExtension
|
||||||
@@ -56,10 +56,10 @@ class LithoViewTreeObserver(val context: Context) : TreeObserver<LithoView>() {
|
|||||||
@SuppressLint("PrivateApi")
|
@SuppressLint("PrivateApi")
|
||||||
override fun subscribe(node: Any) {
|
override fun subscribe(node: Any) {
|
||||||
|
|
||||||
Log.d(LogTag, "Subscribing to litho view ${node.nodeId()}")
|
|
||||||
|
|
||||||
nodeRef = node as LithoView
|
nodeRef = node as LithoView
|
||||||
|
|
||||||
|
Log.d(LogTag, "Subscribing to litho view ${nodeRef?.objectIdentity()}")
|
||||||
|
|
||||||
val lithoDebuggerExtension = LithoDebuggerExtension(this)
|
val lithoDebuggerExtension = LithoDebuggerExtension(this)
|
||||||
node.registerUIDebugger(lithoDebuggerExtension)
|
node.registerUIDebugger(lithoDebuggerExtension)
|
||||||
|
|
||||||
@@ -69,7 +69,8 @@ class LithoViewTreeObserver(val context: Context) : TreeObserver<LithoView>() {
|
|||||||
val bounds = ViewDescriptor.onGetBounds(node)
|
val bounds = ViewDescriptor.onGetBounds(node)
|
||||||
if (bounds != lastBounds) {
|
if (bounds != lastBounds) {
|
||||||
context.treeObserverManager.enqueueUpdate(
|
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
|
lastBounds = bounds
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -89,7 +90,7 @@ class LithoViewTreeObserver(val context: Context) : TreeObserver<LithoView>() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
override fun unsubscribe() {
|
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?.viewTreeObserver?.removeOnPreDrawListener(preDrawListener)
|
||||||
nodeRef?.unregisterUIDebugger()
|
nodeRef?.unregisterUIDebugger()
|
||||||
nodeRef = null
|
nodeRef = null
|
||||||
@@ -107,7 +108,7 @@ class LithoDebuggerExtension(val observer: LithoViewTreeObserver) : MountExtensi
|
|||||||
* mounting includes adding updating or removing views from the heriachy
|
* mounting includes adding updating or removing views from the heriachy
|
||||||
*/
|
*/
|
||||||
override fun afterMount(state: ExtensionState<Void?>) {
|
override fun afterMount(state: ExtensionState<Void?>) {
|
||||||
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.lastBounds = ViewDescriptor.onGetBounds(state.rootHost)
|
||||||
observer.processUpdate(observer.context, state.rootHost as Any)
|
observer.processUpdate(observer.context, state.rootHost as Any)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -20,6 +20,12 @@ import com.facebook.litho.DebugComponent
|
|||||||
class DebugComponentDescriptor(val register: DescriptorRegister) : NodeDescriptor<DebugComponent> {
|
class DebugComponentDescriptor(val register: DescriptorRegister) : NodeDescriptor<DebugComponent> {
|
||||||
private val NAMESPACE = "DebugComponent"
|
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 getName(node: DebugComponent): String = node.component.simpleName
|
||||||
|
|
||||||
override fun getQualifiedName(node: com.facebook.litho.DebugComponent): String =
|
override fun getQualifiedName(node: com.facebook.litho.DebugComponent): String =
|
||||||
|
|||||||
@@ -12,9 +12,9 @@ import android.util.Log
|
|||||||
import com.facebook.flipper.core.FlipperConnection
|
import com.facebook.flipper.core.FlipperConnection
|
||||||
import com.facebook.flipper.core.FlipperPlugin
|
import com.facebook.flipper.core.FlipperPlugin
|
||||||
import com.facebook.flipper.plugins.uidebugger.core.*
|
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.DescriptorRegister
|
||||||
import com.facebook.flipper.plugins.uidebugger.descriptors.MetadataRegister
|
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.InitEvent
|
||||||
import com.facebook.flipper.plugins.uidebugger.model.MetadataUpdateEvent
|
import com.facebook.flipper.plugins.uidebugger.model.MetadataUpdateEvent
|
||||||
import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverFactory
|
import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverFactory
|
||||||
@@ -51,7 +51,9 @@ class UIDebuggerFlipperPlugin(
|
|||||||
|
|
||||||
connection.send(
|
connection.send(
|
||||||
InitEvent.name,
|
InitEvent.name,
|
||||||
Json.encodeToString(InitEvent.serializer(), InitEvent(context.applicationRef.nodeId())))
|
Json.encodeToString(
|
||||||
|
InitEvent.serializer(),
|
||||||
|
InitEvent(ApplicationRefDescriptor.getId(context.applicationRef))))
|
||||||
|
|
||||||
connection.send(
|
connection.send(
|
||||||
MetadataUpdateEvent.name,
|
MetadataUpdateEvent.name,
|
||||||
|
|||||||
@@ -10,8 +10,8 @@ package com.facebook.flipper.plugins.uidebugger.core
|
|||||||
import android.os.Looper
|
import android.os.Looper
|
||||||
import android.util.Log
|
import android.util.Log
|
||||||
import com.facebook.flipper.plugins.uidebugger.LogTag
|
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.MetadataRegister
|
||||||
import com.facebook.flipper.plugins.uidebugger.descriptors.nodeId
|
|
||||||
import com.facebook.flipper.plugins.uidebugger.model.MetadataUpdateEvent
|
import com.facebook.flipper.plugins.uidebugger.model.MetadataUpdateEvent
|
||||||
import com.facebook.flipper.plugins.uidebugger.model.Node
|
import com.facebook.flipper.plugins.uidebugger.model.Node
|
||||||
import com.facebook.flipper.plugins.uidebugger.model.PerfStatsEvent
|
import com.facebook.flipper.plugins.uidebugger.model.PerfStatsEvent
|
||||||
@@ -66,7 +66,10 @@ class NativeScanScheduler(val context: Context) : Scheduler.Task<ScanResult> {
|
|||||||
Json.encodeToString(
|
Json.encodeToString(
|
||||||
SubtreeUpdateEvent.serializer(),
|
SubtreeUpdateEvent.serializer(),
|
||||||
SubtreeUpdateEvent(
|
SubtreeUpdateEvent(
|
||||||
input.txId, observerType, context.applicationRef.nodeId(), input.nodes))
|
input.txId,
|
||||||
|
observerType,
|
||||||
|
ApplicationRefDescriptor.getId(context.applicationRef),
|
||||||
|
input.nodes))
|
||||||
val serializationEnd = System.currentTimeMillis()
|
val serializationEnd = System.currentTimeMillis()
|
||||||
|
|
||||||
context.connectionRef.connection?.send(
|
context.connectionRef.connection?.send(
|
||||||
|
|||||||
@@ -31,8 +31,13 @@ object BaseTags {
|
|||||||
val AccessibleNativeAndroid = setOf(Android, Native, Accessibility)
|
val AccessibleNativeAndroid = setOf(Android, Native, Accessibility)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
typealias Id = Int
|
||||||
|
|
||||||
interface NodeDescriptor<T> {
|
interface NodeDescriptor<T> {
|
||||||
|
|
||||||
|
/** Used to detect the same node across traversals */
|
||||||
|
fun getId(node: T): Id = System.identityHashCode(node)
|
||||||
|
|
||||||
/** Should be w.r.t the direct parent */
|
/** Should be w.r.t the direct parent */
|
||||||
fun getBounds(node: T): Bounds
|
fun getBounds(node: T): Bounds
|
||||||
|
|
||||||
@@ -76,9 +81,3 @@ interface NodeDescriptor<T> {
|
|||||||
*/
|
*/
|
||||||
fun getTags(node: T): Set<String>
|
fun getTags(node: T): Set<String>
|
||||||
}
|
}
|
||||||
|
|
||||||
typealias Id = Int
|
|
||||||
|
|
||||||
fun Any.nodeId(): Id {
|
|
||||||
return System.identityHashCode(this)
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -21,6 +21,8 @@ class OffsetChild(val child: Any, val descriptor: NodeDescriptor<Any>, val x: In
|
|||||||
|
|
||||||
object OffsetChildDescriptor : NodeDescriptor<OffsetChild> {
|
object OffsetChildDescriptor : NodeDescriptor<OffsetChild> {
|
||||||
|
|
||||||
|
override fun getId(node: OffsetChild): Id = node.descriptor.getId(node.child)
|
||||||
|
|
||||||
override fun getBounds(node: OffsetChild): Bounds {
|
override fun getBounds(node: OffsetChild): Bounds {
|
||||||
val bounds = node.descriptor.getBounds(node.child)
|
val bounds = node.descriptor.getBounds(node.child)
|
||||||
return Bounds(node.x, node.y, bounds.width, bounds.height)
|
return Bounds(node.x, node.y, bounds.width, bounds.height)
|
||||||
|
|||||||
@@ -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.core.Context
|
||||||
import com.facebook.flipper.plugins.uidebugger.descriptors.Id
|
import com.facebook.flipper.plugins.uidebugger.descriptors.Id
|
||||||
import com.facebook.flipper.plugins.uidebugger.descriptors.NodeDescriptor
|
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.
|
* Represents a stateful observer that manages some subtree in the UI Hierarchy.
|
||||||
@@ -49,19 +49,19 @@ abstract class TreeObserver<T> {
|
|||||||
|
|
||||||
// Add any new observers
|
// Add any new observers
|
||||||
observableRoots.forEach { observable ->
|
observableRoots.forEach { observable ->
|
||||||
if (!children.containsKey(observable.nodeId())) {
|
if (!children.containsKey(observable.objectIdentity())) {
|
||||||
context.observerFactory.createObserver(observable, context)?.let { observer ->
|
context.observerFactory.createObserver(observable, context)?.let { observer ->
|
||||||
Log.d(
|
Log.d(
|
||||||
LogTag,
|
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)
|
observer.subscribe(observable)
|
||||||
children[observable.nodeId()] = observer
|
children[observable.objectIdentity()] = observer
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Remove any old observers
|
// Remove any old observers
|
||||||
val observableRootsIdentifiers = observableRoots.map { it.nodeId() }
|
val observableRootsIdentifiers = observableRoots.map { it.objectIdentity() }
|
||||||
val removables = mutableListOf<Id>()
|
val removables = mutableListOf<Id>()
|
||||||
children.keys.forEach { key ->
|
children.keys.forEach { key ->
|
||||||
if (!observableRootsIdentifiers.contains(key)) {
|
if (!observableRootsIdentifiers.contains(key)) {
|
||||||
@@ -92,7 +92,7 @@ abstract class TreeObserver<T> {
|
|||||||
context.treeObserverManager.enqueueUpdate(
|
context.treeObserverManager.enqueueUpdate(
|
||||||
SubtreeUpdate(
|
SubtreeUpdate(
|
||||||
type,
|
type,
|
||||||
root.nodeId(),
|
root.objectIdentity(),
|
||||||
visitedNodes,
|
visitedNodes,
|
||||||
startTimestamp,
|
startTimestamp,
|
||||||
traversalCompleteTime,
|
traversalCompleteTime,
|
||||||
|
|||||||
@@ -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.DescriptorRegister
|
||||||
import com.facebook.flipper.plugins.uidebugger.descriptors.Id
|
import com.facebook.flipper.plugins.uidebugger.descriptors.Id
|
||||||
import com.facebook.flipper.plugins.uidebugger.descriptors.NodeDescriptor
|
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.model.Node
|
||||||
import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverFactory
|
import com.facebook.flipper.plugins.uidebugger.observers.TreeObserverFactory
|
||||||
|
|
||||||
@@ -55,7 +54,7 @@ class PartialLayoutTraversal(
|
|||||||
if (shallow.contains(node)) {
|
if (shallow.contains(node)) {
|
||||||
visited.add(
|
visited.add(
|
||||||
Node(
|
Node(
|
||||||
node.nodeId(),
|
descriptor.getId(node),
|
||||||
descriptor.getQualifiedName(node),
|
descriptor.getQualifiedName(node),
|
||||||
descriptor.getName(node),
|
descriptor.getName(node),
|
||||||
emptyMap(),
|
emptyMap(),
|
||||||
@@ -71,14 +70,18 @@ class PartialLayoutTraversal(
|
|||||||
val children = descriptor.getChildren(node)
|
val children = descriptor.getChildren(node)
|
||||||
|
|
||||||
val activeChild = descriptor.getActiveChild(node)
|
val activeChild = descriptor.getActiveChild(node)
|
||||||
|
|
||||||
var activeChildId: Id? = null
|
var activeChildId: Id? = null
|
||||||
if (activeChild != null) {
|
if (activeChild != null) {
|
||||||
activeChildId = activeChild.nodeId()
|
val activeChildDescriptor =
|
||||||
|
descriptorRegister.descriptorForClassUnsafe(activeChild.javaClass)
|
||||||
|
activeChildId = activeChildDescriptor.getId(activeChild)
|
||||||
}
|
}
|
||||||
|
|
||||||
val childrenIds = mutableListOf<Id>()
|
val childrenIds = mutableListOf<Id>()
|
||||||
children.forEach { child ->
|
children.forEach { child ->
|
||||||
childrenIds.add(child.nodeId())
|
val childDescriptor = descriptorRegister.descriptorForClassUnsafe(child.javaClass)
|
||||||
|
childrenIds.add(childDescriptor.getId(child))
|
||||||
stack.add(child)
|
stack.add(child)
|
||||||
// If there is an active child then don't traverse it
|
// If there is an active child then don't traverse it
|
||||||
if (activeChild != null && activeChild != child) {
|
if (activeChild != null && activeChild != child) {
|
||||||
@@ -91,7 +94,7 @@ class PartialLayoutTraversal(
|
|||||||
val tags = descriptor.getTags(node)
|
val tags = descriptor.getTags(node)
|
||||||
visited.add(
|
visited.add(
|
||||||
Node(
|
Node(
|
||||||
node.nodeId(),
|
descriptor.getId(node),
|
||||||
descriptor.getQualifiedName(node),
|
descriptor.getQualifiedName(node),
|
||||||
descriptor.getName(node),
|
descriptor.getName(node),
|
||||||
attributes,
|
attributes,
|
||||||
|
|||||||
@@ -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)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user