From d97350dcbcb3cdbc9eca743f830e943af0e02c2a Mon Sep 17 00:00:00 2001 From: Andrei Shikov Date: Tue, 6 Apr 2021 11:18:02 -0700 Subject: [PATCH] Queue mount items executed before root view is attached Summary: Changelog: [Internal] After D27016919, some mounting instructions can be executed before the view is attached, which is invalid. This change adds additional queue for such items, which can be later dispatched after view is ready. The new queue is expected to be empty for usual rendering and used in prerendering flows only. In case of prerendering, it should only hold intermediate items between early start of `SurfaceHandler` and view attach. Reviewed By: mdvacca Differential Revision: D27291706 fbshipit-source-id: f383c1d0d7050f271993553b51bf2e387efe1e9e --- .../fabric/mounting/MountItemDispatcher.java | 18 +++++-- .../fabric/mounting/MountingManager.java | 49 ++++++++++--------- .../mounting/SurfaceMountingManager.java | 35 ++++++++++++- 3 files changed, 74 insertions(+), 28 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java index 68ec64ac5cc..5aa1c5788fd 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java @@ -183,7 +183,7 @@ public class MountItemDispatcher { printMountItem(command, "dispatchMountItems: Executing viewCommandMountItem"); } try { - command.execute(mMountingManager); + executeOrEnqueue(command); } catch (RetryableMountingLayerException e) { // If the exception is marked as Retryable, we retry the viewcommand exactly once, after // the current batch of mount items has finished executing. @@ -225,7 +225,7 @@ public class MountItemDispatcher { + preMountItemsToDispatch.size()); for (PreAllocateViewMountItem preMountItem : preMountItemsToDispatch) { - preMountItem.execute(mMountingManager); + executeOrEnqueue(preMountItem); } Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); @@ -244,7 +244,7 @@ public class MountItemDispatcher { } try { - mountItem.execute(mMountingManager); + executeOrEnqueue(mountItem); } catch (Throwable e) { // If there's an exception, we want to log diagnostics in prod and rethrow. FLog.e(TAG, "dispatchMountItems: caught exception, displaying all MountItems", e); @@ -288,7 +288,7 @@ public class MountItemDispatcher { break; } - preMountItemToDispatch.execute(mMountingManager); + executeOrEnqueue(preMountItemToDispatch); } } finally { mInDispatch = false; @@ -297,6 +297,16 @@ public class MountItemDispatcher { Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } + private void executeOrEnqueue(MountItem item) { + if (mMountingManager.isWaitingForViewAttach(item.getSurfaceId())) { + SurfaceMountingManager surfaceMountingManager = + mMountingManager.getSurfaceManager(item.getSurfaceId()); + surfaceMountingManager.executeOnViewAttach(item); + } else { + item.execute(mMountingManager); + } + } + @Nullable private static List drainConcurrentItemQueue( ConcurrentLinkedQueue queue) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java index 197f592129f..5313e1a9154 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountingManager.java @@ -26,7 +26,6 @@ import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.mapbuffer.ReadableMapBuffer; import com.facebook.react.fabric.FabricUIManager; import com.facebook.react.fabric.events.EventEmitterWrapper; -import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.touch.JSResponderHandler; import com.facebook.react.uimanager.RootViewManager; import com.facebook.react.uimanager.ThemedReactContext; @@ -40,7 +39,7 @@ import java.util.concurrent.CopyOnWriteArrayList; /** * Class responsible for actually dispatching view updates enqueued via {@link - * FabricUIManager#scheduleMountItems(int, MountItem[])} on the UI thread. + * FabricUIManager#scheduleMountItem} on the UI thread. */ public class MountingManager { public static final String TAG = MountingManager.class.getSimpleName(); @@ -53,14 +52,12 @@ public class MountingManager { private final CopyOnWriteArrayList mStoppedSurfaceIds = new CopyOnWriteArrayList<>(); @Nullable private SurfaceMountingManager mMostRecentSurfaceMountingManager; + @Nullable private SurfaceMountingManager mLastQueriedSurfaceMountingManager; @NonNull private final JSResponderHandler mJSResponderHandler = new JSResponderHandler(); @NonNull private final ViewManagerRegistry mViewManagerRegistry; @NonNull private final RootViewManager mRootViewManager = new RootViewManager(); - private volatile int mStoppedSurfaceCacheLastId = View.NO_ID; - private volatile boolean mStoppedSurfaceCacheLastResult = false; - public MountingManager(@NonNull ViewManagerRegistry viewManagerRegistry) { mViewManagerRegistry = viewManagerRegistry; } @@ -81,7 +78,7 @@ public class MountingManager { public SurfaceMountingManager startSurface(final int surfaceId) { SurfaceMountingManager surfaceMountingManager = new SurfaceMountingManager( - surfaceId, mJSResponderHandler, mViewManagerRegistry, mRootViewManager); + surfaceId, mJSResponderHandler, mViewManagerRegistry, mRootViewManager, this); // There could technically be a race condition here if addRootView is called twice from // different threads, though this is (probably) extremely unlikely, and likely an error. @@ -117,8 +114,6 @@ public class MountingManager { @AnyThread public void stopSurface(final int surfaceId) { - mStoppedSurfaceCacheLastId = View.NO_ID; - SurfaceMountingManager surfaceMountingManager = mSurfaceIdToManager.get(surfaceId); if (surfaceMountingManager != null) { // Maximum number of stopped surfaces to keep track of @@ -145,6 +140,15 @@ public class MountingManager { @Nullable public SurfaceMountingManager getSurfaceManager(int surfaceId) { + if (mLastQueriedSurfaceMountingManager != null + && mLastQueriedSurfaceMountingManager.getSurfaceId() == surfaceId) { + return mLastQueriedSurfaceMountingManager; + } + + if (mMostRecentSurfaceMountingManager != null + && mMostRecentSurfaceMountingManager.getSurfaceId() == surfaceId) { + return mMostRecentSurfaceMountingManager; + } return mSurfaceIdToManager.get(surfaceId); } @@ -164,25 +168,11 @@ public class MountingManager { } public boolean surfaceIsStopped(int surfaceId) { - if (surfaceId == View.NO_ID) { - return false; - } - if (surfaceId == mStoppedSurfaceCacheLastId) { - return mStoppedSurfaceCacheLastResult; - } - - boolean res = surfaceIsStoppedImpl(surfaceId); - mStoppedSurfaceCacheLastResult = res; - mStoppedSurfaceCacheLastId = surfaceId; - return res; - } - - private boolean surfaceIsStoppedImpl(int surfaceId) { if (mStoppedSurfaceIds.contains(surfaceId)) { return true; } - SurfaceMountingManager surfaceMountingManager = mSurfaceIdToManager.get(surfaceId); + SurfaceMountingManager surfaceMountingManager = getSurfaceManager(surfaceId); if (surfaceMountingManager != null && surfaceMountingManager.isStopped()) { return true; } @@ -190,6 +180,19 @@ public class MountingManager { return false; } + public boolean isWaitingForViewAttach(int surfaceId) { + SurfaceMountingManager mountingManager = getSurfaceManager(surfaceId); + if (mountingManager == null) { + return false; + } + + if (mountingManager.isStopped()) { + return false; + } + + return !mountingManager.isRootViewAttached(); + } + /** * Get SurfaceMountingManager associated with a ReactTag. Unfortunately, this requires lookups * over N maps, where N is the number of active or recently-stopped Surfaces. Each lookup will diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java index d5a678ce59d..6002cff9ef6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.java @@ -26,6 +26,7 @@ import com.facebook.react.bridge.SoftAssertions; import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.common.build.ReactBuildConfig; import com.facebook.react.fabric.events.EventEmitterWrapper; +import com.facebook.react.fabric.mounting.mountitems.MountItem; import com.facebook.react.touch.JSResponderHandler; import com.facebook.react.uimanager.IllegalViewOperationException; import com.facebook.react.uimanager.ReactRoot; @@ -39,6 +40,7 @@ import com.facebook.react.uimanager.ViewManager; import com.facebook.react.uimanager.ViewManagerRegistry; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentLinkedQueue; import javax.annotation.Nullable; public class SurfaceMountingManager { @@ -47,15 +49,18 @@ public class SurfaceMountingManager { private static final boolean SHOW_CHANGED_VIEW_HIERARCHIES = ReactBuildConfig.DEBUG && false; private volatile boolean mIsStopped = false; + private volatile boolean mRootViewAttached = false; @Nullable private ThemedReactContext mThemedReactContext; // These are all non-null, until StopSurface is called private ConcurrentHashMap mTagToViewState = new ConcurrentHashMap<>(); // any thread + private ConcurrentLinkedQueue mOnViewAttachItems = new ConcurrentLinkedQueue<>(); private JSResponderHandler mJSResponderHandler; private ViewManagerRegistry mViewManagerRegistry; private RootViewManager mRootViewManager; + private MountingManager mMountingManager; // This is null *until* StopSurface is called. private Set mTagSetForStoppedSurface; @@ -67,12 +72,14 @@ public class SurfaceMountingManager { int surfaceId, @NonNull JSResponderHandler jsResponderHandler, @NonNull ViewManagerRegistry viewManagerRegistry, - @NonNull RootViewManager rootViewManager) { + @NonNull RootViewManager rootViewManager, + @NonNull MountingManager mountingManager) { mSurfaceId = surfaceId; mJSResponderHandler = jsResponderHandler; mViewManagerRegistry = viewManagerRegistry; mRootViewManager = rootViewManager; + mMountingManager = mountingManager; } public boolean isStopped() { @@ -84,6 +91,14 @@ public class SurfaceMountingManager { addRootView(rootView); } + public int getSurfaceId() { + return mSurfaceId; + } + + public boolean isRootViewAttached() { + return mRootViewAttached; + } + @Nullable public ThemedReactContext getContext() { return mThemedReactContext; @@ -133,6 +148,11 @@ public class SurfaceMountingManager { return mTagToViewState.containsKey(tag); } + @AnyThread + public void executeOnViewAttach(MountItem item) { + mOnViewAttachItems.add(item); + } + @AnyThread private void addRootView(@NonNull final View rootView) { if (isStopped()) { @@ -174,6 +194,9 @@ public class SurfaceMountingManager { if (rootView instanceof ReactRoot) { ((ReactRoot) rootView).setRootViewTag(mSurfaceId); } + mRootViewAttached = true; + + executeViewAttachMountItems(); } }; @@ -184,6 +207,14 @@ public class SurfaceMountingManager { } } + @UiThread + private void executeViewAttachMountItems() { + while (!mOnViewAttachItems.isEmpty()) { + MountItem item = mOnViewAttachItems.poll(); + item.execute(mMountingManager); + } + } + /** * Stop surface and all operations within it. Garbage-collect Views (caller is responsible for * removing RootView from View layer). @@ -232,6 +263,8 @@ public class SurfaceMountingManager { mTagToViewState = null; mJSResponderHandler = null; mRootViewManager = null; + mMountingManager = null; + mOnViewAttachItems.clear(); } };