Summary:
When playing around with issues in debug mode but without having a debugger attached, these logs are helpful.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27407772
fbshipit-source-id: 2083901094d3bd2b0b6c6799baffcf5a60cf57ae
Summary:
When reconstructing a second animation based on a conflict with a first, we want to ensure we set 'viewPrev' of the second animation properly.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27407777
fbshipit-source-id: 8fcc72c4c5fadaab4287824a944ad21230f2f97d
Summary:
In general, when an animation is interrupted or completed, we want to forcibly "flush" an update that will make the StubViewTree/mounting layer consistent with the ShadowTree.
However, in cases where a conflict creates a second animation and stops the first, we simply updated the "viewPrev" of the second animation and smoothly transition to it.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27407774
fbshipit-source-id: 928363867d8994c9d69c53154a07bda94f954afa
Summary:
Detect conflicts not just when a node is updated, but when its parent is DELETEd or CREATEd.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27407781
fbshipit-source-id: 719d9a8822bf691d9059073a30d8b5ccb50eece1
Summary:
This impacts Android only. It's not high-impact to check the consistency of these views and they can become "inconsistent" easily in a way that is not interesting or useful to us,
since we handle "virtual" views differently on the platform.
The need for these hacks should go away in the future if/when we account for virtual views in the differ, and stop sending redundant instructions entirely.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27407779
fbshipit-source-id: 01a7ea5106ed5d94b908de03a6a710cc6275018d
Summary:
pull out tag so it's easier to read
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27407770
fbshipit-source-id: e7d2a3ce8e4ac55b6446cddc8c39e73a18fa7170
Summary:
One of the struggles with the LA engine is to make sure that LA doesn't break the StubViewTree. In particular, we have strict requirements that the "oldShadowView" with every mutation matches exactly what is in the shadow tree, so it appears that there is a linear progression of ShadowNodes for every mutation instruction.
This is a struggle to do with the current setup, requires some wacky code, and doesn't work properly. Instead of spreading REMOVE/DELETE (especially) or INSERT/UPDATE instructions across multiple keyframes, we now allow keyframes to have multiple "final" instructions to execute,
which makes it much easier to keep state consistent.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27407769
fbshipit-source-id: 3a709503dc30be69efc345690cb1920eb9591e9a
Summary:
Problem: In paper, there is a handy API called `[uiManager viewForReactTag:]`. Fabric does not have this mapping. The Fabric interop layer still relies on this Paper mapping.
Solution: As a workaround, re-create this mapping in the Fabric interop layer. Therefore, whenever Fabric interop layer asks a paper view manager to create a view, store a weak reference to the view in a `NSMapTable`. NSMapTable allows us to customize the strong/weak relationship. I've added a comment explaining that `RCTWeakViewHolder` only needs to be used for this special circumstance.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27438899
fbshipit-source-id: 94663ef06479a8c863ce58b0f36d42109fa1c4f3
Summary:
Problem: `RCTLegacyViewManagerInteropCoordinator.mm` handles view commands by looking up `RCTModuleData` from the bridge, and then dispatching that method. In bridgeless mode, `RCTModuleData` doesn't exist.
Solution: Instead of relying on `RCTModuleData` (which does a ton of things), manually create a `_moduleMethods` array to store the methods that view manager exposes. This manual creation code was copied from how `RCTModuleData` performs lookup.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27377257
fbshipit-source-id: c3e820808e6aca03bae6486d5510156b39462215
Summary:
Fabric/Paper interop expects bridge to be contained in `contextContainer`, and crashes if it's not there.
This diff avoids a crash, and instead fails silently. Future diffs in stack will enable `RCTComponentData` and `RCTLegacyViewManagerInteropCoordinator` to function without the bridge.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D27374809
fbshipit-source-id: 2c9e03a1eaf5bcaa57887b203221111015cf4de5
Summary:
Changelog: [internal]
Add support for a return value when calling JavaScript functions in UIManagerBinding.
Reviewed By: mdvacca
Differential Revision: D27470817
fbshipit-source-id: 38de92dd913af61d879f1cc5962d417f51da73b0
Summary:
Changelog: [internal]
These ShadowNode subclasses should not be further subclassed. compiler can produce smaller and faster code.
Reviewed By: JoshuaGross
Differential Revision: D27463255
fbshipit-source-id: cb10cc61a3d80731476ac0c51af7f9a47e3f9ab7
Summary:
Just makes it easier to log and inspect these values.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27407778
fbshipit-source-id: 68e93d100b56bc9065f0ff1370260412d729312d
Summary:
changelog: [internal]
Fabric is missing implementation of PlatformColor APIs. If it is used, it causes a crash.
Until it is implemented, let's remove abort statement and continue with transparent color.
Reviewed By: mdvacca
Differential Revision: D27395917
fbshipit-source-id: 50d541c5cacc10a7652f7f1ddc97e086a4ba8f03
Summary:
We need to do this to break a dependency cycle that would happen if we try to have `view` depend on `mounting` just to add some telemetry to `view`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26827446
fbshipit-source-id: 4c415ebf5be3a02c18c80ea8a4a77068cae0f0fe
Summary:
This diff changes the syntax of class variables in MapBufferBuilder class to make it consistent with standards
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D27303495
fbshipit-source-id: b62067a07c5b2df1ae79b0134343d30ec819a55f
Summary:
This diff changes the syntax of class variables in MapBuffer class to make it consistent with standards
changelog: [internal] internal
Reviewed By: JoshuaGross, sammy-SC
Differential Revision: D27303497
fbshipit-source-id: 7581ce248559726c301ae96312fcb597a061ad5f
Summary:
This is a core part of the Timeline feature (aka Time Travel Debugger). With these new primitives, any external library can initiate "saving" all the previous interface changes (commits) and unwind to any previous one (in order to introspect and validate visual side-effects).
The next diff in the stack will implement UI for this feature integrated into Debug menu on iOS.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D25926660
fbshipit-source-id: 2e5f6892351d3053db8f64c1cf6ff445b0867ad7
Summary:
This diff contains the code from the 35 diff stack - D27210587
This diff implement and integrates Mapbuffer into Fabric text measure system
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D27241836
fbshipit-source-id: f40a780df0723f27da440f709a8676cfcca63953
Summary:
In my haste to get out D27238439 (https://github.com/facebook/react-native/commit/aca0f418baf4890b8be3c1214be70819e4b88a95), parts of it are sloppy. Nothing critical and no known bugs, but we should clean up the commented code, and add back these asserts.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27266288
fbshipit-source-id: f242c26401dfc8851cb1ee0ef8911d19d9c1d9ae
Summary:
In the previous diff, we moved the destruction of a ShadowTree object from `UIManager::stopSurface` method. And the next logical step is to align `UIManager::startSurface` method with the symmetrical approach. Now `stopSurface` accepts a unique pointer to a ShadowTree and `stopSurface` returns it back to the caller; this way we can avoid returning a raw pointer from `startSurface`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D27010533
fbshipit-source-id: b9211fb7e67763cc190d6b8c86cb866d20c6693d
Summary:
TextLayoutManager indirectly holds ShadowNodes, which could hold onto TextLayoutManager via a shared_ptr; so we probably have some reference cycles here.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27238438
fbshipit-source-id: 033101fa1b20d3a3b20ec4bae63f938493ed5147
Summary:
TextLayoutManager indirectly holds ShadowNodes, which could hold onto TextLayoutManager via a shared_ptr; so we probably have some reference cycles here.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27238439
fbshipit-source-id: b0b65cc451891e75bafddb7a08aa34ddf86d6a35
Summary:
Changelog: [internal]
Introducing LeakChecker. A tool that checks if all native components have been cleaned up when surface is stopped.
**Known shortcomings**:
- LeakChecker is only enabled in debug builds and the existence of leaks is logged to console.
- For now, Leak Checker looks at N-1 screen. This is intentional as there is a known limitation of React that doesn't free up all shadow nodes when surface is stopped. Because of this, the use of LeakChecker is not intuitive and I'll work with React team to try to work around this.
- It doesn't help locating the leak, it only informs that leak is present. I'll be looking into ways to help locate the leak.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D26727461
fbshipit-source-id: 8350190b99f24642f8e15a3c2e1d79cfaa810d3d
Summary:
The non-Fabric API has a `blockNativeResponder` param in setJSResponder. Make sure to pass that along in Fabric.
On Android this allows us to respect the flag and do the same thing non-Fabric was doing if `blockNativeResponder` is false. It's not clear yet what the impact is on iOS.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27058806
fbshipit-source-id: aa5074fa46191d78f5292a93d9040ab4bb58ca66
Summary:
On Android we have the notion of "virtual views", which are defined consistently but the logic is scattered and duplicated throughout the codebase.
The logic exists to mark nodes that exist in the ShadowTree, but not the View tree. We want to CREATE, UPDATE, and DELETE them on the platform, but not INSERT or REMOVE
them. They basically exist as EventEmitter objects.
The only issue with this is (1) duplicated code, which opened the possibility for inconsistent definition (2) StubViewTree did not account for virtualized views, which caused
assert crashes in debug mode for certain LayoutAnimations on Android.
By moving the definition to ShadowViewMutation and accounting for it in StubViewTree, asserts are correct and consistent on all platforms.
This was not caught until recently, because, until recently, no asserts actually ran on Android.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D27001199
fbshipit-source-id: eb29085317037ba8a286d7813bdd57095ad4746f
Summary:
Changelog: [internal]
Creates new function `RCTInstallNativeComponentRegistryBinding` which can be called during JS setup to register JS function `__nativeComponentRegistry__hasComponent`.
Note: This diff doesn't call `RCTInstallNativeComponentRegistryBinding`.
Reviewed By: shergin
Differential Revision: D26946176
fbshipit-source-id: 0625b8dd6090bc9e08baa38ba60b9cbe48268184
Summary:
Right now there are places in UIManagerBinding.cpp where native java exceptions will be thrown if calling JSI functions results in errors, such as:
```Trying to report error getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
Error: getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
```
https://www.internalfb.com/intern/logview/details/facebook_android_javascripterrors/358181062b47b9561e60427bbb3816a9
In this diff I added LOG(ERROR) and checks because:
1, Throwing errors neither prevents the JSI errors nor handles them properly, checks prevent JSI errors from happening.
2, Errors are aggregated in Logview with other JSI errors as "Error: android_crash:com.facebook.react.devsupport.JSException:facebook::jni::throwNewJavaException" which keeps the SLA task open forever, checks can prevent JSI errors so they won't lead to exceptions, and LOG(ERROR) will make sure we have enough info for debugging.
Changelog:
[General][Changed] - Add checks and logs to for better error handling
Reviewed By: JoshuaGross
Differential Revision: D26885562
fbshipit-source-id: c0c1c057342e9efc0ff708188703f4332036e7a9
Summary:
As part of stoping a Surface, we have to commit an empty shadow node tree. And to avoid a deadlock we have to do it not under a `linkMutex_`. To do so, we need to move the part that commits an empty tree from UIManager to SurfaceHandler. And this diff does exactly this.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D27010535
fbshipit-source-id: 60cc79b1c81d7340b550e78653737d2cc5bec24d
Summary:
When EventTarget is created in createNode, it's currently treated as optional in native, but it's not optional in JS. We should assert that it's non-null to make the contract more clear.
For now, we keep it as an assert so we catch issues in debug/dev, but it won't impact prod.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27002506
fbshipit-source-id: c541dc02d3de22df19292deaef1a3b68fe57ba1f
Summary:
Now cumulative time spent on text measuring is available as as part of SurfaceTelemetry.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26827445
fbshipit-source-id: 8ad3178557ea71170557fcf19b6f62f7072d6da2
Summary:
Now we not only measure how many times we measured text but also measure how much time it takes. This way we can see which portion of the layout process is spent by layout itself (and measuring embedded components).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26827447
fbshipit-source-id: e0b09fcacc86aed50dd94b48458215adbb0a60ef
Summary:
Changelog: [internal]
Extract componentNameByReactViewName to separate file so it can be used elsewhere.
Reviewed By: shergin, mdvacca
Differential Revision: D26946159
fbshipit-source-id: cf69df1f80f1c1938fc667f4666a5d3fec5a9658
Summary:
Fix warnings about implicit type truncation.
## Changelog
[Internal] [Fixed] - Fix various C++ warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/31002
Test Plan:
Almost all the changes here are simply making explicit conversions which are already occurring. With the exception of a couple of constants being changed from doubles to floats.
With these changes I am able to remove a bunch of warning suppressions in react-native-windows.
Reviewed By: shergin
Differential Revision: D26900502
Pulled By: rozele
fbshipit-source-id: d5e415282815c2212a840a863713287bbf118c10
Summary:
Removing unsafe cast from `int` to `uint16_t`.
Also, adding code to detect multiplication overflow during buffer resize.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[General] [Fix] - Fix unsafe cast and detect overflow in MapBuffer.
Pull Request resolved: https://github.com/facebook/react-native/pull/31106
Test Plan: Code compiles in Visual Studio 2019 without the unsafe cast warning (or error depending on the configuration).
Reviewed By: mdvacca
Differential Revision: D26865138
Pulled By: rozele
fbshipit-source-id: 4692a38b05fc873e31fbbe94d70803244e82de5d
Summary:
Changelog: [Internal]
Updates `ReactSurface` to use `SurfaceHandler` internally.
This removes most of the internal state in `ReactSurface` and propagates all the calls to the `SurfaceHandler`.
`FabricUIManager` now uses `SurfaceHandler` to start/stop the surface.
SurfaceId is still used for view operations. SurfaceId is also now mutable to play better with existing Android infra.
Reviewed By: shergin, mdvacca
Differential Revision: D26112992
fbshipit-source-id: 52e6860084d739381317035dc3011956d452063c
Summary:
Changelog: [internal]
The assumption was wrong. There can be more than one view on iOS in ScrollView. The other view is pull the refresh component.
Reviewed By: mdvacca
Differential Revision: D26880403
fbshipit-source-id: 69fb1668b5ee7d5f4a4ab3c89222587689c10591
Summary:
Changelog: [internal]
UIManager shouldn't be retained in lambda because if it outlives runtime it causes a crash.
UIManager -> ComponentDescriptorRegistry -> ParagraphComponentDescriptor -> TextLayoutManager's cache -> AttributedString::Fragment -> ShaviewView -> EventEmitter -> EventTarget -> Pointer
This is the chain of ownership that can cause a crash if UIManager is retained for longer than JS runtime.
{F459238235}
Reviewed By: JoshuaGross
Differential Revision: D26851194
fbshipit-source-id: 644cdee34ba9286618659d847fb2e78bc301d049
