Summary:
This is another attempt to fix an issue very similar to T59424871. The previous attempt was in D19249490. I don't know why we don't see production crashes (stalls) but it happened to me (and not to me) in the debugger. The previous attempt didn't work because we still could have a deadlock because we tried to acquired shared mutex already owned exclusively by the `suspend` method.
Here is another approach: Instead of using one shared mutex, now we use two. One is similar to what we had and another that protects `suspend` and `resume`. Besides that, now we pass a Scheduler to functions that use that explicitly. This way we can be more explicit about acquiring mutexes and the overall flow of execution. The idea is: Now an arbitrary code that can be reentrant does not cover with mutexes, so the deadlock is not possible.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20639228
fbshipit-source-id: 98515742f00f2ae94b50b585c9f1f0611e169ebe
Summary:
Changelog: [internal]
layoutDirectionPass down layoutDirection from `RCTSurfacePresenter` down to `YogaLayutableShadowNode`.
In `ParagraphShadowNode`, we propagate layoutDirection from yoga node to `TextAttributes.layoutDirection`.
Reviewed By: shergin
Differential Revision: D20420041
fbshipit-source-id: 86e01d31ea9415acb8579c44c470cac870ec1b8f
Summary:
Changelog: [Internal]
Use LayoutContext to pass `fontSizeMultiplier` down to ParagrapShadowNode.
Reviewed By: shergin
Differential Revision: D20184596
fbshipit-source-id: 3965a127069a21328ed19cb3f9732f0a2d1c4d58
Summary:
In order to build dynamic text sizing, `LayoutableShadowNode::measure` needs to accept `LayoutContext`
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D20184598
fbshipit-source-id: 8928b59d51948caf3654f40049212a89a91dceb6
Summary:
Changelog: [Internal]
Exposes `synchronouslyWaitForStage` to `RCTFabricSurface`.
This is a first step towards having screenshot tests rendered with Fabric.
Reviewed By: shergin
Differential Revision: D19603837
fbshipit-source-id: 26c14cf3bbd67fea96319ff08d3321557ddcdd9c
Summary:
This fixes a deadlock caused by recursive calling of `-[RCTSurfacePresenter setMinimumSize:minimumSize:]` (because of an attempt to lock the mutex which was already locked upper the call stack).
Seems there is no reason why this operation should not be allowed. This diff replaces a regular mutex with a shared one which allows multiple shared locks and this prevents the deadlock. A Scheduler is already a thread-safe object and the mutex only protected the mutation of a pointer to it.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19249490
fbshipit-source-id: 00e8934c6502328f34b78ea6ec004b7216665db1
Summary:
I moved towards C function to return component class instead of delegate.
We need this in order to register components that are optimistically registered, such as View, Image and Text.
In D19088558, we need this in order to register Image.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19088559
fbshipit-source-id: 061f3ba15dfb44b9acce7be2dc6828b9afbecbfa
Summary:
We are seeing some non-trivial amount of crashes happened because `[RCTSurfacePresenter synchronouslyUpdateViewOnUIThread:props:]` requests a Component Descriptor which does not register in the registry.
And the problem here is that ComponentDescriptors are not being designed to be used outside of C++ UIManager, and we only use that in RCTSurfacePresenter only because it's an old workaround that makes Native Animation Driver works with Fabric.
Messing with ComponentDescriptors are in general dangerous. Accessing them outside of UIManager means that they might be deallocated at any time, extending their lifetime will cause other crashes on the other side. So, that's why this is "BROKEN".
This diff does not make the code worse, it just designates the problem that was there for a long time, so it kinda makes it better.
We don't know for sure why some ComponentDescriptors are not registered but this diff at least makes it not crash in such case.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D18869651
fbshipit-source-id: 9d945ac7a2bd24a69a9bbb83b4fdd3cd19808f87
Summary:
This diff introduces RCTComponentViewDescriptor - the container for a view and associated with this view properties which mounting infra uses for bookkeeping (and recycling) views.
The previous implementation used raw Objective-C pointers to `UIView`s to store and manipulate them. The new way has a bunch of advantages:
* It allows using high-performant C++ data collections for storing views and their properties (in future diffs).
* The new approach allows us to avoid hacks around NSMapTable (such as ` [_registry setObject:componentView forKey:(__bridge id)(void *)tag];`) that were needed because NSMapTable wasn't designed for our use-case.
* Dealing with `RCTComponentViewDescriptor` which stores a pointer to a view sometimes is actually more efficient than dealing with those pointers themselfs because we can deal with `const &` to a descriptor which does not require a ref-counter bump.
* A new approach is much more flexible, it allows us to store additional data alongside view instances which we will use in coming diffs.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18217102
fbshipit-source-id: 063e6c7df794a2e1fd690c194fb31ad6833eaba7
Summary:
In addition to already exiting method `componentViewByTag`, this diff adds a new one `findComponentViewWithTag` which does pretty much the same thing but returns `nil` in a case when it cannot find a view with a given tag. The first method now returns a non-nullable object, whereas the second (new) one return nullable.
We need this to explicitly designate call-sites that are resilient to such kind of errors, where such errors are expected because of still-existing consistency problems (because of some legacy implementation approaches).
At the same time, for the call-sites where a missing view situation is unrecoverable, we should throw early and stop paying for nullability overhead.
In the future diffs, we will change the internal implementation of the registry where the price for dealing with nullability will be actually meaningful.
This is the first diff on the road to an implementation of `RCTMountingTransactionObserving` protocol.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18217099
fbshipit-source-id: dc50033ecadd43c43c5a65fed62649a1a7acf2b5
Summary:
Yes, this is a pretty big diff; because of the high interconnectedness of the things, I failed to split it apart.
This change does:
Introduces a new class RCTSurfacePresenterBridgeAdapter which decouples all Bridge-related functionality from RCTSurfacePresenter.
The new class allows "replacing" one instance of the bridge with another for a single RCTSurfacePresenter.
This change allows implementing unloading a bridge (e.g. during memory pressure warning) and suspending all RN surfaces with the future possibility of reloading a bridge and resuming surfaces.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D17960239
fbshipit-source-id: 7ae556ed91030f4c5ab187689ce6bd161fabde93
Summary:
We need this for better decoupling RCTSurfacePresenter from Bridge.
Changelog: [Internal] - Changes in RCTSurfacePresenter (Fabric)
Reviewed By: sammy-SC
Differential Revision: D17960237
fbshipit-source-id: e91cd04eab967745c6e5151187eb68ba108f3986
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:
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:
1. Start surface when first run, before, when we first load js bundle, `_startAllSurfaces` would not be called because `bridge == _batchedBridge`.
2. I think this fixes https://github.com/facebook/react-native/issues/23910, so I add the `return` in swizzle method to check.
cc shergin.
## Changelog
[iOS] [Fixed] - Make surface presenter manage start of surface
Pull Request resolved: https://github.com/facebook/react-native/pull/24372
Test Plan: N/A.
Differential Revision: D17337865
Pulled By: shergin
fbshipit-source-id: 2a1e413c006cae74ef6ca4c2b6b5ee8a46434b7f
Summary: Previously, the `_scheduler` method in `RCTSurfacePresenter` was implemented as a lazy getter. The only problem with that is that Scheduler instance might be (re)created in the middle of the hot-reloading process (e.g. external request to relayout some Surface might trigger that). Since it does not make any sense to create an empty Scheduler during the reloading process, now the Scheduler creation only happens in constructor and right after the VM is reloaded.
Reviewed By: JoshuaGross
Differential Revision: D17299441
fbshipit-source-id: 273451bbb03e8cdf532131adfdf3bc60c34e997e
Summary:
This diff fixes a redbox happens on every single hot-reload practically saying that ImageLoader is misconfigured. The issue happened because after reloading Fabric used the previous obsolete instance of `ImageLoader` created and stored inside old (and already destroyed) instance of Bridge. The fix is simple: We update all dependencies after the Bridge was reloaded.
See https://fb.workplace.com/groups/rn.support/permalink/2677343372314261/ for more details.
Reviewed By: mdvacca
Differential Revision: D17173702
fbshipit-source-id: 5ff0c418feae10ede9b76c184cd24ad06ee008b7
Summary:
The purpose of `EventBeat` is handling an asynchronous callback to itself which is being delivered on some different thread. That brings a challenge of ensuring that the `EventBeat` object stays valid during the timeframe of callback execution. The concept of Owner helps with that.
The owner is a shared pointer that retains (probably indirectly) the `EventBeat` object. To ensure the correctness of the call, `EventBeat` retains the owner (practically creating a retain cycle) during executing the callback. In case if the pointer to the owner already null, `EventBeat` skips executing the callback.
It's impossible to retain itself directly or refer to the shared pointer to itself from a constructor. `OwnerBox` is designed to work around this issue; it allows to store the pointer later, right after the creation of some other object that owns an `EventBeat`.
Reviewed By: JoshuaGross
Differential Revision: D17128549
fbshipit-source-id: 7ed34fd865430975157fd362f51c4a3d64214430
Summary: Storing a strong shared pointer to `ComponentDescriptor` can cause a memory leak. Therefore we enforce all call sides and params to be weak pointers. The only Scheduler preserves a retaining pointer to it (to prevent preliminary deallocation).
Reviewed By: sammy-SC
Differential Revision: D17115540
fbshipit-source-id: fdea7d19f742ff04d5ba5470dd9748a5b226aa7c
Facebook:
This technically culminates M8 for iOS in Venice :D
We're now rendering `View` and `Text` completely without the bridge!
Reviewed By: shergin
Differential Revision: D16950332
fbshipit-source-id: f8c896323756411f5ac97586c0d85fdd6e39ed40
Summary: Instead of getting `RuntimeExecutor` from the bridge, pass it from above. Right now pass through `null`, but eventually this will work :)
Reviewed By: RSNara
Differential Revision: D16626288
fbshipit-source-id: bce527f85c0a79cfe6cf240a3633bbbe357f75c4
Summary: Instead of grabbing `imageManager` from the bridge, pass it from above. Right now, still use bridge to pass from above, but this gives us flexibility to not use bridge in the future.
Reviewed By: shergin
Differential Revision: D16504270
fbshipit-source-id: 7977a7957b659375f8348d26cd57b648e9d5959f
Summary: The previous rename from RCT->RN prefix ended up causing some confusions on which prefix to use for which files and under what circumstances. To avoid further confusion before we're done with the re-architecture project, let's keep them as RCT.
Reviewed By: mdvacca
Differential Revision: D16705566
fbshipit-source-id: 395bff771c84e5ded6b2261a84c7549df1e6c5e5
Summary: Fabric ObjC(++) files will be prefixed by RN* for the time being, this codemod is a simple rename. This includes `interface` and `protocol` definition
Reviewed By: PeteTheHeat, yungsters
Differential Revision: D16611524
fbshipit-source-id: 868d2571ea2414dde4cbb3b75b1334b779b5d832
Summary:
Duplicate category method implementations cause undefined behavior in Objective-C; one is chosen essentially at random. The linker also issues a noisy warning about this (which is how I noticed this case).
It didn't matter in this particular case since both implementations do the same thing, but we should clean this up so people don't get desensitized to these linker warnings. There is no need to have two implementations.
Reviewed By: fkgozali
Differential Revision: D16587219
fbshipit-source-id: 56dc3493735443c476484092f4a7eacfcddee8cb
Summary: Right now RuntimeExecutor is only used in Fabric. Moving it out of Fabric's uimanager/primitives.h and into react/utils so we can use it more broadly.
Reviewed By: shergin
Differential Revision: D16385366
fbshipit-source-id: 96063e536e1480bac078a9376fe55f7d8750477e
Summary: We no longer want to access RCTImageLoader from the bridge category. Instead, let's use the `moduleForClass` API.
Reviewed By: shergin
Differential Revision: D16389113
fbshipit-source-id: c638f4b9851698afc53aaaa2b302d21cc19f76e7
Summary: Supporting View Manager Commands on the new UIManager in Fabric. This is needed for things like scrollTo on ScrollView.
Reviewed By: JoshuaGross
Differential Revision: D16175575
fbshipit-source-id: a74effdf7e47b56a150a4e3fb6c4d787659e0250
Summary: We have to use a getter instead of an ivar to enable lazy initialization of ContextContainer.
Reviewed By: mdvacca
Differential Revision: D15731877
fbshipit-source-id: eb4d0e70c337026a91cb12a3eb26ed4d94f39f9f
Summary:
... and slighly new behaviour for one of them.
The method does nothing if given `key` already exists in the container.
This diff finishes the transition of ContextContainer from an internal bag of things with unclear yet ownership into a legit dedicated dependency injection container for the product code.
The original names of methods imply that the container can have only one object of a given type which is no longer true. The new API is much more generic and idiomatic to C++, it mimics `std:map` API which is intuitive to anyone who familiar with C++ containers.
Besides the naming, `insert` method changed the semantic a bit; now it does nothing in case of inserting an object with a key that already exists. That might seem counterintuitive for "normal" people, but C++ has some wired reasons for that and, hopefully, it's expected behavior in the C++ community.
Fun fact: We need this to fix hot-reload.
Reviewed By: sahrens
Differential Revision: D15681736
fbshipit-source-id: 194f342528446a911eaf072ba3a94a5d8af3cb52
Summary: RCTComponentViewRegistry is not a thread-safe class and must be accessed only from the main thread.
Reviewed By: JoshuaGross
Differential Revision: D15296475
fbshipit-source-id: 67192abd6290191f3b8119972efc41cec48a793a
Summary: Now it's implementled differently (see -[RCTComponentViewRegistry preallocateViewComponents]), so this code is not being used.
Reviewed By: mdvacca
Differential Revision: D15242045
fbshipit-source-id: c02eceb978cf1eae778f84a73456e7156ccf503b
Summary:
ImageLoader is an actual external dependency, not a ImageManager.
That change allows to remove dependency on ImageManager from SurfacePresenter and make some other code simpler.
Reviewed By: mdvacca
Differential Revision: D15242047
fbshipit-source-id: 8622d15b8fdb5c3a7e25091adf7be1108f87ecd5
Summary: The particular meaningful piece of code that diff removes was added as (successful) attempt to fix an issue which was lately fixed by D14868547.
Reviewed By: mdvacca
Differential Revision: D15242046
fbshipit-source-id: 88a3e3c629d7c5f84c402b03e45034644147fec4
Summary: RCTBridge does not need to retain RCTSurfacePresenter, so we enforce that using `OBJC_ASSOCIATION_ASSIGN`.
Reviewed By: mdvacca
Differential Revision: D15273325
fbshipit-source-id: f223192ff5f781d9e905b004907739a36882bb63
Summary: Apparently we can/should not have in RCTConversions because it creates unnecessary dependency to core iOS module.
Reviewed By: mdvacca
Differential Revision: D15055325
fbshipit-source-id: 507f5a40c03b5c261967de4504297d31ecd02783
Summary:
`MountingTransaction` encapsulates all artifacts of `ShadowTree` commit, particularly list of mutations and meta-data.
We will rely on this heavily in the coming diffs.
Reviewed By: JoshuaGross
Differential Revision: D15021795
fbshipit-source-id: 811da7afd7b929a34a81aa66566193d46bbc34f8
Summary:
Registries, providers, providers of registries, registres of providers. All that can be really confusing, but that represents best the constraints and desires that we have:
* We need to be able to register components on-the-fly (so we need a mechanism that will propagate changes);
* We don't want to register ComponentDescriptors separately from ComponentView classes;
* C++ not always gives us abstractions that we want (e.g. pointers to constructors).
After the change, we can remove the whole Buck target that has a bunch of handwritten boilerplate code.
There is a still room for polish and removing some concepts, types or classes but this diff is already huge.
Reviewed By: JoshuaGross
Differential Revision: D14983906
fbshipit-source-id: ce536ebea0c905059c7a4d644dc25233e2809761
Summary:
ComponentDescriptorProvider represents unified way to create a particular descriptor.
Now all ComponentViews (which support RCTComponentViewProtocol) expose a `ComponentDescriptorProvider` which will allow creating and registering ComponentDescriptor instances for all visual components automatically as a part of ComponentView registration process.
Don't panic, everything is still being as explicit as it always was, no magic involved; we just will have only one registration step instead of two parallel.
That also opens a way to register components on the fly.
Reviewed By: JoshuaGross
Differential Revision: D14963488
fbshipit-source-id: 9e9d9166fabaf7b30b35b8647faa6e3a19cd2435
Samuel Susla
Valentin Shergin
Andres Suarez
zhongwuzw
Peter Argany
Kevin Gozali
Adam Ernst
Emily Janzer
Eli White