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 (https://github.com/facebook/react-native/commit/dfa8eb0558338f18ea01f294a64d355f6deeff06), 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 (https://github.com/facebook/react-native/commit/86321a35c03df340b6c1ea740ea7f29e2659b0ab) 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 (https://github.com/facebook/react-native/commit/00959ffd6b9aebf52ca51ce7a747a375b5a8cdd1). 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 (https://github.com/facebook/react-native/commit/630ac87591eb9d535bc32c6c42c624bfc3f1953f).
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