From d954828bbcc950a4e57db1814d2adda1451343aa Mon Sep 17 00:00:00 2001 From: Lorenzo Blasa Date: Wed, 30 Nov 2022 07:23:29 -0800 Subject: [PATCH] Remove concept of dynamic metadata Summary: Although conceptually it made sense, it creates an issue with registration. ### Let me explain Effectively, as an engineer, what constitutes static and dynamic metadata and how can I ensure that is treated as such? It may be trivial to answer those questions but there was a fundamental assumption for static metadata that no longer holds true. Static metadata was registered when descriptors were first referenced, which most of the times happens when added to the descriptors registry. This used worked fine until we introduced the concept of deferred attributes. Deferred attributes, even though static, may register its metadata only when requested to. The issue is even more fundamental as not all the objects that declare static metadata will be loaded into memory by the time the plugin is connected and sends this. ### Solution To simplify, as the concept was never really used, there's only metadata. There's only pending metadata which is metadata that is yet to be sent to Flipper Desktop. Reviewed By: LukeDefeo Differential Revision: D41614186 fbshipit-source-id: 85d62eeff81ff22ae6e969d7b5aed8628b336258 --- .../props/ComponentDataExtractor.kt | 2 +- .../uidebugger/UIDebuggerFlipperPlugin.kt | 4 +- .../uidebugger/core/NativeScanScheduler.kt | 2 +- .../FragmentFrameworkDescriptor.kt | 3 +- .../descriptors/FragmentSupportDescriptor.kt | 2 +- .../descriptors/MetadataRegister.kt | 80 +++++++------------ .../descriptors/WindowDescriptor.kt | 3 +- .../observers/TreeObserverManager.kt | 2 +- 8 files changed, 37 insertions(+), 61 deletions(-) diff --git a/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/props/ComponentDataExtractor.kt b/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/props/ComponentDataExtractor.kt index 1920f3d58..8808b17ae 100644 --- a/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/props/ComponentDataExtractor.kt +++ b/android/plugins/litho/src/main/java/com/facebook/flipper/plugins/uidebugger/litho/descriptors/props/ComponentDataExtractor.kt @@ -107,7 +107,7 @@ object ComponentDataExtractor { val metadata = MetadataRegister.get(namespace, key) val identifier = metadata?.id - ?: MetadataRegister.registerDynamic( + ?: MetadataRegister.register( MetadataRegister.TYPE_ATTRIBUTE, namespace, key, mutable, possibleValues) return identifier } 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 b5bf9ae54..f725c80f7 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 @@ -59,7 +59,7 @@ class UIDebuggerFlipperPlugin( MetadataUpdateEvent.name, Json.encodeToString( MetadataUpdateEvent.serializer(), - MetadataUpdateEvent(MetadataRegister.staticMetadata()))) + MetadataUpdateEvent(MetadataRegister.getPendingMetadata()))) context.treeObserverManager.start() } @@ -69,7 +69,7 @@ class UIDebuggerFlipperPlugin( this.context.connectionRef.connection = null Log.i(LogTag, "Disconnected") - MetadataRegister.clear() + MetadataRegister.reset() context.treeObserverManager.stop() context.bitmapPool.recycleAll() 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 53eb74d07..6223aa511 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 @@ -54,7 +54,7 @@ class NativeScanScheduler(val context: Context) : Scheduler.Task { } private fun sendMetadata() { - val metadata = MetadataRegister.dynamicMetadata() + val metadata = MetadataRegister.getPendingMetadata() if (metadata.size > 0) { context.connectionRef.connection?.send( MetadataUpdateEvent.name, diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentFrameworkDescriptor.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentFrameworkDescriptor.kt index 88570b9f8..136317ef7 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentFrameworkDescriptor.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentFrameworkDescriptor.kt @@ -42,8 +42,7 @@ class FragmentFrameworkDescriptor(val register: DescriptorRegister) : for (key in args.keySet()) { val metadata = MetadataRegister.get(NAMESPACE, key) val identifier = - metadata?.id - ?: MetadataRegister.registerDynamic(MetadataRegister.TYPE_ATTRIBUTE, NAMESPACE, key) + metadata?.id ?: MetadataRegister.register(MetadataRegister.TYPE_ATTRIBUTE, NAMESPACE, key) when (val value = args[key]) { is Number -> props[identifier] = InspectableValue.Number(value) diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentSupportDescriptor.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentSupportDescriptor.kt index 593f51461..a45f9409e 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentSupportDescriptor.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/FragmentSupportDescriptor.kt @@ -42,7 +42,7 @@ class FragmentSupportDescriptor(val register: DescriptorRegister) : val metadata = MetadataRegister.get(NAMESPACE, key) val identifier = metadata?.id - ?: MetadataRegister.registerDynamic(MetadataRegister.TYPE_ATTRIBUTE, NAMESPACE, key) + ?: MetadataRegister.register(MetadataRegister.TYPE_ATTRIBUTE, NAMESPACE, key) when (val value = bundle[key]) { is Number -> props[identifier] = InspectableValue.Number(value) is Boolean -> props[identifier] = InspectableValue.Boolean(value) diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/MetadataRegister.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/MetadataRegister.kt index 5282d28c4..55a780b07 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/MetadataRegister.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/MetadataRegister.kt @@ -24,29 +24,10 @@ object MetadataRegister { const val TYPE_DOCUMENTATION = "documentation" private var generator: MetadataId = 0 - private val staticMetadata: MutableMap = mutableMapOf() - private val dynamicMetadata: MutableMap = mutableMapOf() - private val queried: MutableSet = mutableSetOf() + private val register: MutableMap = mutableMapOf() + private val pending: MutableSet = mutableSetOf() - fun key(namespace: String, name: String): String = "${namespace}_$name" - - private fun register( - metadata: MutableMap, - type: String, - namespace: String, - name: String, - mutable: Boolean, - possibleValues: Set? - ): MetadataId { - val key = key(namespace, name) - metadata[key]?.let { m -> - return m.id - } - - val identifier = ++generator - metadata[key] = Metadata(identifier, type, namespace, name, mutable, possibleValues) - return identifier - } + private fun key(namespace: String, name: String): String = "${namespace}_$name" fun register( type: String, @@ -55,46 +36,43 @@ object MetadataRegister { mutable: Boolean = false, possibleValues: Set? = emptySet() ): MetadataId { - return register(staticMetadata, type, namespace, name, mutable, possibleValues) - } + val key = key(namespace, name) + register[key]?.let { m -> + return m.id + } - fun registerDynamic( - type: String, - namespace: String, - name: String, - mutable: Boolean = false, - possibleValues: Set? = emptySet() - ): MetadataId { - return register(dynamicMetadata, type, namespace, name, mutable, possibleValues) + val identifier = ++generator + val metadata = Metadata(identifier, type, namespace, name, mutable, possibleValues) + + register[key] = metadata + pending.add(key) + + return identifier } fun get(namespace: String, name: String): Metadata? { val key = key(namespace, name) - staticMetadata[key]?.let { - return it - } - return dynamicMetadata[key] + return register[key] } - fun staticMetadata(): Map { + fun getMetadata(): Map { val metadata: MutableMap = mutableMapOf() - staticMetadata.forEach { entry -> metadata[entry.value.id] = entry.value } - return metadata - } - - fun dynamicMetadata(): Map { - val metadata: MutableMap = mutableMapOf() - dynamicMetadata.forEach { entry -> - if (!queried.contains(entry.value.id)) { - metadata[entry.value.id] = entry.value - queried.add(entry.value.id) - } - } + register.forEach { entry -> metadata[entry.value.id] = entry.value } return metadata } - fun clear() { - queried.clear() + fun getPendingMetadata(): Map { + val pendingMetadata: MutableMap = mutableMapOf() + pending.forEach { key -> + register[key]?.let { metadata -> pendingMetadata[metadata.id] = metadata } + } + + return pendingMetadata + } + + fun reset() { + pending.clear() + register.forEach { entry -> pending.add(entry.key) } } } diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/WindowDescriptor.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/WindowDescriptor.kt index 60913a292..91a942274 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/WindowDescriptor.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/descriptors/WindowDescriptor.kt @@ -79,8 +79,7 @@ object WindowDescriptor : ChainedDescriptor() { val metadata = MetadataRegister.get(NAMESPACE, name) val identifier = metadata?.id - ?: MetadataRegister.registerDynamic( - MetadataRegister.TYPE_ATTRIBUTE, NAMESPACE, name) + ?: MetadataRegister.register(MetadataRegister.TYPE_ATTRIBUTE, NAMESPACE, name) when (typedValue.type) { TypedValue.TYPE_STRING -> diff --git a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt index 7c5502e92..2c088011a 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt +++ b/android/src/main/java/com/facebook/flipper/plugins/uidebugger/observers/TreeObserverManager.kt @@ -90,7 +90,7 @@ class TreeObserverManager(val context: Context) { } private fun sendMetadata() { - val metadata = MetadataRegister.dynamicMetadata() + val metadata = MetadataRegister.getPendingMetadata() if (metadata.size > 0) { context.connectionRef.connection?.send( MetadataUpdateEvent.name,