Summary:
## Motivation
With the bridge, every call into JS flushes the queue of NativeModule calls. Fabric bypasses this mechanism, because it uses a RuntimeExecutor that schedules work directly on the JavaScript thread. This diff makes Fabric's RuntimeExecutor also flush the queue of NativeModule calls.
This likely won't fix anything in Fabric, because we don't execute any async NativeModule calls on Fb4a. However, this is necessary for the drainMicrotask work we're doing in D27729702 (7310847758), because (1) we need to drain the Hermes microtask queue on every call from C++ -> JavaScript (2) we drain the microtask queue [inside JSIExecutor::flush()](de477a0df6/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp (L427),L445).
Changelog: [Android][Fixed] - Flush JSIExecutor in Fabric's RuntimeExecutor
Reviewed By: JoshuaGross
Differential Revision: D27975839
fbshipit-source-id: 27f031fb36593253da116a033e30998475eb1473
Summary:
By the time that we call [ReactContext.assertOnNativeModulesQueueThread()](https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java?commit=747a25280435d276fb975ccfe36fd5e60254c4e4&lines=355%2C359), ReactContext.mNativeModulesMessageQueueThread must be non-null. This implies that two things must have happened:
1. We initialized the ReactContext
2. After initialization, ReactContext.mNativeModulesMessageQueueThread must be non-null.
According to T85807990, ReactContext.mNativeModulesMessageQueueThread is null. Since ReactContext doesn't ever write to ReactContext.mNativeModulesMessageQueueThread aside from during initialization, it must mean that we either didn't initialize properly, or we initialized, but set the NativeModules thread to null. This diff throws IllegalStateExceptions inside ReactContext initialization, which should help narrow down the crash in T85807990.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D27729355
fbshipit-source-id: e39030b7db8862ae76fb644efaafb382a79b8ad0
Summary:
D27682424 (ea1ff374de) updated how animated node batches are executed in Fabric. On Paper, these batches were controlled by native module in some places (batch was executed ~every 2 frames), but some animations were switching animation batching control to JS globally there as well.
This change updates two things:
- If batching is controlled by native, it makes sure batches are calculated correctly.
- At the same time, this change switches control for animation node batching to JS, aligning it with Fabric.
Changelog: [Internal]
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D27939659
fbshipit-source-id: d6251bce2a303a4a007dc10297edc0175cc4b348
Summary:
Noticed while working in MobileHome with android device, when interacting with the Tasks change progress/priority components (MobileHomeTasksDetailsSelectorToken), which provides `borderRadius` style and `backgroundColor: ifSelected ? value : null`, and when `backgroundColor` is `null`, the line changed in this diff crashes (throwing the `NoSuchKeyException` at `ReadableNativeMap:110` [because of isNull check on `ReadableNativeMap:107`])
Changelog:
[Android][Fixed] - Fixed crash when using style borderRadius: any with backgroundColor: null
Reviewed By: JoshuaGross
Differential Revision: D27932828
fbshipit-source-id: 801b04c856ee9dc5a36bbf3e6e3d81de9b1e81a1
Summary:
This diff replaces all usages of int by int32_t. This is to ensure we always use a fixed size for int that matches what's expected on Java.
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27915608
fbshipit-source-id: 634c45796dda1d4434c3ad6ff3e199931c22940b
Summary:
Adds surface id for string logs of the IntBufferBatchMountItem to help debug updates with multiple surfaces.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D27884232
fbshipit-source-id: c5ec65585830f7aa5b902603bcd1e91b61cfe4c1
Summary:
Refactor MapBufferBuilder to use int to store size of Mapbuffer data
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D27904646
fbshipit-source-id: 6b8b96fdd30184b6d35c1d612743eae653854d6d
Summary:
DynamicData can contain a big amount of data, refactoring type to use int instead of short
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27904643
fbshipit-source-id: 157064b280e27a9c7c4a4f55af310392b178feda
Summary:
Add extra asserts and early deallocation in ReadableMapBuffer::importByteBufferAllocateDirect
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27904645
fbshipit-source-id: 075a007c4ec5e005b839add054bd68c233b65801
Summary:
This diff fixes the importByteBufferAllocateDirect method.
This was tested enabling importByteBufferAllocateDirect in ReadableMapBuffer.java
changelog: [internal] internal
Reviewed By: sammy-SC
Differential Revision: D27867055
fbshipit-source-id: 9ef5e93ff6c7903782598dde1c499daa82cd467b
Summary:
To fix [the CI error](https://app.circleci.com/pipelines/github/facebook/react-native/8708/workflows/d584348e-941f-4653-96c2-46375894dfaa/jobs/196410)
There are two errors:
`error: undefined reference to '__android_log_write'` which solved by adding `-llog` linker flag.
`ld: error: cannot find -lc++` which workaround by static linking.
For the root cause, I am thinking that is after [NDK r19](https://developer.android.com/ndk/guides/other_build_systems),
the `-target` should specificy api level.
However, buck does not add this accordingly, e.g. `-target armv7-none-linux-androideabi`
Given wrong target will make NDK to have `cannot find -lc++` error.
The workaround is to use static linking.
Since it was an oss_cxx_library, the change should not have impact to Facebook internal testing.
## Changelog
[Internal] [Fixed] - Fix CI "Build Tests: Android Instrumentation Tests" errors
Pull Request resolved: https://github.com/facebook/react-native/pull/31352
Test Plan: Make CI green
Reviewed By: JoshuaGross
Differential Revision: D27757838
Pulled By: fkgozali
fbshipit-source-id: 8f9c80a89c6240938218abacb8a82e3e2e71adbc
Summary:
Changelog: [Internal]
Inverts registration of a SurfaceHandler with the scheduler: instead of passing a scheduler to the SurfaceHandlerBinding, we can now query the SurfaceHandler and register it in place.
Reviewed By: JoshuaGross
Differential Revision: D27624541
fbshipit-source-id: db5d7f1375fad72a805309a3fcd5a33080e4a4a7
Summary:
Changelog: [Internal]
Links APIs in Fabric and Venice to create a surface without a view and mount it separately when surface is started the usual way.
Reviewed By: mdvacca
Differential Revision: D27339365
fbshipit-source-id: d1b674ce856957465eb6f3a5d7f26eb0ab625353
Summary:
Changelog: [Internal]
`NativeAnimatedModule` on Android currently enforces all animation operations to be processed in batches to ensure that all associated operations are processed at the same time.
Some operations, however, can be triggered outside of the batching calls (e.g. when using `Animated` for tracking touches `PanResponder`), and they are not processed until the next batch.
This change tracks if we are currently processing a batch and doesn't assign a batch number if an operation was triggered outside of `startOperationBatch`/`finishOperationBatch` pair.
Reviewed By: mdvacca
Differential Revision: D27682424
fbshipit-source-id: 2ea8737c353c81557fa586b15aa5760db3e8813f
Summary:
This diff moves DisplayMode out of SurfaceHandler, this is necessary in order to use it from react/uimanager package
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D27669846
fbshipit-source-id: 274869d8f2907b1b159f51240440acece09a746f
Summary:
Extends https://github.com/facebook/react-native/pull/30694 to fix tests.
OkHttp v4 was released almost a year ago. Even though v3 is still receiving security and bug fixes, most of the new improvements and features are landing in v4. This PR bumps OkHttp from v3 to v4 and addresses backward-incompatible changes.
Side effects of this upgrade:
- OkHttp v4 depends on Kotlin's standard library, so react-native will have a transitive dependency on it.
- The dex method count of test apk has exceeded the maximum, so multidexing had to be enabled for android tests.
## 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
-->
[Android] [Changed] - Bumping OkHttp from v3 to v4.
Pull Request resolved: https://github.com/facebook/react-native/pull/31084
Test Plan: Automated (relying on the test suite) and manual testing.
Reviewed By: fkgozali
Differential Revision: D27597430
Pulled By: ShikaSD
fbshipit-source-id: 967379b41c2bcd7cfd4083f65059f5da467b8a91
Summary:
## Rationale
- AsyncDevSupportManager.loadSplitBundleFromServer() is an override of DevSupportManager.loadSplitBundleFromServer(), which is used by the bridge. However, AsyncDevSupportManager.loadJSBundleFromServer() has no bridge analogue. This is confusing: Are the methods in AsyncDevSupportManager Venice overrides for bridge related methods? It's easy to think yes, but the answer is no.
- AsyncDevSupportManager.loadJSBundleFromServer() is an additional layer of indirection that provides very little value: all it does it create the JSBundleLoader, and call onReactContextCreated. However, it does so in 11 lines of very confusing code.
A discussion we don't have to have now: Inheritance hierarchies are very difficult to understand and de-tangle. So, instead of using inheritance to make DevSupportManager work with Venice (via AsyncDevSupportManager), should we just refactor DevSupportManager so that it can be customized to work with Venice?
Changelog: [Internal]
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D27577591
fbshipit-source-id: b64dcd65e9a7c85b89443d860d441a0635547916
Summary:
Changelog: [Internal]
After D27016919, some mounting instructions can be executed before the view is attached, which is invalid. This change adds additional queue for such items, which can be later dispatched after view is ready.
The new queue is expected to be empty for usual rendering and used in prerendering flows only. In case of prerendering, it should only hold intermediate items between early start of `SurfaceHandler` and view attach.
Reviewed By: mdvacca
Differential Revision: D27291706
fbshipit-source-id: f383c1d0d7050f271993553b51bf2e387efe1e9e
Summary:
Changelog: [Internal]
Adjusts SurfaceMountingManager to allow starting surface without a view and attaching the view while surface is active.
This change is only meant to work with ReactSurface to allow creating `SurfaceHandler` without mounting views.
Reviewed By: mdvacca
Differential Revision: D27016919
fbshipit-source-id: 995383bc4f1bd298953516007743ffae0edf17c2
Summary:
Changes React Native so that when `accessibilityState` is used to change a view from `selected: true` to `selected: false`, the change in state is announced.
This is how `checked` works; it is unclear why Android does not do this for `selected`, too.
Changelog:
[Android][Added] - TalkBack now announces "unselected" when changing `accessibilityState.selected` to false.
Reviewed By: blavalla
Differential Revision: D27449293
fbshipit-source-id: a6d77b55d63655973ad93c4d5e3743742501f378
Summary:
This issue fixes https://github.com/facebook/react-native/issues/30955 and is a follow up to pr https://github.com/facebook/react-native/pull/24608 which added the basic Accessibility functionalities to React Native.
TextInput should announce "selected" to the user when screenreader focused.
The focus is moved to the TextInput by navigating with the screenreader to the TextInput.
This PR adds call to View#setSelected in BaseViewManager https://developer.android.com/reference/android/view/View#setSelected(boolean)
The View#setSelected method definition https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/view/View.java
```java
/**
* Changes the selection state of this view. A view can be selected or not.
* Note that selection is not the same as focus. Views are typically
* selected in the context of an AdapterView like ListView or GridView;
* the selected view is the view that is highlighted.
*
* param selected true if the view must be selected, false otherwise
*/
public void setSelected(boolean selected) {
if (((mPrivateFlags & PFLAG_SELECTED) != 0) != selected) {
// ... hidden logic
if (selected) {
sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_SELECTED);
} // ... hidden logic
}
}
```
VoiceOver and TalkBack was tested with video samples included below.
## 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
-->
[Android] [Fixed] - Fix Selected State does not announce when TextInput Component selected on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/31144
Test Plan:
**<details><summary>CLICK TO OPEN TESTS RESULTS</summary>**
<p>
**ENABLE THE AUDIO** to hear the TalkBack announcing **SELECTED** when the user taps on the TextInput
```javascript
<TextInput
accessibilityLabel="element 20"
accessibilityState={{
selected: true,
}} />
```
| selected is true |
|:-------------------------:|
| <video src="https://user-images.githubusercontent.com/24992535/111652826-afc4f000-8807-11eb-9c79-8c51d7bf455b.mp4" width="700" height="" /> |
```javascript
<TextInput
accessibilityLabel="element 20"
accessibilityState={{
selected: false,
}} />
```
| selected is false |
|:-------------------------:|
| <video src="https://user-images.githubusercontent.com/24992535/111652919-c10dfc80-8807-11eb-8244-83db6c327bcd.mp4" width="700" height="" /> |
The functionality does not present issues on iOS
| iOS testing |
|:-------------------------:|
| <video src="https://user-images.githubusercontent.com/24992535/111647656-f401c180-8802-11eb-9fa9-a4c211cf1665.mp4" width="400" height="" /> |
</p>
</details>
</p>
</details>
Reviewed By: blavalla
Differential Revision: D27306166
Pulled By: kacieb
fbshipit-source-id: 1b3cb37b2d0875cf53f6f1bff4bf095a877b2f0e
Summary:
Previously I renamed hasActiveCatalystInstance() API in D27335055 (dfa8eb0558), however this API is still used in an OSS class.
In this diff hasActiveCatalystInstance() is added back and marked as Deprecated to avoid breakages and leave the migration choice to OSS users.
Changelog:
[Android][Changed] Mark hasActiveCatalystInstance() as Deprecated
Reviewed By: yungsters
Differential Revision: D27538449
fbshipit-source-id: 30f2f890580ad7f8b41908e18013234b5bac72e9
Summary:
JSI callbacks are only destroyed if the callback is called. If the callback is never called, we're potentially leaking a lot of callbacks.
To mitigate this, we add a wrapper object that is owned by the std::function. Whenever the std::function is destroyed, the wrapper is destroyed and it deallocates the callback as well.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D27436402
fbshipit-source-id: d153640d5d7988c7fadaf2cb332ec00dadd0689a
Summary:
D26581756 (86321a35c0) was hacked for fixing T85822390, which was later properly fixed in D27369721, so I removed it in this diff.
Changelog:
[Android][Fixed] - Remove previous fix for "Fix text in ReactTextView sometimes being vertically displayed"
Reviewed By: mdvacca
Differential Revision: D27501405
fbshipit-source-id: 81417069d355936721868ce659b3fe8ce916302e
Summary:
Changelog: [Internal] enable support for C++ 17.
C++ 17 in React Native targets.
Short and comprehensive list of C++ features:
https://github.com/AnthonyCalandra/modern-cpp-features#c17-language-features
Reviewed By: JoshuaGross
Differential Revision: D27431145
fbshipit-source-id: e8da6fe9d70e9b7343a8caec21cdbeb043478575
Summary:
Sometimes ```hasActiveCatalystInstance()``` is used to check if it's safe to access the CatalystInstance, which will still crash in Venice.
Previously we mitigate this by changing ```reactContext.hasActiveCatalystInstance()``` to ```reactContext.hasActiveCatalystInstance() || reactContext.isBridgeless()```.
To solve this for all and good the plan is:
1, Rename ```hasActiveCatalystInstance()``` to ```hasActiveReactInstance()``` so it won't sounds like CatalystInstance-only.
2, Implement hasActiveReactInstance() for Venice. D27343867
3, Remove previous mitigation. D27343952
This diff is the first step, by xbgs there are **58** non-generated callsites of ```hasActiveCatalystInstance()``` in code base which are all renamed in this diff.
Changelog:
[Android][Changed] - Rename "hasActiveCatalystInstance" to "hasActiveReactInstance"
Reviewed By: mdvacca
Differential Revision: D27335055
fbshipit-source-id: 5b8ff5e09b79a492e910bb8f197e70fa1360bcef
Summary:
`InternalNode` will eventually not have a pointer to its parent. This diff removes one of the usages of the `InternalNode#getParent()` API. `InternalNode` will also not host the `YogaNode` eventually; so this diff also removes one of the usages of the `InternalNode#getYogaNode()` api.
Now the `Inputs#freeze` api will pass the parent's `YogaNode` and the `YogaNode` of the node (this) being measured.
Changelog: [Internal] Passes The YogaNode and parent YogaNode in the Inputs.freeze API
Reviewed By: SidharthGuglani
Differential Revision: D27240229
fbshipit-source-id: efc4ec3249a963c3181111f9b989d8ed9e17feb4
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:
Create YogaProps Interface; this interface represents the inputs to YogaNode for layout calculation.
Changelog: [Internal] Create YogaProps Interface; this interface represents the inputs to YogaNode for layout calculation.
Reviewed By: mihaelao
Differential Revision: D27229274
fbshipit-source-id: 5205caf2384661369d7a2d7e7f3e49ff831a1c92
Summary:
Instead of annotating individual methods with DoNotStrip, we actually want to ensure that nothing in this class gets stripped out.
This also upgrades the yoga/proguard-annotations package for Gradle builds.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D27335181
fbshipit-source-id: 5b696c26faf4d1b32a3fd885e42a996aff23f0be
Summary:
android fbjni dependency bump to 0.2.2
The motivation is to address https://github.com/pytorch/pytorch/issues/34682 that pytorch_android and react-native use different versions of fbjni with the same name libfbjni.so
In case of loading 0.0.2 version it is missing functions in binary
Fbjni is not changing much, that's why this will solve that problem for a long time (Until fbjni changed and versions will not be aligned)
## Changelog
[Android][Added] fbjni version bump to 0.0.3
Pull Request resolved: https://github.com/facebook/react-native/pull/31191
Test Plan: Automated (relying on the test suite) and manual testing.
Reviewed By: passy
Differential Revision: D27330370
Pulled By: IvanKobzarev
fbshipit-source-id: 2ea07d80d23f8dbc80e946a8818c1ecb8eb746e8
Summary:
Changelog: [Internal]
FabricUIManager contains a lot of logic related to mounting items and manipulating dispatch queues, which can be safely extracted outside. Apart from decreased logical complexity, this change enables easier modification of the queuing behavior later in this stack.
Majority of the changes is caused by moving existing logic to a different class, no change in behavior expected.
Reviewed By: JoshuaGross
Differential Revision: D27271116
fbshipit-source-id: 86fe45b19bb839f96fde8ba607f72006f6401cc7
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:
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:
Media picking wasn't working for Venice because we didn't implement onActivityResult in BridgelessReactFragment so the listener in FBProfileGemstoneReactModule didn't called.
Changelog:
[Android][Changed] - Added Nullable annotation
Reviewed By: mdvacca
Differential Revision: D27045861
fbshipit-source-id: 0ab2961ef0570d92259856b4132507ebb264eb9d
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:
In DevSupportManagerBase.java->updateLastErrorInfo(), errorType was not recorded like errorMessage and errorStack, we could either remove errorType as a parameter or recorded it for future use. This diff recorded it since it would make the error info complete.
Changelog:
[Android][Changed] - Record latest error type in dev support
Reviewed By: PeteTheHeat
Differential Revision: D26884647
fbshipit-source-id: 712d82667bdc4b3410f4c83a3df9a456af6d9061
Summary:
Changelog: [Internal]
We were using RN util to get pixel density, but it depends on the surface being created after venice instance is initialized. Given that we have context every time we update constraints, it makes sense to use it directly.
Reviewed By: mdvacca
Differential Revision: D26959430
fbshipit-source-id: 78701786efd82857812df689a725ba094fbd226e
Summary:
Just adding more logs I found useful while playing around with the last diff (to verify that these methods were not actually implicated at all).
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26972725
fbshipit-source-id: 0e048e1edbfbe5ed32c5277f17a7197e0afdc04f
Summary:
When the height of a HorizontalScrollView changes and there is a `layout` event, it can cause the underlying platform View code to scroll slightly to the right.
I'm... not really sure why, even after looking at the View code for a while. But it is clearly detectable and mirrors issues with RTL that were fixed recently.
This might warrant more investigation, but I believe the fix is relatively safe - we detect if there's an autoscroll only if the height changes and only if the scroll happens in "layout". That scopes the hack pretty well to just this bug.
There aren't really times when we actually want layout to scroll to the right, so... I think this is reasonable.
Changelog: [Changed][Android] Fixed issue that causes HorizontalScrollView to shift to the right when a TextInput is selected and keyboard pops up
Reviewed By: mdvacca
Differential Revision: D26972710
fbshipit-source-id: 441b1a3f07b9b68195a9e5e9a0c8d75c9d24a109
Summary:
Does not impact prod or even debug builds unless you switch on the debug flag. The issue is that we were trying to coerce booleans to integers.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D26970274
fbshipit-source-id: 3327029ae3afc307dd19b089c23c190cb9e3150c
Summary:
This NativeModule will now be type-safe, and TurboModule-compatible.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D26956332
fbshipit-source-id: 6651a003c70819934869dd6a8c664ef984d0efcb
Summary:
Create MC to gate execution of JS Responder in Fabric Android
MC.react_fabric.enable_js_responder_fabric_android is enabled by default in the server
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26905296
fbshipit-source-id: 82504174394d1e10fd017435cccd38952404fda0
Summary:
We can override the `scrollTo` method and it's likely/possible that Android internals are calling scrollTo directly. So, we can capture
more cases where the scroll position is changing and needs to be updated in Fabric State.
Unfortunately we still cannot override smoothScrollTo because it is marked as final. For now we just keep the custom `reactSmoothScrollTo` method
and hope that we can catch more cases with `scrollTo`.
Changelog: [Internal]
Reviewed By: sammy-SC, mdvacca
Differential Revision: D26887028
fbshipit-source-id: e2678f1a20640d598abbec9671d6102635f65bb2
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:
Followup to D26858584 (00959ffd6b). We should also immediately destroy C++ state memory (which will decrement a shared_ptr) when deleting a view.
This could improve memory while a surface is being used.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D26876537
fbshipit-source-id: fc8353bed47db8fdbf5c7f6c6253ac788c460d9a
Summary:
NativeModules have an initialize() method that they use to allocate any resources, set up listeners, etc. This diff imports that method into the TurboModule interface. This way, we don't have to cast TurboModules to NativeModules to initialize them. Also, it makes sense to import this initialization mechanism into the TurboModule infra.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D26871552
fbshipit-source-id: b8ae515b22928ed678b4003096e0756e991e10ff
Summary:
This diff migrates all NativeModules away from onCatalystInstanceDestroy() to the invalidate() method.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D26871595
fbshipit-source-id: 132f6b75e485361835769a2b53bc742eefa47b59
Summary:
## Rationale
The CatalystInstance is going away after we delete the bridge. So, we should migrate away from onCatalystInstanceDestroy() to something else: invalidate().
## Changes
- Introduce the NativeModule.invalidate() cleanup hook.
- Both the NativeModule and TurboModule infra now call this invalidate() method, **as opposed to** onCatalystInstanceDestroy(), to perform NativeModule cleanup.
- **Is this safe?** All our NativeModules extend BaseJavaModule. BaseJavaModule.invalidate() delegates to NativeModule.onCatalystInstanceDestroy(), so NativeModules that implement onCatalystInstanceDestroy(), but not invalidate(), still have their onCatalystInstanceDestroy() method called.
Changelog: [Android][Deprecated] - Deprecate NativeModule.onCatalystInstanceDestroy() for NativeModule.invalidate()
Reviewed By: JoshuaGross
Differential Revision: D26871001
fbshipit-source-id: e3bdfa0cf653ecbfe42791631bc6229af62f4817
Summary:
When debugging this class a lot, I found it helpful to have these logs and it would have been nice if they were here already - I had to rewrite these several times.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26836318
fbshipit-source-id: 08eb9ae19923fc593d1aba031586a02a193d6b2d
Summary:
changelog: [internal]
There were three separate problems preventing measure infra to work correctly with views inside horizontal scroll view in RTL environment.
1. Initial offset is wasn't communicated to Fabric. This is resolved separately as it doesn't affect only RTL: D26778991 (630ac87591).
3. On Android when layout direction is RTL, offset of scrollview is calculated from right.
Reviewed By: mdvacca
Differential Revision: D26779860
fbshipit-source-id: 61572c78091a1f5417102eb38d88ba7d172e6102
Summary:
Whenever layout updates in a horizontal scrollview, in RTL mode we adjust the position - the impact *should* be that initially, we jump from position 0 to the right side of the scroll view,
such that scrolling starts from the right.
However, we were doing this entirely too aggressively before. We should only make this adjustment *if the layout changes the width*.
Changelog: [Android][Changed] Fixed jumpy RTL horizontal ScrollViews. If you have Android-specific JS hacks for handling RTL in ScrollViews, you probably can/probably want to remove them, because they should be reliable now and require fewer hacks.
Reviewed By: mdvacca
Differential Revision: D26771366
fbshipit-source-id: de11bd1cae1414018d88ce44b3583a8b15f3b330
Summary:
There are races between BackgroundExecutor and Fabric/View teardown, where, because of the way things are set up currently, a View will strongly retain a pointer into some native State object which can keep a host of other objects alive in C++, even after stopSurface, and even after RN itself starts tearing down.
To alleviate this, we more aggressively clear State from Java, without waiting for Java GC: 1) on stopSurface, 2) whenever a State object is stale from Java's perspective.
This should allow us to keep all common updateState semantics, while only introducing a new edge-case that stateWrapper can be destroyed during mounting if stopSurface happens at the same time. In those cases, checking for NPEs should be sufficient.
The possible race condition only really happens with updateState, so it's easier to check for. There should practically be no cases where there's a race between `stopSurface` and `getState`, because `getState` is only really called around state updates and view preallocation, and never after; we still check for NPEs in those cases, but it shouldn't be an issue.
Changelog: [Internal]
Reviewed By: thurn, sammy-SC, mdvacca
Differential Revision: D26858584
fbshipit-source-id: 2ef7467220865380037d69d8de322fe8797f6a12
Summary:
If modules are *not* eagerly init'd and expect lifecycle events, make sure (1) onHostResume is called immediately it it's currently active and (2) that listeners are removed in onCatalystInstanceDestroy.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26859161
fbshipit-source-id: 654c055c53c0e420c6d9f2b0135055aec34269c9
Summary:
If modules are *not* eagerly init'd and expect lifecycle events, make sure (1) onHostResume is called immediately it it's currently active and (2) that listeners are removed in onCatalystInstanceDestroy.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26859157
fbshipit-source-id: 754a5b4ede8defa8b7742cc42e09cc7cbfe4e18d
Summary:
If modules are *not* eagerly init'd and expect lifecycle events, make sure (1) onHostResume is called immediately it it's currently active and (2) that listeners are removed in onCatalystInstanceDestroy.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26859160
fbshipit-source-id: ce84deafd1f20d1680d333d1a176b0493623a4ee
Summary:
If modules are *not* eagerly init'd and expect lifecycle events, make sure (1) onHostResume is called immediately it it's currently active and (2) that listeners are removed in onCatalystInstanceDestroy.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26859205
fbshipit-source-id: 5398d24d2592de3fbb80ca59192b5b46543aa5c5
Summary:
If modules are *not* eagerly init'd and expect lifecycle events, make sure (1) onHostResume is called immediately it it's currently active and (2) that listeners are removed in onCatalystInstanceDestroy.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26859159
fbshipit-source-id: 8e47cedd4b218a47b33d1209f3ede2fd1531015d
Summary:
If modules are *not* eagerly init'd and expect lifecycle events, make sure (1) onHostResume is called immediately it it's currently active and (2) that listeners are removed in onCatalystInstanceDestroy.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26859158
fbshipit-source-id: 4966d3c49d194c4cb4063edf3a035f6077b76cd9
Summary:
I'm not sure what broke this, but in some cases, DialogModule doesn't get subscribed to LifecycleEventListener events.
This seems to fix it.
Changelog: [Internal]
Reviewed By: mdvacca, RSNara
Differential Revision: D26856016
fbshipit-source-id: 868baf102b85b202180adcbb8bb181dfe603188f
Summary:
Found this crash when rendering ReactEditText under Venice, from comment it's supposed to be called only in Paper, so I adde a Venice check to avoid calling this method.
{F446844248}
Changelog:
[Android][Changed] - Add a new check to avoid calling this method
Reviewed By: mdvacca
Differential Revision: D26781457
fbshipit-source-id: f4c2e890156a37e35aa153c736b50924254e67bc
Summary:
Ship responsibility for most View creation logic to ViewManager, where it already largely lies, and simplify code in Fabric and non-Fabric mounting layers.
Notably, some of this work was *already* being duplicated so we can expect an extremely tiny perf gain here.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26742711
fbshipit-source-id: 4213766d4cd366bc69cd47d4654f7b269bb9e7f4
Summary:
https://github.com/facebook/react-native/issues/31051
When trying to create custom viewmanagers, we don't have props, I have a use case where I want to create views on the basis of provided react prop from react native.
## 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
-->
[Android] [Changed] - pass initial props to ViewManager createViewInstance method in non-Fabric
Pull Request resolved: https://github.com/facebook/react-native/pull/31053
Test Plan:
Tested Manually.
(Facebook - see D26719538, which should land first)
Reviewed By: mdvacca
Differential Revision: D26719510
Pulled By: JoshuaGross
fbshipit-source-id: ced78aa919e6b433e22ddb7c9eccc3e3e91950e9
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:
setRemoveClippedSubviews in ReactHorizontalScrollContainerView.java in RTL mode is overzealous and unexpectedly clips out views in a way that is not desirable.
It seems like what is actually happening is that the computed rect for the view is "0,0" and so contents are assumed to always be outside of this rect.
For now I've disabled this feature. We can investigate as a followup.
Changelog: [Android][Changed] Clipping subviews has been temporarily disabled in HorizontalScrollView in RTL mode. Minor/negligible perf impact.
Reviewed By: sammy-SC
Differential Revision: D26808937
fbshipit-source-id: 85af9c3fb542db9ca3aae03413a475695cd53391
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:
This diff integrates the ComponentNameResolver class into ReactInstanceManager
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26716900
fbshipit-source-id: e3a5f44485f659a32bf6094eee7985daf634f50f
Summary:
This diff introduces the ComponentNameResolverManager and ComponentNameResolver classes. The purpose of these classes is to integrate NativeComponentRegistryBinding into RN Android
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26716899
fbshipit-source-id: c62fb5c38ddce5325890d2506a6fb17d26043175
Summary:
Changelog: [internal]
Calling `scrollTo` does not report offset change to Fabric core and measure infra can't compute correct values. This results in unresponsive buttons if horizontal scroll view on Android has initial offset set to anything besides default value 0.
Reviewed By: JoshuaGross
Differential Revision: D26778991
fbshipit-source-id: 5cad5cb9926c7923f6efcd56cb4e15c3b958c245
Summary:
## Summary
Bump Android compileSdkVersion and targetSdkVersion to 30
## Changelog
[Android][Changed] Bump Android compileSdkVersion and targetSdkVersion from 29 to 30
Pull Request resolved: https://github.com/facebook/react-native/pull/31078
Test Plan: Circle CI and Sandcastle
Reviewed By: mdvacca
Differential Revision: D26765188
Pulled By: hramos
fbshipit-source-id: a971641cea4860df58ce6e9b0f14405bfc4e0979
Summary:
Simplify addLifecycleEventListener for the flaky test because we just want to test that listener is working.
Changelog:
[Android][Changed] - Add a spare implementation of addLifecycleEventListener for test purpose.
Reviewed By: PeteTheHeat
Differential Revision: D26749256
fbshipit-source-id: 5af216e6bfa37a15eb189aa24a3df35a7a7112de
Summary:
This diff removes an unnecessary dependency from react buck module
This was causing a regression in apk size in IG
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26755329
fbshipit-source-id: bc45d9717bb0343cd26ed2ccbaa016b55f56b9bf
Summary:
Timestamp is computed differently now and uses system millis as the basis for a monotonic clock. Updating this fixes tests.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D26739611
fbshipit-source-id: 4908da68e1c126ea2b0772aaf408d892798549aa
Summary:
This diff moves the method getViewportOffset out of ReactRootView. This is necessary to avoid Fabric to depend from paper.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26716901
fbshipit-source-id: cec67c24860a776fb361d7cda08d3142e1214c8c
Summary:
In Fabric we're seeing setJSResponderHandler called during teardown of a surface, which causes a crash because the SurfaceId is no longer available at that point.
Guard against setJSResponderHandler being called on a dead surface.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26734786
fbshipit-source-id: 838d682ee0dd1d4de49993fa479dc2097cf33521
Summary:
We want to be able to instrument touch processing delays in JS, which does not have access to systemUptime; therefore we want a UNIX timestamp, which JS has access to and can compare to the touch time.
It only matters that there is relative consistency between multiple touch events in JS, which is still the case; so this should have no impact on product code.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D26705429
fbshipit-source-id: 0f2db726048fcab9a30e830970d7d8a8d2eae446
Summary:
I thought we'd need LayoutDirection on the platform at some point, but it turns out we never use it, and don't seem to need it since components just query via `I18nUtil` one time,
and it's expected that apps restart if language changes and we need to switch between LTR and RTL. So, it seems like we'll not have any need for this on the platform at any point.
And we can save a byte per layout instruction now. Huzzah!
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26660908
fbshipit-source-id: 54c7d132f5fa260a93fc7f09f7cf63059d52ed1f
Summary:
We recently fixed RTL scrolling in Fabric on iOS: D26608231 (e5921f7f38)
Turns out, the mechanism for RTL scrolling on Android is completely different. It requires that content be wrapped in a "directional content view", which is `View` in LTR and `AndroidHorizontalScrollContentView` in RTL, backed by `ReactHorizontalScrollContainerView.java`.
iOS doesn't require that and just uses View and some custom logic in ScrollView itself.
In the future it would be great to align the platforms, but for now, for backwards-compat with non-Fabric and so we don't have to tear apart ScrollView.js, we codegen the AndroidHorizontalScrollContentView so it exists in C++, register the component, and stop mapping it to View explicitly in C++.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26659686
fbshipit-source-id: 3b9c646dbdb7fe9527d24d42bdc6acb1aca00945
Summary:
Changelog: [Internal]
Adds react_debug dependency in Android.mk where it was missing
Reviewed By: mdvacca
Differential Revision: D26617400
fbshipit-source-id: 5ac799269b106eadd881d30490ac34bd2134a9b7
Summary:
## Issue:
Sometimes ReactTextView are vertically displayed as one column with bridgeless.
## Root cause:
After debugging, I found out this is caused by a workaround in 2016 to fix a crash caused by mLayout occasionally being non-null and triggers relayout during setText.
https://github.com/facebook/react-native/pull/7011
## Fix
Revert previous hack, if the crash happens again I'll try to fix it.
Changelog:
[Android][Fixed] - Fix text in ReactTextView sometimes being vertically displayed
Reviewed By: mdvacca
Differential Revision: D26581756
fbshipit-source-id: a373d84dc1ab3d787bda7ec82f2d0865a354cf60
Summary:
The purpose of this change is to make TTRC markers work similarly for bridge loading and bridgeless loading so we could compare performance between them.
There are mainly four cases involved:
```REACT_BRIDGE_LOADING_START,
REACT_BRIDGE_LOADING_END,
REACT_BRIDGELESS_LOADING_START,
REACT_BRIDGELESS_LOADING_END
```
First 2 are for beginning/ending of bridge loading which includes creating fragment, loading JS bundle, running JS bundle and creating react instance. Last 2 are for similar purpose with bridgeless.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D26514499
fbshipit-source-id: 65d6f3cc7de9e07a7a3a802dd77138e74c23aa5b
Summary:
Change lifecycle of stopSurface in a subtle way: mark the surface as stopped in Java first, then in Cxx (currently it happens in Cxx, then Java).
This will cause us / allow us to ignore the final mounting instructions for the Surface, which are all irrelevant since they just have to do with View removal.
We can rely on GC and `unmountReactApplication` to do all of this for us, and save some CPU cycles on stopSurface.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D26469741
fbshipit-source-id: a7f81d44c3cb2138f0ab31feb38852910410c638
Summary:
If there's a JS/native crash and the surface is stopped, the ReactRootView could be reused when the surface is restarted (or potentially to show the errors in a logbox; I'm don't think that's possible, but not 100% sure).
Regardless, now it will crash in those cases because of T83802049. There's no reason to do that, so just mark the ReactRootView as explicitly unused once the surface is stopped.
Changelog: [Internal]
Reviewed By: ShikaSD
Differential Revision: D26469742
fbshipit-source-id: ee79c094393a8ae8b5c53f04c14b90bc8e1a5a17
Summary:
Litho needs a new API which is called immediately before yoga begins layout calculations so that the InternalNode gets the opportunity to finalise itself; i.e. perform the last mutations and in effect avoid any more mutations to the hierarchy.
See D26373731 where the mutations from `Layout#collectResults` is moved back into the InternalNode.
Changelog: [Internal] Adds new API to YogaNodeJNIBase
Reviewed By: SidharthGuglani
Differential Revision: D26373730
fbshipit-source-id: 471346d3444986ada91e86c95f5f9fb98bcd2fa6
Summary:
Right now the FbReactInstanceDelegate provides a list of view managers to the instance during initialization, this means that we're basically eagerly loading all of the view manager classes.
In this change we use ViewManagerResolver instead.
Changelog:
[Android][Changed] - Move ViewManagerResolver into a seperate file
Reviewed By: mdvacca
Differential Revision: D26424214
fbshipit-source-id: 550ade31c256a56d6e32c463f112ea16dd55a218
Summary:
Now we use SurfaceHandler-based APIs to control surfaces in `Binding.cpp` on Android instead of using Scheduler-based APIs.
This is a transitional change; eventually, we will need to wrap C++ SurfaceHandler's into JNI wrappers. For now, it will allow to clean up the C++ part.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D26375641
fbshipit-source-id: 6f293e79cecf50de72294e90d5243ebb02d71236
Summary:
This bug doesn't reproduce anymore in v301+. MC has been enabled since december
https://www.internalfb.com/intern/logview/details/facebook_android_javascripterrors/419f8892e7b1a02f205810219ddfc299/trends?selected-logview-tab=All%20Traces&drillstate={%22start%22:%22Thu,%2028%20Jan%202021%2000:59:54%20-0800%22,%22end%22:%22Thu,%2011%20Feb%202021%2000:59:54%20-0800%22,%22constraints%22:[{%22col%22:%22mid%22,%22op%22:%22==%22,%22vals%22:[%22419f8892e7b1a02f205810219ddfc299%22]}],%22context%22:%22facebook_android_javascripterrors%22,%22metric%22:%22count%22}
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D26398484
fbshipit-source-id: ca85ca211f1a38aa2691f150956a27c878d243bc
Summary:
TurboModuleManager used to be provided by a JSIModules package. In D26193053 (13f100f788), we moved TurboModuleManager creation off JSIModules. However, we didn't move the creation outside the JSIModules guard. So, when there were no JSIModules registered, we simply wouldn't create the TurboModuleManager. This diff fixes that mistake.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26380674
fbshipit-source-id: 66939d11205b1d2eccd8c3b59ca4782e90645cd9
Summary:
Building ReactAndroid from source on Windows has recently hit the limitation of maximum path lengths.
At build time, during the `:ReactAndroid:buildReactNdkLib` task, the linker tries to access several of the intermediate binaries located deep in the tmp folder hierarchy, eg.
```
D:\r\ReactAndroid\build\tmp\buildReactNdkLib/local/armeabi-v7a/objs/react_render_components_progressbar/D_/r/ReactAndroid/__/ReactCommon/react/renderer/components/progressbar/android/react/renderer/components/progressbar/AndroidProgressBarMeasurementsManager.o
```
**Suggested fix:** for modules such as `react_render_components_progressbar` and `react_render_components_picker`, rename them to `rrc_progressbar` etc.
**NOTE**: this assumes that the fix from https://github.com/facebook/react-native/issues/30535 is in place. This regression happened while https://github.com/facebook/react-native/issues/30535 has been pending checkin.
**Other mitigations I've tried:**
- setting [`LOCAL_SHORT_COMMANDS`](https://developer.android.com/ndk/guides/android_mk#local_short_commands) for the problematic modules or `APP_SHORT_COMMANDS` for the root project. Turns out those commands don't work on the NDK version RN requires, but even after manually applying a [patch ](https://android-review.googlesource.com/c/platform/ndk/+/1126440) to my local copy of the NDK, these flags had no effect.
- moving the repo directory higher in the file system tree, and using short directory names `D:\r\...` was not enough
- creating virtual drive letters for specific long paths with the [`sust`](https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/subst#examples) command is not workable, since they depend on the source folder structure, and get partly generated by the build system, which I can't plug into
- just enabling long path support on Windows is not enough, since the compiler toolchain doesn't support them.
## 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
-->
[Android] [Fixed] - Fix source build on Windows machines vol. 2
Pull Request resolved: https://github.com/facebook/react-native/pull/30776
Test Plan:
Run `.\gradlew installArchives`
Before:
Now:
Differential Revision: D26194286
Pulled By: mdvacca
fbshipit-source-id: 778b5a0515148e2ace19e7902f74301831ebed94
Summary:
In addViewAt, we have this check. Add this same check to removeViewAt.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26381475
fbshipit-source-id: 1050377aa4e528668446fd561ff09c61f27c700f
Summary:
Remove out of data TODO
changelog: [internal] internal
Reviewed By: PeteTheHeat
Differential Revision: D26144400
fbshipit-source-id: c5a97ce98cd7251e40adc15c16fceed4b9c76f81
Summary:
In this diff I'm adding debug assertions to verify that there are no overflows when muptiplying layout metrics by the pointScaleFactor
Ideally these should crash the app, but I'm trying to be conservative.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26297396
fbshipit-source-id: 068c60f4d89ea9cfd04a2e2174da2043ae150928
Summary:
We have no evidence of this happening on Android, but we are hitting a similar invariant on iOS. Adding this to Android for debugging purposes.
For now it's a SoftException to catch in debug and capture information; if we don't hit this prod at all, we'll elevate to a hard crash.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26281585
fbshipit-source-id: 8ea9cf3ac555b13bf311f24c81bbbbc2845521d5
Summary:
This diff cleans listeners on the destruction of the ReactContext.
changelog: [inernal] internal
Reviewed By: JoshuaGross
Differential Revision: D26259929
fbshipit-source-id: 1843cabdac2fa3e67dcc890afd923b82472d8f66
Summary:
This diff clears the internal maps of NativeModuleRegistry during turn down of the bridge.
This is necessary for a proper cleanup of these modules.
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D26239303
fbshipit-source-id: 6e98e5db60a4f54d02e99b03339b03c17ecc183d
Summary:
This diff unregisters the UIManagerModule from LifecycleEventListener during turn down of the bridge.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26239156
fbshipit-source-id: b230949228c6e580cca088c395b970a3cff94839
Summary:
This diff setup a global variable to control the staticViewConfig experiment before the bundle is loaded.
This global variable only has a meaning when RN uses a Bridge
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D26237670
fbshipit-source-id: 25ae63f36fba9c1e640ab2e70de88b71452ad8e6
Summary:
This diff refactor the initialization and deallocation of EventDispatcher in FabricUIManager when StaticViewConfigs are enabled.
The goal of this diff is to make sure that the EventDispatcher is deallocated correctly when using StaticViewConfigs
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D26166413
fbshipit-source-id: e5bdad7ba923edc677c6b73f3a4d1271941f41cc
Summary:
This caused a few apps to crash on launch.
Changelog: [Android][Fixed] - Crashes due to "Make NativeModules immediately initializable"
Reviewed By: olegbl
Differential Revision: D26278594
fbshipit-source-id: 969a3dc49a843366c4ae6ed19a9233d1e6f39b13
Summary:
Changelog:
[Internal][Yoga] - Added instance of checks in `YogaNodeJNIBase` class to prevent `ClassCastException`s. This was happening for some NT android tests - Mocked Yoga Node object was being passed in the `addChildAt` api
Stack Trace of exception
java.lang.ClassCastException: com.facebook.yoga.YogaNode$MockitoMock$1408896622 cannot be cast to com.facebook.yoga.YogaNodeJNIBase
at com.facebook.yoga.YogaNodeJNIBase.addChildAt(YogaNodeJNIBase.java:86)
at com.facebook.litho.DefaultInternalNode.addChildAt(DefaultInternalNode.java:220)
at com.facebook.litho.DefaultInternalNode.child(DefaultInternalNode.java:377)
at com.facebook.litho.DefaultInternalNode.child(DefaultInternalNode.java:360)
at com.facebook.litho.Column.resolve(Column.java:118)
at com.facebook.litho.Layout.create(Layout.java:172)
Reviewed By: Andrey-Mishanin
Differential Revision: D26114992
fbshipit-source-id: 774a689609e67f9244b81c6788b62cd61cd96d14
Summary:
The new IntBufferMountItem queueing actually enforces a global ordering of mutation types: CREATEs, then INSERT, then REMOVE, then UPDATE, then DELETE.
See comments for more details. In general this ordering is fine, but if a DELETE animation is in progress and (due to view unflattening) the same view is recreated, the CREATE will be executed and then the in-process DELETE (since conflicting animations get flushed).
To mitigate this, in Binding we detect this and simply remove DELETE operations queued when we detect that we want to CREATE the node. `DELETE...CREATE` is valid but `CREATE...DELETE` in a single frame is not, so this is safe.
This does complicate and add more assumptions to Binding than I would like, but it should unblock us for now.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26271299
fbshipit-source-id: 9453fe17b8b541e484a047dc9637674dbdcc8e9a
Summary:
This is a Fabric-compliant implementation of `JSResponder` feature. To make it work e2e we also need to update FabricRenderer in React repository. But before we can do this, we need to ship the native changes.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D24630027
fbshipit-source-id: 70c30e1250b554d83862956b536714704093072f
Summary:
## Problem:
NativeModules can only be initialized after we mark them initializable on the NativeModules thread. This work is scheduled in ReactInstanceManager.setupReactContext(), after we schedule the execution of the JS bundle on the JavaScript thread in ReactInstanceManager.createReactContext():
https://www.internalfb.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java?commit=9b58f58b8eb12281567b8d24d6d000e868e61a1f&lines=1256-1257%2C1331%2C1334-1335%2C1333-1334
So, if the timings don't work out, the JavaScript thread could start executing the JS bundle before the NativeModule thread makes all NativeModules initializable. In this case, those NativeModule will just appear to be null in C++/JavaScript.
## Fix
This diff makes all NativeModules initializable immediately after their ModuleHolder is created. ModuleHolder.markInitializable() simply initializes initializes modules that were eagerly created.
Changelog: [Android][Fixed] - Make NativeModules immediately initializable
Reviewed By: JoshuaGross
Differential Revision: D26233372
fbshipit-source-id: a9223ff093da5b80781169be88e6ec9516c7a29b
Summary:
How does an application register a TurboModuleManagerDelegate with ReactInstanceManager?
1. Call ReactInstanceManagerBuilder.setReactPackageTurboModuleManagerDelegateBuilder(ReactPackageTurboModuleManagerDelegate.Builder)
2. Override ReactNativeHost.getReactPackageTurboModuleManagerDelegateBuilder()
Changelog: [Android][Added] - Introduce API to allow applications to register TurboModuleManagerDelegates with ReactInstanceManager
Reviewed By: mdvacca
Differential Revision: D26193055
fbshipit-source-id: bf82e63e6ab1c0c8f12bada92ac6852c992ec9cb
Summary:
Previously, owner(TurboModuleManager) used to depend on owner(ReactInstanceManager). Now, owner(ReactInstanceManager) depends on owner(TurboModuleManager).
**Rationale:** This allows ReactInstanceManager to create TurboModuleManager.
## Changes
We moved ReactPackageTurboModuleManagerDelegate to com.facebook.react.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26163661
fbshipit-source-id: 3ebff16ef2aa77e20bb55500ed44c9214acb91dd
Summary:
Crash in debug, log a warning and continue in production.
Changelog: [Internal]
Differential Revision: D26133167
fbshipit-source-id: 60279363a3e90d592e7ddbde188c13cda89c28c6
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056823
fbshipit-source-id: 1d25afb2d4cfd7e539214d4592e361260f98fc56
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056815
fbshipit-source-id: d0e03a69f84de47385e064c74f0a79c52c61022d
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056813
fbshipit-source-id: e3b0132fac6335e83ff5b1992424edcb98703803
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056808
fbshipit-source-id: aa7ff9520a6a2470c642f06797757e1c0362abe9
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056802
fbshipit-source-id: 72eb498673aecce48145c785a4c2a48e0432059d
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056725
fbshipit-source-id: 771e5dd4cd1aeca3d2afd1a67ee58b9ac21eda79
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056723
fbshipit-source-id: c20183a4a1189f13b15a138968937080888a200b
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056720
fbshipit-source-id: f36a6caf4e748c915b66f66fd9b4cad6826ecacf
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056714
fbshipit-source-id: 215d8e44d7909f30f4a45f57e5d22a32a635d0ba
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056700
fbshipit-source-id: b3452125678ec8770dec9587106b8bf0f2490027
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056696
fbshipit-source-id: 50c0f01164e078b0ad32f66dda80c965f731f1fb
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056690
fbshipit-source-id: 208982ca5b53985bd8079c4b206a7eb4bb883494
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056685
fbshipit-source-id: a9abb451e62c9e378b0a5667fa4c5f82952bd02b
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056680
fbshipit-source-id: 2a57f15c01a585af9ddbe08cf6dd60922d4f64b3
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056676
fbshipit-source-id: 6baa5d9ba0ef15218ce02cbb51862f18d98b465c
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26056672
fbshipit-source-id: 2f3e4b9bbed339f15b22fe87e27d81af083fd481
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D26055276
fbshipit-source-id: f38f9e94ab313fb842d1e3037ab667cd460f1043
Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26054855
fbshipit-source-id: 9592d854962d53f64d836b8361256cac5c4325df
Summary:
Support RCTModernEventEmitter +RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26054856
fbshipit-source-id: 228cc08a624e793aff4caf36e1df8285f3b3519d
Summary:
Support RCTModernEventEmitter +RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26043861
fbshipit-source-id: 37757508c835cbd1181f0e0e774abc62fdbfee2b
Summary:
Support RCTModernEventEmitter +RCTEventEmitter in an Event class. This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26043393
fbshipit-source-id: b126658d818a18b7ffb2794de8e59a49c0e3d258
Summary:
The `Event` interface has been improved such that:
1. `getEventData` is now a default method on Event that returns null
2. `dispatch` has a default implementation that relies on getEventName() and getEventData()
3. `dispatchModern` can detect if surfaceId and event data are present; if not, it falls back to dispatch
This will dramatically ease future migrations: at some point in the (distant) future, we can simply delete RCTEventEmitter and
all use-cases will be supported by the current `Event` class without needing to introduce a 3rd transitional interface; and 99%
of all Event classes can be simplified, delet their `dispatch` implementation and need no further work.
At their core, all Events are simply: (1) a name, (2) data, (3) a target (surfaceId and tag). The interface now reflects that but still
allows for flexibility of the data and names being generated on-demand if necessary; but for the vast majority of Event classes, code
will be dramatically simplified.
I also migrate a single Event class, ContentSizeChangeEvent, to use this new method of dispatch.
Changelog: [Android][Changed] Added convenience methods to simplify native Event classes and ease migrations
Reviewed By: mdvacca
Differential Revision: D26043325
fbshipit-source-id: bc308105f7f6e654d45fd156dbf4a2bcbc45819c
Summary:
We assume that startSurface is always called off the UI thread; see if we can synchronously call setId and setSurfaceId, and in general, call setSurfaceId sooner.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26053050
fbshipit-source-id: a657584502bb0018e9591fe610eac0fc9fab2ca9
Summary:
Before this change, `mountingOverrideDelegate` was proxied via `Scheduler::startSurface` and `ShadowTree::ShadowTree` constructor down to `MountingCordinator`. Now we set it on the `MountingCoordinator` directly.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26049076
fbshipit-source-id: 7f1ecf2c8b6f264a7e59d19881464fe529c53d30
Summary:
The RootView being managed by Fabric should have an id of View.NO_ID when it is "handed over" to RN. This is true for Fabric and non-Fabric
and setting a custom id on the ReactRootView has never been supported. I'm temporarily (?) adding an additional check earlier and into ReactRootView to hopefully
catch any of these issues early.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26077509
fbshipit-source-id: 59e1ec080504e50698acc654c29120f039238a96
Summary:
There is a race between PreAllocateMountItems executing and killing off stale SurfaceMountingManagers.
For now I'm simplifying the "eviction" mechanism. We just keep up to 15 SurfaceMountingManagers around and start evicting them from the least-recently-referenced one.
Hopefully this logic can be simplified in the future.
I'm also sneaking in a small perf optimization for PreAllocateMountItem: don't queue them if the associated surface is already stopped, because chewing through a bunch of dead PreAllocateMountItems on the UI thread can be expensive.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26059378
fbshipit-source-id: 3b2503d7d8e5f045ae741d0d4a5880d1b37758d2
Summary:
In some cases, onMeasure/onLayout are called on the RootView before `startSurface` in Fabric has been able to set surfaceId on the RootView.
With the new SurfaceMountingManager, this causes a crash because we need a valid surfaceId to perform an operation. Before the SurfaceMountingManager refactor, a surfaceId of 0 would be passed to `mBinding.setConstraints` in the FabricUIManager, and setConstraints in C++ noops if there's an invalid surfaceId.
For now, FabricUIManager will also fail silently if the surfaceId is invalid when updateRootLayoutSpecs is called, just to be conservative and to be consistent with previous behavior. This will be upgraded to a hard-crash in the future.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26051266
fbshipit-source-id: ca2d80f899cdba9b3962af68546bd83b77be0680
Summary:
This diff removes references to FabricUIManager from UIManagerModule, these callsited were originally used for NativeAnimatedDriver, but they are not used anymore
changelog: [internal] internal
Reviewed By: JoshuaGross, shergin
Differential Revision: D26035388
fbshipit-source-id: d4825af17f6948d922c42670f2c7b02498c12039
Summary:
See title. dispatchV2 was just introduced yesterday, so nothing relies on it yet / it's safe to rename (there are no released versions of RN with `dispatchV2`).
And... dispatchModern is a better name.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26039008
fbshipit-source-id: 009ae721b8caef23a389c33b51f6f8952a6a73da
Summary:
In SurfaceMountingManager/MountingManager/FabricUIManager infra, we try to blackhole events that are sent to a stopped surface. In this case I just forgot to add a null check. Add here to protect against events sent to stopped surfaces - for example, if a TextInput is focused when the surface is stopped, a "blur" event will be sent and will crash here otherwise. Now it blackholes silently, as expected.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26031260
fbshipit-source-id: 9936466ca2d00267efaf7fa594c9bcd59f7aad2a
Summary:
Support RCTEventEmitterV2 in ReactTextInput*Event classes for optimal Fabric perf. Backwards-compatible with non-Fabric renderer.
Changelog: [Changed][Android] Old native ReactTextInput*Event creation APIs have changed, and will be removed in the (distant) future. The old methods will work for now with minor perf implications for Fabric.
Reviewed By: mdvacca
Differential Revision: D26031261
fbshipit-source-id: 7c972ecfd1e395104c4639995bb5ecc5f7c6baae
Summary:
Support RCTEventEmitterV2 in ImageLoadEvent for optimal Fabric perf. Backwards-compatible with non-Fabric renderer.
Changelog: [Changed][Android] Old native ImageLoadEvent creation APIs have changed, and will be removed in the (distant) future. The old methods will work for now with minor perf implications for Fabric.
Reviewed By: mdvacca
Differential Revision: D26029773
fbshipit-source-id: c8e00e06a2f9d6682367f9099bdf7f5fc12890e0
Summary:
Motivation: perf, simplicity, adhering to new SurfaceMountingManager APIs available to us. Backwards-compatible with events sent through old system or Fabric, to Fabric or non-Fabric Views.
Changelog: [Changed][Android] Old Native method to create ScrollEvent has been deprecated and will be removed at some point in the (distant) future
Reviewed By: mdvacca
Differential Revision: D26027105
fbshipit-source-id: b9dba5b56c2bfed3b8fc4488c54b271b85ab5fa0
Summary:
Refactor EventEmitters to take an optional SurfaceId that Fabric will use, and non-Fabric will not.
Migrating touches is a good proof-of-concept for how this could be used generally, and as it turns out, TouchEvent's API is more flexible than most other event APIs (because it uses a dictionary to pass data around, so we can just stuff SurfaceId into it - not efficient, but flexible).
All new APIs are backwards-compatible and designed to work with old-style events, with Fabric and non-Fabric. Native Views that migrate to the new API will be backwards-compatible and get an efficiency boost in Fabric.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26025135
fbshipit-source-id: 5b418951e9d0a3882f2d67398f2aaadac8a3a556
Summary:
Create V2 EventEmitter that surfaceId can be passed into, with a backwards-compat layer, and some debug-only logging to help assist with migration.
Changelog: [Changed][Android] RCTEventEmitter has been deprecated in favor of RCTModernEventEmitter for optimal Fabric support; RCTEventEmitter will continue to work in Fabric, but there are minor perf implications.
Reviewed By: mdvacca
Differential Revision: D26027104
fbshipit-source-id: 784ca092bbc88d19c82f6c42537c34460d96de86
Summary:
There's a field called `surfaceID` in a couple of classes that isn't the same as the integer `surfaceId` in Fabric.
For consistency, I've deprecated a couple of them, or renamed when appropriate.
In addition, now we're passing the actual integer surfaceId into the ThemedReactContext. This means that every single View created in Fabric gets annotated with the surfaceId it's in. Currently this isn't used, but the idea is that now each View has a mapping back to its surface, which could be used to simplify / optimize operations with SurfaceMountingManager. In particular, we might be able to use this in the future to optimize animations and/or event emitters.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26021571
fbshipit-source-id: b7db7de123db07fa928a6f815be86bdbb030e62c
Summary:
This refactors MountingManager into a minimal API that shims into a more fully-featured SurfaceMountingManager. The SurfaceMountingManager keeps track of surface start/stop, surface ID, and surface Context.
This solves a number of issues around (1) race conditions around StopSurface/StartSurface, (2) memory management of Views, (3)
Concrete improvements:
1. Simpler to reason about race conditions around StopSurface/StartSurface.
2. 1:1 relationship between SurfaceId and all Views/tags.
3. When surface is stopped, all descendent Views can be GC'd immediately.
4. Fixed separation of concerns and leaky abstractions: surfaceId/rootTag and Surface Context are now stored and manipulated *only* in SurfaceMountingManager.
5. Simpler StopSurface flow: we simply remove references to all Views, and the Fragment (outside of the scope of this code) removes the RootView. This will trigger GC and we do ~0 work. Previously, we ran a REMOVE and DELETE instruction and kept track of each View in a HashMap. Now we can simply delete the map and move on.
The caveat: NativeAnimated (or other native modules that go through UIManager). APIs like `updateProps` currently uses only the ReactTag and does not store SurfaceId. This is a good argument for moving away from ReactTag, at least in its current incarnation, but: for now this requires that you do a lookup of a ReactTag across N surfaces (worst-case) to determine which Surface a ReactTag is in.
So, to summarize, the "con" of this approach is that now `getSurfaceManagerForViewEnforced` could be slower. It is used in:
* NativeAnimatedModule calls `updateProps` through UIManager
* FabricEventEmitter calls `receiveEvent` on FabricUIManager directly
* On audit, I could find zero native callsites to `sendAccessibilityEvent` through UIManager
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26000781
fbshipit-source-id: 386ae40c4333f8c584e05818c404868dbee6ce73
Summary:
MountingManager keeps a map of tags to Views, and attempts to clean it up by (1) deleting tags when they're explicitly deleted, (2) recursively deleting all Views when the View hierarchy is torn down.
However, there appear to be.... substantial gaps here. In tests, when navigating between screen X -> screen Y -> back to X (triggering a StopSurface), each "StopSurface" resulted in 200-600 Views being leaked (!).
What is causing these leaks? Well, for one, the "dropView" mechanism isn't perfect, so it might be missed Views. Second, Views don't always guarantee that `reactTag == getId()`, so that could result in leaks. Third, View preallocation on Android complicates things: Views can be preallocated and then never even inserted into the View hierarchy, so DELETE mutations could never be issued. Fourth, StopSurface is also complicated on Android (largely because of View preallocatioAndroid (largely because of View preallocation).
So, I introduce a new mechanism: keep a list of all tags for a surface, and remove all tags for a surface when the surface is torn down. This should be fool-proof: it handles preallocation and normal creation; it can handle deletes; and we're guaranteed that tags cannot be added after a surface is stopped.
Is this overly complicating things? Well, hopefully we can simplify all of this in the longterm. But until we get rid of View Preallocation, it seems like we need this mechanism - and View Preallocation might be around for a while, or forever.
Other thoughts: it's possible that using other data-structures could be more efficient, but I'm guessing the perf implications here are marginal (compared to the insane amount of memory leaks we're fixing). It could also simplify things to have a SurfaceMountingManager interface that implies all actions happen on a specific surface, including teardown.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25985409
fbshipit-source-id: f55b533770b1630c6c2a9b7a694d953aa3324428
Summary:
Original commit changeset: 3ed8e78e31b0
Backing-out D25938851 (69b3016171) and D25935785 (bdea479a1f). Based on analysis documented in T83141606, I believe this issue should be fixed in JS.
Additionally, this crash actually has nothing to do with (un)flattening or the differ; it is a side-effect of stale ShadowNodes being cloned, which I believe is either UB or a contract violation. Either way, it should probably be fixed either in JS, or in node cloning. So this isn't the right solution for this issue and should be reverted.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25949569
fbshipit-source-id: 8cf1094a767da98fff4430da60d223412e029545
Summary:
This diff changes the type of the SwipeRefreshLayoutManager.size prop from Int to String in Fabric.
The current implementation of this prop allows JS developers to use "int" type when fabric is enables and "int or string" types when using Fabric is disabled.
Since long term we want to only support "string" type for this prop, I'm changing the type of the prop to be String.
After my diff Fabric will start supporting only "string" types, non fabric screens will keep supporting "int or string" values.
**Will this break production?**
No, because there are no usages of RefreshControl.Size prop in fbsource
**What about if someone start using this prop next week?**
IMO It's very unlikely because of the nature of this prop, I will be monitoring next week and if there's an usage it will be detected by flow when trying to land D25933457.
Changelog: [Android][Changed] - RefreshControl.size prop changed its type to string, the valid values are: 'default' and 'large'
Reviewed By: JoshuaGross
Differential Revision: D25933458
fbshipit-source-id: 55067d7405b063f1e8d9bb7a5fd7731f5f168960
Summary:
Android has some optimizations around view allocation and pre-allocation that, in the case of View Unflattening, can cause "Create" mutations to be skipped.
To make sure that doesn't happen, we add a flag to ShadowViewMutation (in the core) that any platform can consume, that indicates if the mutation is a "recreation" mutation.
It is still a bit unclear why this is needed, in the sense that I would expect props revision to increment if a view is unflattened. However, there is at least one documented reproduction where that is *not* the case. So for now, we'll have a hack pending further investigation.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25935785
fbshipit-source-id: 6fb4f0a6dedba0fe46ba3cd558ac1daa70f671f5
Summary:
MountItem logging was broken in D25841763 (4076293aa1), by adding a field that wasn't logged.
Fixed here.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25934390
fbshipit-source-id: 2db4a809b150ad33ddf886db1db6585889bd7013
Summary:
This diff refactors the intialization of Fabric in order to avoid loading UIManagerModule as part of the creation of FabricJSIModuleProvider.
One caveat is that now we are not taking into consideration the flag mLazyViewManagersEnabled
master/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/CoreModulesPackage.java177
if (mLazyViewManagersEnabled) {
As a side effect of this diff view managers will be initialized twice if the user has fabric and paper enabled
This diff was originally backed out in D25739854 (4984c1e525) because it produced a couple of bugs:
https://fb.workplace.com/groups/rn.support/permalink/4917641074951135/https://fb.workplace.com/groups/rn.support/permalink/4918163014898941/
These bugs are fixed by D25667987 (2e63147109).
This diff was reverted a couple of times because of the change in the registration of eventDispatcher. That's why I'm gating that behavior change as part of the "StaticViewConfig" QE.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D25858934
fbshipit-source-id: a632799ccac728d4efca44ee685519713b4a7cbb
Summary:
These methods can all throw exceptions that get caught and reported by JS. The logviews aren't currently very helpful; hopefully adding additional information will make batch debugging a little easier.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D25870788
fbshipit-source-id: a1cab225b11a3d2868f098d4575e475ee4064e65
Summary:
This diff migrates all the lookups of EventDispatcher to not depend on UIManagerModule anymore.
This refactor is necessary because:
- Users running in Fabric / Venice should not load on the UIManagerModule class
- D25858934 will introduce a change that will break all of these callsites
In the migration I'm relying on the method UIManagerHelper.getEventDispatcherFromReactTag() that returns the correct EventDispatcher for a reactTag.
I'm planning to land this change early in the week (to catch potential errors in alpha / beta versions)
As a followup we need to deprecate and prevent developers to continue using getNativeModule(UIManagerModule.class) moving forward. That will be part of another diff
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D25858933
fbshipit-source-id: e26c99759307517b5bef483274fe0e0d71bb4c6c
Summary:
Hermes has a way to set up a callback that is invoked when a fatal error
such as Out of Memory occurs. It is a static API that should be called at
most once, so it uses `std::call_once` to achieve that.
The fatal error handler is simple, it just uses glog to log an error message
to logcat, then aborts (using `__android_log_assert`).
The reason is typically very helpful for understanding why `hermes_fatal` was called.
Changelog:
[Android][Internal] - Print a logcat message when Hermes has a fatal error
Reviewed By: mhorowitz
Differential Revision: D25792805
fbshipit-source-id: 45de70d71e9bd8eaa880526d8835b4e32aab8fe3
Summary:
This diff logs a SoftError when there is not EventDispatcher associated to UIManager
The app will crash in Debug mode, this will not affect production users
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D25859546
fbshipit-source-id: 8045bcd67f613ea6286f30fe6f3c66113c700b0b
Summary:
The purpose of this diff is to ensure that visibility changes are handled correctly when the value of "display" for a View changes from 'flex' to 'none'.
RNTester is nesting several Views with different kind of visibilities. When the user tap on an item there's a state update that changes the visibility styles for some of these views. Fabric does not reflect the right changes of visibility on the screen.
changelog: internal
Reviewed By: shergin
Differential Revision: D25841763
fbshipit-source-id: 769b97afb72939d346a4c6f2669ff938b35596bc
Summary:
This is the Android native implementation of sendAccessibilityEvent for Fabric.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25852418
fbshipit-source-id: cb51e667a7f673da6b9c9e539770225b02bdc902
Ramanpreet Nara
Andrei Shikov
Peter Laraia
David Vacca
Lulu Wu
CodemodService FBSourceBuckFormatLinterBot
Kudo Chien
Mike
Araz Abishov
grgr-dkrk
Tim Yung
fabriziobertoglio1987
Joshua Gross
Samuel Susla
Aditya Sharat
Ivan Kobzarev
generatedunixname89002005287564
Gaurav Gupta
Héctor Ramos
generatedunixname89002005325676
Valentin Shergin
Igor Klemenski
Albert Sun
Sidharth Guglani
generatedunixname89002005325674
Ron Edelstein
Dulmandakh
Riley Dulin
Andres Suarez
