Skip to content

Commit

Permalink
Refactor MountingManager into MountingManager + SurfaceMountingManager
Browse files Browse the repository at this point in the history
Summary:
This refactors MountingManager into a minimal API that shims into a more fully-featured SurfaceMountingManager. The SurfaceMountingManager keeps track of surface start/stop, surface ID, and surface Context.

This solves a number of issues around (1) race conditions around StopSurface/StartSurface, (2) memory management of Views, (3)

Concrete improvements:

1. Simpler to reason about race conditions around StopSurface/StartSurface.
2. 1:1 relationship between SurfaceId and all Views/tags.
3. When surface is stopped, all descendent Views can be GC'd immediately.
4. Fixed separation of concerns and leaky abstractions: surfaceId/rootTag and Surface Context are now stored and manipulated *only* in SurfaceMountingManager.
5. Simpler StopSurface flow: we simply remove references to all Views, and the Fragment (outside of the scope of this code) removes the RootView. This will trigger GC and we do ~0 work. Previously, we ran a REMOVE and DELETE instruction and kept track of each View in a HashMap. Now we can simply delete the map and move on.

The caveat: NativeAnimated (or other native modules that go through UIManager). APIs like `updateProps` currently uses only the ReactTag and does not store SurfaceId. This is a good argument for moving away from ReactTag, at least in its current incarnation, but: for now this requires that you do a lookup of a ReactTag across N surfaces (worst-case) to determine which Surface a ReactTag is in.

So, to summarize, the "con" of this approach is that now `getSurfaceManagerForViewEnforced` could be slower. It is used in:
* NativeAnimatedModule calls `updateProps` through UIManager
* FabricEventEmitter calls `receiveEvent` on FabricUIManager directly
* On audit, I could find zero native callsites to `sendAccessibilityEvent` through UIManager

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D26000781

fbshipit-source-id: 386ae40c4333f8c584e05818c404868dbee6ce73
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Jan 22, 2021
1 parent 82e9cb1 commit 29eb632
Show file tree
Hide file tree
Showing 11 changed files with 1,186 additions and 871 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ void updateRootLayoutSpecs(
* layout-related propertied won't be handled properly. Make sure you know what you're doing
* before calling this method :)
*
* @param tag {@link int} that identifies the view that will be updated
* @param reactTag {@link int} that identifies the view that will be updated
* @param props {@link ReadableMap} props that should be immediately updated in view
*/
@UiThread
Expand Down
Loading

0 comments on commit 29eb632

Please sign in to comment.