From a888f0c0a1f2f6998676855d7fcabb22068ad639 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 8 Sep 2022 06:10:50 -0700 Subject: [PATCH] Fix broken systrace marker in getViewManagerNames Summary: Noticed this marker was not properly ended in the case where an early return happens. The fact that that early return is happening there is problematic too, so added a warning to follow-up on. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D39271917 fbshipit-source-id: 86dd5b1a46978629584f7cb0d615f2ad99d5a2f8 --- .../facebook/react/ReactInstanceManager.java | 58 ++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index 0fe6100cacc89d..6e7a9af39bd1c6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -961,39 +961,43 @@ public List getOrCreateViewManagers( public Collection getViewManagerNames() { Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstanceManager.getViewManagerNames"); - Collection viewManagerNames = mViewManagerNames; - if (viewManagerNames != null) { - return viewManagerNames; - } - ReactApplicationContext context; - synchronized (mReactContextLock) { - context = (ReactApplicationContext) getCurrentReactContext(); - if (context == null || !context.hasActiveReactInstance()) { - return Collections.emptyList(); + try { + Collection viewManagerNames = mViewManagerNames; + if (viewManagerNames != null) { + return viewManagerNames; + } + ReactApplicationContext context; + synchronized (mReactContextLock) { + context = (ReactApplicationContext) getCurrentReactContext(); + if (context == null || !context.hasActiveReactInstance()) { + FLog.w(ReactConstants.TAG, "Calling getViewManagerNames without active context"); + return Collections.emptyList(); + } } - } - synchronized (mPackages) { - if (mViewManagerNames == null) { - Set uniqueNames = new HashSet<>(); - for (ReactPackage reactPackage : mPackages) { - SystraceMessage.beginSection( - TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstanceManager.getViewManagerName") - .arg("Package", reactPackage.getClass().getSimpleName()) - .flush(); - if (reactPackage instanceof ViewManagerOnDemandReactPackage) { - Collection names = - ((ViewManagerOnDemandReactPackage) reactPackage).getViewManagerNames(context); - if (names != null) { - uniqueNames.addAll(names); + synchronized (mPackages) { + if (mViewManagerNames == null) { + Set uniqueNames = new HashSet<>(); + for (ReactPackage reactPackage : mPackages) { + SystraceMessage.beginSection( + TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstanceManager.getViewManagerName") + .arg("Package", reactPackage.getClass().getSimpleName()) + .flush(); + if (reactPackage instanceof ViewManagerOnDemandReactPackage) { + Collection names = + ((ViewManagerOnDemandReactPackage) reactPackage).getViewManagerNames(context); + if (names != null) { + uniqueNames.addAll(names); + } } + Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); } - SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush(); + mViewManagerNames = uniqueNames; } - Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); - mViewManagerNames = uniqueNames; + return mViewManagerNames; } - return mViewManagerNames; + } finally { + Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE); } }