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:
Fixing `warning C4244: 'argument': conversion from 'double' to 'float', possible loss of data`
Changelog: [Internal]
Reviewed By: SidharthGuglani
Differential Revision: D27132355
fbshipit-source-id: 55ff35be368ef4f6093865eb88c17e753250d179
Summary:
In the Chrome inspector, add support for `HeapProfiler.startSampling`
and `HeapProfiler.stopSampling`. These two protocols turn the sampling
heap profiler on and off, and write the results on the socket.
Then Chrome's visualizer pieces the information together to produce a flame graph
of memory usage.
Added a unit test to make sure sampling can be turned on and off, and there are some
samples returned.
Changelog: [Internal]
Reviewed By: avp
Differential Revision: D26835148
fbshipit-source-id: d1be3cee791e42da5d9e117c3b8259b6622b98f4
Summary:
The sampling heap profiler will need to be enable-able by the
inspector, and the previous snapshot/profiling APIs are all
defined on `jsi::instrumentation`.
Follow their lead and define the new sampling heap profiler APIs on
`jsi::instrumentation` as well, and implement it for Hermes.
Changelog: [Internal]
Reviewed By: neildhar
Differential Revision: D26835152
fbshipit-source-id: 4625be3214297489e04a4ceea8a22622d14299c4
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:
These methods are only ever called just after setting the various YGStyle props. Therefore we can read any padding / margin / border state from there rather than recalculating it.
Changelog: [Internal]
Reviewed By: SidharthGuglani
Differential Revision: D27010098
fbshipit-source-id: a33f879b25c54cfdb0ffc724b6aa325858e97df5
Summary:
Avoid recalculating margin and padding by pre-subtracting the margin from `availableWidth` and inlining the calculation of `paddingAndBorderAxisCross`.
Changelog: [Internal]
Reviewed By: SidharthGuglani
Differential Revision: D27010094
fbshipit-source-id: afc3cf251a0306b9e5d7f0dc6856feee8d1dca6e
Summary:
Noticed in simpleperf this was a very hot method, showing 8ms spent in these methods in our sample application. By splitting the method out in a horizontal and vertical variant we can simplify cases enormously and check for begin/end in one go.
Changelog: [Internal]
Reviewed By: SidharthGuglani
Differential Revision: D27010008
fbshipit-source-id: 22fed58c7476e1d716b0191b55997c7a06e63223
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:
The documentation of jsi::Value::strictEquals was confusing it with SameValue (which differs in treatment of signed zeroes and NaN). Fix it by referring to the correct part of the spec.
Changelog:
[Internal][Fixed] - Fix incorrect comment
Reviewed By: avp
Differential Revision: D26430216
fbshipit-source-id: 0ea4208fbdda40c87f8028231ddefb2323b6eb96
Summary:
Pull Request resolved: https://github.com/facebook/yoga/pull/1068
There is an issue in react-native when the Yoga node position type is set to absolute and display: none is set where the node layout calculation gives the absolute dimensions, rather than the expected 0 x 0.
Here are some OSS issues tracking this:
https://github.com/facebook/react-native/issues/18415https://github.com/microsoft/react-native-windows/issues/7289
## Changelog
[General] [Fix] - Fixes layout of nodes with YGDisplayNone and YGPositionTypeAbsolute
Reviewed By: Andrey-Mishanin
Differential Revision: D26849307
fbshipit-source-id: 197618aa3c4e1b3b7efeba7ea4efd30b2d1c982d
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:
Changelog:
These are no longer being used by Buck, we can just remove them.
Reviewed By: zigwei
Differential Revision: D26913017
fbshipit-source-id: c75a07b2dc8c337ceef6da275b0046c0ac048ba7
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
Summary:
Changelog: [internal]
StateWrapperImpl shouldn't retain State strongly because cleanup of `StateWrapperImpl` is triggered from Java and isn't guaranteed to take happen before runtime is destroyed.
This should resolve crash where `StateWrapperImpl`'s destruction causes a `~Pointer` to be called after runtime is destroyed.
Chain of ownership that will be broken by storing State weakly inside `StateWrapperImpl`.
`StateWrapperImpl -> ParagraphState -> TextLayourManager's cache -> AttributedString -> ShadowView -> EventEmitter -> EventTarget -> Pointer`
{F451105831}
Reviewed By: JoshuaGross
Differential Revision: D26815275
fbshipit-source-id: 0703c6dccc62c1d152923b786a83273fa8a03694
Summary:
std::thread's constructor is nodiscard. This breaks in MSVC 16.9 when nodiscard starts to be enforced. Either we should hold on to the created object or detach the temporary which is what I think this function intends to do anyway.
Fixes https://github.com/facebook/react-native/issues/31088
Fixes an invalid usage of std::thread's constructor
## 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] [Fixed] - fixes usage of std::thread in runtime executor
Pull Request resolved: https://github.com/facebook/react-native/pull/31090
Reviewed By: sammy-SC
Differential Revision: D26783963
Pulled By: appden
fbshipit-source-id: fed4d072792aafa058dd742e8fce30a207f991c1
Summary:
This diff extracts ComponentNameRegistry out of Fabric modules
This is necessary to avoid depending on Fabric and regressing APK size for other RN apps (e.g. IG)
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26765328
fbshipit-source-id: 0a22c4279146f5243473c74a84e78fad7f08f956
Summary:
Introduce the NativeComponentRegistryBinding class, the purpose of this class is to register the global function __nativeComponentRegistry__hasComponent into JS
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D26716902
fbshipit-source-id: d883b195b30600d3781d41158fd8c57cf22431df
Summary:
In T85279528, we're trying to add a RCTURLRequestHandler to the NativeModule system, when the NativeModule system is in an invalid state. This causes a crash. Longer term, this crash will go away when we delete the legacy NativeModule system. However, in the short term:
1. The parent of this diff (i.e: D26741053) ensures that all RCTURLRequestHandlers are TurboModule-compatible. This makes the modules std::vector passed into ModuleRegistry::resigerModules empty.
2. This diff makes ModuleRegistry::registerModules() noop when the modules std::vector is empty.
In tandem, these two diffs should mitigate this crash, by making sure we don't execute the code that crashes.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26741417
fbshipit-source-id: fc4a09f6adcbdd6dbe197c9aa6a55af077bd818b
Summary:
Checking if the moduleInfo is null, before calling moduleInfo.asObject isn't sufficient. Before calling moduleInfo.asObject, we should first check if the moduleInfo is an Object.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D26735367
fbshipit-source-id: b726216857bf23a3c9bab14414bf45b10900e033
Summary:
When we call JSIExecutor::nativeCallSyncHook, we assume that the third argument is an object and call Value::asObject on it, before checking if the Object is an Array. Calling Value::asObject throws an error if the Value isn't an Object.
This diff includes an isObject check on the third argument.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D26735262
fbshipit-source-id: 96eb43d6c8bc1d78f3b5e0dc24ed6d419a446ecf
Samuel Susla
David Vacca
Valentin Shergin
Joshua Gross
Altaibayar Tseveenbayar
Riley Dulin
Pieter De Baets
Lulu Wu
Daniel Andersson
Eric Rozell
Andrew Coates
Pranjal Shankhdhar
Igor Klemenski
Andrei Shikov
Alexander Sklar
Ramanpreet Nara