Summary:
See comments for additional commentary. On Android we pass State to all views upon creation; currently we do Preallocation of views, which means that Views are created on the mounting layer before they're technically committed. Previously we were only assocation State -> ShadowNode upon tree commit.
This meant that during Preallocation, State was accessible but the `updateState` mechanism did not work because that requires a pointer from the State back to its owning ShadowNode.
We explicitly make that connection now when creating the ShadowNode and State. This should fix or prevent many subtle bugs with State, on Android but potentially other platforms as well, since now we guarantee that State is always updateable.
Changelog:
[[Internal]]
Reviewed By: shergin, mdvacca
Differential Revision: D17937687
fbshipit-source-id: a0cfdddbf1d97f1a64624bca6aa9536620f21ad4
Summary:
`xplat` targets add different deps based on what platform the target is being built for.
for anything using `fb_xplat`, we can put all ios supermodules in `fbobjc_labels` and all android sms in `fbandroid_labels`
There's some weirdness with python targets like `thrift_gen` in `/xplat/mobileconfig/tools/generator/gen-py/BUCK` that don't have platform-specific labels because the except_for list for `fbandroid` doesn't need the `fbsource//` prefix (see changes in `/ios/isolation/infra.mobileconfig.sm`)
Changelog: [Internal]
Reviewed By: shergin, joshleibsly
Differential Revision: D17884952
fbshipit-source-id: e245364cf515b75682990094d24f789d53b1f3f5
Summary: Hiding casting madness and complexity behind a helper function to avoid bugs and improve maintainability.
Reviewed By: sammy-SC
Differential Revision: D17923549
fbshipit-source-id: 105891d85b0412fa4a17d7ae8a9e156fc1b151fb
Summary:
Destruction of Scheduler (and ShadowTreeRegistry as part of it) which has some running Surfaces is not a good thing and practically a bug on the application layer.
With this assert we throw early to flag the issue.
Changelog: [Internal] - Dev only assert indicating a broken invariant in ShadowTreeRegistry
Reviewed By: sammy-SC
Differential Revision: D17924491
fbshipit-source-id: 6b7433fe47630e993e7d5b969f3113f96124b6c9
Summary:
The Android makefiles had hard-coded paths to hermes-engine, sometimes looking in `node_modules` and other times looking in `..`. This commit implements the Node module resolution algorithm (see common.mk), which handles both of these cases and also looks further up the root if necessary, handling the case when the `hermes-engine` npm package is hoisted.
This commit does three things:
- Defines `find-node-module` and uses it in the makefiles to find `hermes-engine`
- Removes the unused `/path/to/hermes-engine/include` paths since this directory does not exist and should be `/path/to/hermes-engine/android/include` (`android`)
- Moves the definition of `REACT_NATIVE` in the makefiles to the top. It was defined after every `$(CLEAR_VARS)` invocation but was not actually cleared anyway. `$(CLEAR_VARS)` resets only `LOCAL_*` variables in this list: https://android.googlesource.com/platform/build/+/7dc45a8/core/clear_vars.mk
## Changelog
[Internal] [Changed] - Android Makefiles look for hermes-engine using Node's module resolution algorithm
Pull Request resolved: https://github.com/facebook/react-native/pull/26820
Test Plan: Run `./gradlew :ReactAndroid:installArchives`, which requires the hermes-engine paths to be correct in order to find the headers.
Differential Revision: D17923671
Pulled By: cpojer
fbshipit-source-id: 9238b8718a94080db1abbba6375a6a1d484c871d
Summary:
**Context**
For method calls from JS to Objective C, we have to convert JS values to ObjC objects/primitives. Before we can call our ObjC methods, we need to run both the ObjC primitives and objects through `RCTConvert`. This is necessary, because we sometimes convert `NSDictionary`s to special Objective C objects. Apparently, in `RCTTiming`, we also do the same with `double` (i.e: we convert a `double` arg to another `double` type with different meaning).
**Problem**
`RCTTiming` used `RCTConvert` to convert `double`s into `NSTimeInterval` (also a double). The conversion is defined like this:
```
// i.e: division by 1000
RCT_CUSTOM_CONVERTER(NSTimeInterval, NSTimeInterval, [self double:json] / 1000.0)
```
This diff implements the support necessary to make this work. For completeness, I also implemented the same functionality for `BOOL`s.
Changelog: [iOS][Fixed] Improve method argument RCTConvert logic
Reviewed By: mdvacca
Differential Revision: D17887915
fbshipit-source-id: 3246fdbf4db7e96911f16460d92448b1f1e99444
Summary:
In the legacy system, when NativeModules are supposed to be initialized on the main queue, we do the following synchronously on the main thread:
1. Done: Attach bridge on main queue
2. Register the NativeModule for frame updates on main queue
3. Post Notification that NativeModule was created on main queue
4. Attach methodQueue on main queue
5. Call new on main queue
`[RCTModuleData instance]` is the entrypoint for all of this logic.
We probably shouldn't synchronously execute all this initialization on the main queue, because it can lead to deadlocks down the road. Therefore, this diff makes it so that we still call `new` on the same thread. However, we do all other initialization in the main thread, if that's required.
Changelog: [iOS][Fixed] TurboModule initialization on the main queue
Reviewed By: PeteTheHeat
Differential Revision: D17867583
fbshipit-source-id: a88412ee1e3d93a4f9b5ab0b4dd8fc5213fa91f8
Summary:
This pull request replaces the last remaining Unix headers in `JSBigString` with their equivalent Folly Portability headers, and replaces the calls to `getpagesize()` with `sysconf(_SC_PAGESIZE)` since Folly Portability is missing that function.
The work to get this building on windows was mostly done by acoates-ms, this pull request just adds the finishing touches.
## Changelog:
[General] [Fixed] - Fixed `JSBigString` not compiling on Windows due to Unix-specific headers
Pull Request resolved: https://github.com/facebook/react-native/pull/26826
Test Plan: Compiled with Clang and with MSVC (2017)
Differential Revision: D17903214
Pulled By: cpojer
fbshipit-source-id: 230f8fb410fa81d8f13d8b6ccf1147cfc70358bf
Summary:
In a previous diff I converted this line from a shared_ptr to a raw pointer. I'm documenting the reason why in code here to proactively document assumptions.
Changelog:
[[Internal]]
Reviewed By: shergin
Differential Revision: D17884942
fbshipit-source-id: 25e29a5e3ff0695d081c8d46c1ceaa6458248e26
Summary:
Ensure that on Android, only Scheduler and UIManagerBinder directly retain UIManager. These existing JSI function-retained lambdas will not have owning share_ptr refs to UIManager, just raw pointers.
By the time the VM deallocates all JSI objects and UIManagerBinding goes away, it should be able to deallocate UIManager synchronously when UIManagerBinding is deallocated. The VM should not be executing any of these JSI functions at that time, so this should ensure that the raw pointer access is safe.
This should also provide some extremely small perf wins, and make our memory model easier to reason about, since we're using shared_ptr less often and now `UIManager` is only owned in two places: Scheduler and UIManagerBinding.
So, there are now only two places where UIManager can be deallocated:
1) Hermes is deallocated first, and UIManagerBinding is deallocated before Scheduler. Scheduler holds onto UIManager. Scheduler later is deallocated and frees UIManager; this would cause a crash if it's not completed before Hermes is torn down, but we currently ensure that Scheduler is torn down before Hermes. This is fine but we should document this contract, or make Scheduler not own UIManager.
2) Scheduler is deallocated first, decrementing the shared_ptr to UIManager. Later Hermes is torn down, which deallocates UIManagerBinding via the JSI pointer table, which synchronously deallocates UIManager before Hermes has shut down. This is the desired outcome.
Changelog:
[[Internal]]
Reviewed By: shergin
Differential Revision: D17872586
fbshipit-source-id: cbb25b5cd025f5f8597dc7a66073fe64edc57aa8
Summary: Storing a Bridge introducing an retain cycle, so we need to store that weakly.
Reviewed By: sammy-SC
Differential Revision: D17773698
fbshipit-source-id: 824a83a6086f9ae6efb7c458d833931954c55643
Summary: We need to have a weak semantic for the ManagedObjectWrapper, this diff implements that.
Reviewed By: sammy-SC
Differential Revision: D17773699
fbshipit-source-id: 978fa62191361c3b2c0e46c423240831ed1dd233
Summary: In D16805827, I moved `RCTImageLoader`, `RCTImageStoreManager`, and `RCTImageEditingManager` to `CoreModules`. This was necessary to turn `RCTImageLoader` into a TurboModule. However, after D17671288 landed, it's no longer necessary to have OSS NativeModules in `CoreModules`. Therefore, I'm moving these NativeModules back to `RCTImage`.
Reviewed By: PeteTheHeat
Differential Revision: D17720575
fbshipit-source-id: 44b07cfa07cbb2b87254132810f86974edc7edab
Summary:
`MountingCoordinator::waitForTransaction()` allows waiting for a new coming transaction synchronously, as efficient as waiting for a mutex.
This feature can be used to implement more high-level synchronous rendering APIs.
Reviewed By: mdvacca
Differential Revision: D17629425
fbshipit-source-id: acd91b941e4d6d43bc4518f332a1604e14506be9
Summary: We need commands to work with `LegacyViewManagerInterop`. We will need to rethink this once Fabric has command-execution pipeline in place.
Reviewed By: shergin
Differential Revision: D17787294
fbshipit-source-id: a6b3dbfae41f04e7e7f5bafb1f7b4ad0de0eedc3
Summary:
LegacyViewManagerInterop layer can now handle events as well.
We expose `eventInterceptor` from `NSComponentData`, that way we can hook into event dispatching that happens within `NSComponentData`.
Coordinator has a map of `UIView -> reactTag` which it uses to figure out to which component view to forward the event.
New `LegacyViewManagerInteropViewEventEmitter` exposes APIs to send `folly::dynamic` to javascript.
Reviewed By: shergin
Differential Revision: D17765834
fbshipit-source-id: 25e75e445b32c69ec9ab0189c4ab7fba5f8c7b5d
Summary:
ComponentDescriptor owns a coordinator which is initialised with ViewManager that it represents.
Coordinator is passed to ComponentView through state and ComponentView asks the coordinator for view.
Later, ComponentView will ask coordinator to configure view with specified props.
Reviewed By: shergin
Differential Revision: D17670444
fbshipit-source-id: e920c5c4f033884c4b539ce711286f71c887d896
Summary:
# LegacyViewManagerInterop is born
LegacyViewManagerInterop is a component that should make it possible for legacy components to work in Fabric, new renderer.
This is just a first stage that prints keys of props to screen together with component name.
Reviewed By: shergin
Differential Revision: D17552827
fbshipit-source-id: c3e062f413727729e6a9b683c60f59f0292cc63b
Summary:
We use `ViewManager.onMeasure` to perform measurements of Android views and pass the measured size back to Yoga. For Android in order to the report correct dimensions of a View, this View must be created using a Context that has a theme associated with it. Before, `onMeasure` only had ReactApplicationContext passed as the first parameter and ReactSwitch, for example, could not be measured correctly (because it uses the size of the thumb drawable, which is extracted from the current theme). This diff adds surfaceId as the first parameter of `FabricUIManager.measure`, so that we can retrieve ThemedReactContext and pass it to `ViewManager.onMeasure`.
The size of the Switch component is still incorrect, but at least the size reported back to Yoga is the same as in Paper. So there is more investigation necessary why this happens in Fabric. I will investigate and publish another diff with the fix.
Reviewed By: JoshuaGross, shergin
Differential Revision: D17625959
fbshipit-source-id: 48197a61240fb13042bef3e9f5d681acacc702fb
Summary:
In this diff we integrate the Switch component on Android in Fabric. Since the component has a custom measure function, we need to write some C++ to call the measure method in Java.
The component isn't fully functional yet (setNativeProps isn't supported in Fabric) and has some problems with measuring itself. I will fix the component in the next diffs in this stack.
Reviewed By: JoshuaGross
Differential Revision: D17571258
fbshipit-source-id: be4e201495b9b197ddec44ee3484357bfb6225a8
Summary: I was trying to debug an issue with a method signature mismatch last week, but was having trouble figuring out the problem because the error I was getting was unrelated - apparently JNI will sometimes swallow the error and just fail mysteriously later on. Ramanpreet showed me this macro that will throw any pending exceptions, so let's do that after we try to lookup the Java method in case it fails.
Reviewed By: RSNara
Differential Revision: D17680121
fbshipit-source-id: 1f23e49014f7cc1616e111386d440637e6a74677
Summary: Until ComponentDescriptorProviderRequest is not fully implemented we need to disable the assert because it enforces constraints that we don't satisfy yet.
Reviewed By: JoshuaGross
Differential Revision: D17677856
fbshipit-source-id: bdf96a04a16981f1c639f646f254e8f3dd799419
Summary: This diff implements the core functionality of "reactive component registration". With a new API now we have a public API that allows registering a component by request on-demand. That can be helpful in some environments where loading of all components is undesirable or impossible due to generic app design constrains.
Reviewed By: mdvacca
Differential Revision: D17211914
fbshipit-source-id: 5a957a4f6541420d70952856599b827ceec63e3b
Summary:
Currently, the same ComponentDescriptor class cannot be registered as a responder for components with different names. However, we have marginal cases where we really need it. The examples are `UnimplementedView` or possible universal interop with the classic RN or any other UI framework.
This change adds a special optional argument to ComponentDescript constructor that allows implementing this functionality.
Reviewed By: fkgozali
Differential Revision: D17211915
fbshipit-source-id: 18f59e09fe06b875a8e8975b7b2ab423489238bb
Summary:
This part of the codebase is very perf sensitive and designed to work without exceptions enabled.
Most of the method was `noexcept` all the time, but some of those missing that by mistake.
Reviewed By: sammy-SC
Differential Revision: D17629426
fbshipit-source-id: b311e4b7eff8e2b7cf29518288480d3a812dda44
Summary: Make it possible for a Runtime implementation to provide its own JSON parsing, by making the method on Value call through Runtime, which now has the default implementation.
Reviewed By: tmikov
Differential Revision: D17637395
fbshipit-source-id: b8997f7d1721a7790326417f3abfa86c875923c9
Summary:
There are cases where the CallbackWrapper instances were added from different thread, potentially crashing the inner std::unordered_set<> we're using to keep the wrappers alive for extended time.
To avoid it, let's just use std::mutex.
Reviewed By: shergin
Differential Revision: D17631233
fbshipit-source-id: e8f98004e45a68be31f8f0cda118fb67dcb06d45
Summary:
`addObserver` and `removeObserver` now accepts const references instead of pointers which indicates the intent (non-nullability and non-owning) clearly. The delegate methods are also marked as `const` to designate the possible concurrent execution (`const` means "thread-safe" here).
All changes are pure syntactical, nothing really changes (besides the fact overall code quality and redability).
Reviewed By: JoshuaGross
Differential Revision: D17535395
fbshipit-source-id: b0c6c872d44fee22e38fd067ccd3320e7231c94a
Summary:
# A race condition
The practical thing of this diff is fixing a data race.
Imagine a case where a thread A calls `addObserver` and thread B calls `nativeImageResponseFailed` at the same time.
Thread A might read `status_` exclusively and store result as a local variable and then go sleep.
Then thread B starts and finishes `nativeImageResponseFailed`, it writes `status_` and notifies all observers.
Then thread B wakes up. It adds an observer to a collection of observers and finishes.
As a result, the observer from `addObserver` will never be called.
To fix this, we changed a logic a bit to lock only once per method. During the lock, we read and/or write to storage and then perform side-effects.
In contrast, previously we often locked only around the access of a particular instance variable (several times per method).
The challenge here is that idiomatic/fancy to C++/STL ways to lock mutexes don't work in our case.
# C++ idioms and readability, multiple locks for the same transaction
STL has tools to avoid calling `lock` and `unlock` methods manually (std::lock_guard<> and lamdas). Unfortunately, using that in our use case is quite problematic. That's probably possible but will lead to much less readable code and some copy-pasta in `addObserver`.
Therefore we replaced using `std::lock_guard` with simple `lock` and `unlock` where using `std::lock_guard` was problematic.
# Why we changed `shared_mutex` to a normal one?
After consolidating the locks we found that we have an only case where we can use shared lock (in `nativeImageResponseProgress`). Calling this method in real life is not concurrent, so it makes sense to replace a shared lock with a more simple and performant regular one.
Reviewed By: sammy-SC
Differential Revision: D17368739
fbshipit-source-id: 61d66fb737d8c2dc73001a80a31edaa59a16d886
Summary:
This diff contains some small improvements in `ImageResponseObserverCoordinator` which are pretty minor:
* Now we use `small_vector` instead of a normal one. In the vast majority of cases, the coordinator has only one observer, so having `small_vector` with default size `1` saves us a memory allocation (which is a dozen of allocations for a screen, not bad).
* Empty constructor and destructor were removed.
* Unnecessary copying of ImageResponse was removed. ImageResponse is a practically a shared_pointer, it has value semantic and does not need to be copied. We don't need to acquire mutex to access that.
Reviewed By: sammy-SC
Differential Revision: D17368740
fbshipit-source-id: 828e27a72b9c8ac0063c5fbda00f83ddb255309c
Summary: Now that all the plumbing is done, this diff finally implements async method dispatch on the NativeModule thread.
Reviewed By: mdvacca
Differential Revision: D17480605
fbshipit-source-id: 992aab99954c488a0327144d84a1668a2b158d04
Summary:
Self explanatory.
1. **Existing:** We create the NativeModules thread in CatalystInstanceImpl.cpp.
2. D17422165: We wrap this thread in a `BridgeNativeCallInvoker`.
3. D17422164: We use `CatalystInstanceImpl::getNativeCallInvokerHolder()` to get a hold of the `BridgeNitiveCallInvoker` in Java.
4. D17422163: From Java, we pass this `CallInvokerHolder` to `TurboModuleManager`'s constructor.
5. **This diff:** `TurboModuleManager` then unwraps the `CallInvoker` from `CallInvokerHolder`, and passes it to all TurboModules in their constructor.
Reviewed By: PeteTheHeat
Differential Revision: D17422160
fbshipit-source-id: c0a76dfe5fdedac2e0e21f7a562bc7588dc190fb
Summary: This abstraction will be used by TurboModules to schedule work on the NativeModules thread.
Reviewed By: PeteTheHeat
Differential Revision: D17422165
fbshipit-source-id: d5ca7837a0ecbcc2118813e1bafa6d445ba2ef3b
Summary:
## Motivation
The concept behind JSCallInvoker doesn't necessarily have to apply only to the JS thread. On Android, we need to re-use this abstraction to allow execution of async method calls on the NativeModules thread.
Reviewed By: PeteTheHeat
Differential Revision: D17377313
fbshipit-source-id: 3d9075cbfce0b908d800a366947cfd16a3013d1c
Summary:
If you try to use ART components with fabric, their transform is causes crash (stack trace P108110438). You can see this crash if you use RCTVideo component in fabric and tap video to reveal slider.
The cause of crash is mismatch of formats between what is expected for transform and what is being sent.
In this diff we add a check to see whether the configuration is of expected type.
Reviewed By: shergin
Differential Revision: D17181838
fbshipit-source-id: c4f3c920281a2e7f58ff0ffe1d0ec2af8249a16c
Andres Suarez
Joshua Gross
Janette Cheng
Peter Argany
Valentin Shergin
zhongwuzw
James Ide
Ramanpreet Nara
empyrical
Ashok Menon
Samuel Susla
masaaki1915
Adam Ernst
Oleksandr Melnykov
Emily Janzer
Daniel Andersson
Kevin Gozali
Marek Cirkos