From 17ccdeaf6fe6a4a06ce05a11ac71dd44260c4a82 Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Thu, 23 Apr 2020 04:29:24 -0700 Subject: [PATCH] Change the export logic for the Inspector Summary: This enables the feature which showed the theme information in the layout plugin. It was disabled due to the OOM which occurred while exporting flipper trace. The OOM happened when we tried to serialise the whole layout hierarchy and the amount of info added per node by the theme info was quite heavy. Thus removing it solved the OOM problem at that point, but its not the correct solution. The problem is that each node has too much information and sending it at one stretch is very heavy and causes OOM. So instead of sending it at one stretch, I have broken it into multiple calls at each level of the tree. This no longer causes OOM and we will be able to show theme information too. Also for iOS we don't have AXNodes call or AXRoot feature implemented, so anyway I had to put a check to not make those calls, so instead I have kept the feature of fetching all nodes on the iOS instead, as there has been no problem on the iOS side with regards to the OOM. But I am indifferent, the same logic will work for iOS too, it might increase the time to export. issue discussed [here](https://fb.workplace.com/groups/flippersupport/permalink/854729375007722/) Reviewed By: jknoxville Differential Revision: D21136057 fbshipit-source-id: becd237a6d53c50af082597f2e8ed790c25cb966 --- .../utils/ContextDescriptorUtils.java | 29 +++---- desktop/plugins/layout/index.tsx | 82 +++++++++++++++++-- 2 files changed, 88 insertions(+), 23 deletions(-) diff --git a/android/src/main/java/com/facebook/flipper/plugins/inspector/descriptors/utils/ContextDescriptorUtils.java b/android/src/main/java/com/facebook/flipper/plugins/inspector/descriptors/utils/ContextDescriptorUtils.java index f2b036d17..3ca3601fb 100644 --- a/android/src/main/java/com/facebook/flipper/plugins/inspector/descriptors/utils/ContextDescriptorUtils.java +++ b/android/src/main/java/com/facebook/flipper/plugins/inspector/descriptors/utils/ContextDescriptorUtils.java @@ -51,26 +51,23 @@ public final class ContextDescriptorUtils { || sThemeImplThemeKeyField == null || sThemeKeyResIdField == null || sThemeImplAssetManagerField == null) { + sThemeImplField = theme.getClass().getDeclaredField("mThemeImpl"); + sThemeImplField.setAccessible(true); - // Early exiting as this bit of code causes too much of the metadata to be created and - // ultimately leads to out of memory exception. Reference D20441839 - // sThemeImplField = theme.getClass().getDeclaredField("mThemeImpl"); - // sThemeImplField.setAccessible(true); + themeImpl = sThemeImplField.get(theme); + sThemeImplThemeKeyField = themeImpl.getClass().getDeclaredField("mKey"); + sThemeImplThemeKeyField.setAccessible(true); - // themeImpl = sThemeImplField.get(theme); - // sThemeImplThemeKeyField = themeImpl.getClass().getDeclaredField("mKey"); - // sThemeImplThemeKeyField.setAccessible(true); + sThemeImplAssetManagerField = themeImpl.getClass().getDeclaredField("mAssets"); + sThemeImplAssetManagerField.setAccessible(true); - // sThemeImplAssetManagerField = themeImpl.getClass().getDeclaredField("mAssets"); - // sThemeImplAssetManagerField.setAccessible(true); + sAssetManagerGetStyleAttributesMethod = + assetManager.getClass().getDeclaredMethod("getStyleAttributes", int.class); + sAssetManagerGetStyleAttributesMethod.setAccessible(true); - // sAssetManagerGetStyleAttributesMethod = - // assetManager.getClass().getDeclaredMethod("getStyleAttributes", int.class); - // sAssetManagerGetStyleAttributesMethod.setAccessible(true); - - // themeKey = sThemeImplThemeKeyField.get(themeImpl); - // sThemeKeyResIdField = themeKey.getClass().getDeclaredField("mResId"); - // sThemeKeyResIdField.setAccessible(true); + themeKey = sThemeImplThemeKeyField.get(themeImpl); + sThemeKeyResIdField = themeKey.getClass().getDeclaredField("mResId"); + sThemeKeyResIdField.setAccessible(true); return builderMap; diff --git a/desktop/plugins/layout/index.tsx b/desktop/plugins/layout/index.tsx index 2cc108886..b737957c2 100644 --- a/desktop/plugins/layout/index.tsx +++ b/desktop/plugins/layout/index.tsx @@ -77,6 +77,9 @@ const FlipperADButton = styled(Button)({ margin: 10, }); +type ClientGetNodesCalls = 'getNodes' | 'getAXNodes'; +type ClientMethodCalls = 'getRoot' | 'getAXRoot' | ClientGetNodesCalls; + export default class Layout extends FlipperPlugin { FlipperADBar() { return ( @@ -98,19 +101,84 @@ export default class Layout extends FlipperPlugin { } static exportPersistedState = async ( - callClient: ( - method: 'getAllNodes', - ) => Promise<{ - allNodes: PersistedState; - }>, + callClient: (method: ClientMethodCalls, params?: any) => Promise, persistedState: PersistedState | undefined, store: ReduxState | undefined, + _idler?: Idler | undefined, + statusUpdate?: (msg: string) => void, + supportsMethod?: (method: ClientMethodCalls) => Promise, ): Promise => { if (!store) { return persistedState; } - const {allNodes} = await callClient('getAllNodes'); - return allNodes; + statusUpdate && statusUpdate('Fetching Root Node...'); + // We need not check the if the client supports `getRoot` as if it should and if it doesn't we will get a suppressed notification in Flipper and things will still export, but we will get an error surfaced. + const rootElement: Element | null = await callClient('getRoot'); + const rootAXElement: Element | null = + supportsMethod && (await supportsMethod('getAXRoot')) // getAXRoot only relevant for Android + ? await callClient('getAXRoot') + : null; + const elements: ElementMap = {}; + + if (rootElement) { + statusUpdate && statusUpdate('Fetching Child Nodes...'); + await Layout.getAllNodes( + rootElement, + elements, + callClient, + 'getNodes', + supportsMethod, + ); + } + const AXelements: ElementMap = {}; + if (rootAXElement) { + statusUpdate && statusUpdate('Fetching Child AX Nodes...'); + await Layout.getAllNodes( + rootAXElement, + AXelements, + callClient, + 'getAXNodes', + supportsMethod, + ); + } + statusUpdate && statusUpdate('Finished Fetching Child Nodes...'); + return { + rootElement: rootElement != undefined ? rootElement.id : null, + rootAXElement: rootAXElement != undefined ? rootAXElement.id : null, + elements, + AXelements, + }; + }; + + static getAllNodes = async ( + root: Element, + nodeMap: ElementMap, + callClient: (method: ClientGetNodesCalls, params?: any) => Promise, + method: ClientGetNodesCalls, + supportsMethod?: (method: ClientGetNodesCalls) => Promise, + ): Promise => { + nodeMap[root.id] = root; + if ( + root.children.length > 0 && + supportsMethod && + (await supportsMethod(method)) + ) { + await callClient(method, {ids: root.children}).then( + async ({elements}: {elements: Array}) => { + await Promise.all( + elements.map(async (elem) => { + await Layout.getAllNodes( + elem, + nodeMap, + callClient, + method, + supportsMethod, + ); + }), + ); + }, + ); + } }; static serializePersistedState: (