Summary:
We change yoga pod name to Yoga in 82a8080f07, so let's change it in Fabric pod.
## Changelog
[iOS] [Fixed] - Change Fabric podspec dependency yoga to Yoga
Pull Request resolved: https://github.com/facebook/react-native/pull/26800
Test Plan: Null.
Reviewed By: sammy-SC
Differential Revision: D17853707
Pulled By: mdvacca
fbshipit-source-id: 84a537f3ede7f2a86a08d532ffbbe4bf23cbf846
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
Summary:
Merging react-native-windows to 0.60 - the visual studio compiler seems to take issue with the existing code
## Changelog
[Internal] [Fixed] - Build fix for react-native-windows (MSVC)
Pull Request resolved: https://github.com/facebook/react-native/pull/26462
Test Plan:
No real change, just making compilers happy.
### Side Note
We'll want this change cherry-pickered to RN 0.60 and RN 0.61 so users of react-native-windows dont have to use our fork of react-native.
Reviewed By: mhorowitz
Differential Revision: D17406081
Pulled By: TheSavior
fbshipit-source-id: bc056e1a545c6478fdcbd5645f3a8dea657162c8
Summary: This is just a minor fix to D17380360. Basically, we want to make sure that callbacks can only be called once on the ObjC side.
Reviewed By: fkgozali
Differential Revision: D17403852
fbshipit-source-id: b0d2bbd539daef4837a345bc2953dd9687b3147b
Summary: There is a race condition between tearing down the bridge vs CallbackWrapper invoking the wrapped jsi::Function. This means the wrapper can be stale and unsafe to access after the bridge dies. To avoid unsafe access, let's clean it up properly using weak ref.
Reviewed By: RSNara
Differential Revision: D17380360
fbshipit-source-id: f91ce75d945bf8ed0b141c593bcc674ff465aa8c
Summary:
`getHeapInfo` was creating and return a `jsi::Object`. This object was not being traced
when API tracing was turned on.
This resulted in a bug in synth traces, an object was being acted upon without its creation source
being logged.
In order to prevent this, change the API of `getHeapInfo` to no longer return any objects, or modify other heap
contents.
This is preferable to having `TracingRuntime` attempt to mimic the operations of two different implementations.
## Changelog:
[Internal] [Changed] - Changed `jsi::Instrumentation::getHeapInfo` to use a `std::unordered_map`
Reviewed By: mhorowitz
Differential Revision: D17273235
fbshipit-source-id: f69860dcc524c2cf501746a41dbac20b4db8c456
Summary:
This diff improves on D17244061 by moving the `Jni::JniLocalScope` to earlier on in `JavaTurboModule::invokeJavaMethod`. The problem with D17244061 was two-fold:
1. It completely ignored the local references created by just calling `getConstants()`. So, you could spam `getConstants()` and still get an overflow.
2. The capacity was incorrect. Basically, two things happen when you call `PushLocalFrame`. One, we push an actual Jni frame onto a stack somewhere in the dvm: https://fburl.com/f16wfrxi, https://fburl.com/jhpha563. Popping off this stack is how the local references are cleared when we call `PopLocalFrame`. Two, we do a check to make sure that we can support at least `capacity` more local references: https://fburl.com/jucvew8j. The max is 512. The problem before was that, I was just using the argument count for the capacity. This was inaccurate because we create many `jclass` objects, and several misc. intermediate java objects when we invoke `JavaTurboModule::invokeJavaMethod`. So, I've refined the calculation of capacity to be a bit more accurate. This should make sure that capacity check works, which should help us fail faster if our local reference count approaches 512.
Reviewed By: shergin
Differential Revision: D17337354
fbshipit-source-id: 45574bae4748c52d8f919c1480b9a0936d970628
Summary: Include <cstdarg>, which defines the va_start and va_end macros.
Reviewed By: tmikov
Differential Revision: D17344433
fbshipit-source-id: f51fb53cc6e016eeef1a9270c1f2c468a5edc967
Summary:
Managed to get fabric building in my app. Here are the changes needed in the podspec.
There were other issues with codegen but it would be too involved to fix for now. This is a good first spec to get it working better in OSS.
## Changelog
[iOS] [Fixed] - Add some missing dependencies to the fabric podspec
Pull Request resolved: https://github.com/facebook/react-native/pull/26197
Test Plan: Managed to get fabric building in a project.
Reviewed By: cpojer
Differential Revision: D17091504
Pulled By: osdnk
fbshipit-source-id: a5d0c46899e342e07ceafc2e4514a03fabf1471d
Summary:
This diff changes the value of `EmptyLayoutMetrics` to make it as unusual and useless as possible. This helps when we need to compare sub-values of `LayoutMetrics` to apply only changed ones.
For example, let say we need to make a transition between two `LayoutMetrics` values. Let's say the first (the source) one equals `EmptyLayoutMetrics`. In order to apply only changed part, we need to compare individual sub-values, like `frame` or `displayType`. Before this change, the default value of `borderWidth` was `{0, 0, 0, 0}`. So in case if the second (the destination) value is also `{0, 0, 0, 0}`, the operation will be skipped.
This is undesirable because all new values have to be applied anyway because `EmptyLayoutMetrics` designates that the actual previous values are unknowns.
This fixes some visual issues on iOS caused by this issue because recycled views sometimes have non-default layout values where the transition from `EmptyLayoutMetrics` to some arbitrary value skips some sub-values.
Reviewed By: JoshuaGross
Differential Revision: D17312176
fbshipit-source-id: 7f311baea202ec2662ca87be0ae0ae6c6dd42712
Summary:
Because we use the `PromiseWrapper` struct, we need to explicitly manage its lifecycle to ensure that it doesn't clear before the promise methods are invoked by the ObjC Runtime. This `PromiseWrapper` struct is unnecessary. We could just not have it and create the CallbackWrappers for resolve and reject within the `createPromise` function. Therefore, I moved all the logic from `PromiseWrapper` to the `RCTTurboModule::createPromise` function.
In the next diff, I'm going to keep a track of all the CallbackWrappers we create in instances of RCTTurboModule, and `destroy()` them in the destructor of RCTTurboModule. This should make sure that all `jsi::Function`s are released before we delete the `jsi::Runtime`, which should prevent Marketplace from crashing when we hit CMD + R. See: https://fb.workplace.com/groups/rn.support/permalink/2761112713937326/.
Reviewed By: fkgozali
Differential Revision: D17208729
fbshipit-source-id: ce80c9c01088f0e3dc47c7c29397b7a197d699ce
Summary:
When doing RN init from scratch for 0.61, I noticed pod install was failing. This was because "Yoga.podspec" was mis-spelled to "yoga.podspec" in the yoga ReactCommon folder. Thus, `pod install` was failing
```
[!] No podspec found for `Yoga` in `../node_modules/react-native/ReactCommon/yoga
```
## Changelog
[iOS] [Fixed] - Fix Yoga.podspec case sensitivity
Pull Request resolved: https://github.com/facebook/react-native/pull/26360
Test Plan: `pod install` now runs successfully with a clean build of master.
Differential Revision: D17314027
Pulled By: cpojer
fbshipit-source-id: 8db2ac4dd9295afcc0e074c842dcda71b7b3b668
Summary:
Remove cxx from iOS project, because we have the corresponding implementation in iOS. Otherwise, it would break the logical handling of iOS part.
## Changelog
[iOS] [Fixed] - Remove cxx from iOS project
Pull Request resolved: https://github.com/facebook/react-native/pull/25392
Test Plan: Fabric enabled Tester can work like loading image.
Reviewed By: mdvacca
Differential Revision: D17285463
Pulled By: shergin
fbshipit-source-id: b414406578dcce51f3b54fd06941225efc560e7f
Summary:
The code is moved from the destructor a separate method that we now call in the Scheduler::stopSurface.
That makes the code more readable and resilient to possible races and ownership-related changes.
Reviewed By: JoshuaGross
Differential Revision: D17272294
fbshipit-source-id: 948d76d074577beb3dda6defdf09261b5c8abb98
Summary:
MountingCoordinator is a borderline between Core and Mounting. Some of Core design constraints are impossible/impractical to enforce on Mounting layer, so we have to handle all of those cases in `MountingCoordinator`.
One of the constrains is that all ShadowNodes implicitly depend on associated ComponentDescriptor instances without retaining them (retaining is expensive and creates a retain cycle).
The problem is that the Mounting layer can call `MountingCoordinator::pull()` at any moment (even after the whole Core is already destroyed). To prevent this, the owner of a `MountingCoordinator` on the Core side calls `revoke()` right before being deallocated (right before the moment the owner cannot guarantee the constraint).
Reviewed By: JoshuaGross
Differential Revision: D17272295
fbshipit-source-id: ba8b02eab8f84cce68aa65c1ad36950cd2498049
Summary: Whenever we invoke a method, we convert JS arguments into JNI objects. These JNI objects are all local references that need to be manually destroyed after the method call happens. Therefore, I'm using `JniLocalScope` to automatically do this whenever the stack is cleared after the call to `JavaTurboModule::invokeJavaMethod`. This should hopefully get rid of the JNI table overflow we're seeing in T52817336.
Reviewed By: mdvacca
Differential Revision: D17244061
fbshipit-source-id: 92ca78cdb23ad8dfe2425db46e086c10c9662fe2
Ashok Menon
zhongwuzw
Valentin Shergin
Ramanpreet Nara
Samuel Susla
masaaki1915
Adam Ernst
Oleksandr Melnykov
Emily Janzer
Daniel Andersson
Kevin Gozali
Marek Cirkos
Gabriel Furini
REDMOND\acoates
Riley Dulin
Janic Duplessis
Hassan Bazzi
