Summary:
Typically, ReactTextView#setText is called via ReactTextViewManager#updateExtraData, but natively animated color changes bypass render and layout pass via direct call to SurfaceMountingManager#updateProps from UIManager#synchronouslyUpdateViewOnUIThread.
Thus, for animated color changes to get applied, we need to handle the color prop in ReactTextAnchorViewManager.
In addition, native driver updates are not synchronized with Fabric's mounting; if the native driver update happens before mount, the update is done in updateState.
Changelog:
[Android][Added] - Support animating text color with native driver
Reviewed By: mdvacca
Differential Revision: D34630294
fbshipit-source-id: c0f1e19c801c0e909e84387d623a6556ce6f2d67
Summary:
Not having this disallows including turbo module and extending in places where RTTI is enabled.
There is no additional includes or implementation changes - it merely allows for things to nicely link with other libraries.
Changelog: [General][Fixed] - Allow including TurboModule.h in mixed rtti/no-rtti environment, even if TurboModule.h/cpp is compiled without RTTI.
Reviewed By: appden
Differential Revision: D34637168
fbshipit-source-id: 2e5d9e546bdc5652f06436fec3b12f1aa9daab05
Summary:
The issue to be fixed here is an edge case when in nested scroll view, for example vertical scroll wraps horizontal scroll, when user scrolls horizontally then move the joystick downwards, the horizontal scroll would bounce back. This is shown in the demo:
https://pxl.cl/209f8
The root cause of the issue is due to two workflows fight to set the scroll position and confusing the scroll change dispatcher to calculate fling velocity direction, which caused a wrong fling behavior. Check this log and description to get more details:
{F706620839}
To fix the issue, in this diff we overridden the `onInterceptTouchEvent` method in panel app scroll views, and make it return true when the previous scroll is still running. This effectively skips the drag slop in the default scroll view behavior, as we know the touch event should've been handled by the scroll view if it's still scrolling.
This way, the vertical scroll part on the vertical scroll view will not be dispatched to the horizontal scroll at all. This allows the nested scroll view to work separately.
Changelog:
[Android][Internal] - Share the post intercept touch event logic in scroll views
Reviewed By: javache
Differential Revision: D34627329
fbshipit-source-id: d500ec3bfbe349a45ef5c1360b5c8807c168f682
Summary:
This diff aims to fix an issue I found while working on T113264056. It's to address the [comment](https://www.internalfb.com/diff/D3207608 (https://github.com/facebook/react-native/commit/a3146e41a259175f0c79e3c213523a0fb21613d7)?dst_version_fbid=507451319451602&transaction_fbid=1615731455398227) in an old diff D3207608 (https://github.com/facebook/react-native/commit/a3146e41a259175f0c79e3c213523a0fb21613d7). That issue begins to surface after I made the change in the next diff, which is discussed in detail in the task description (T113385381).
The fix here is to cancel the post touch processing when a new touch down is received. This should've cancel any previous post touch processing as the new touch should take full control of scrolling.
Changelog:
[Android][Fixed] - Cancel post touch process when new touch is received
Reviewed By: javache
Differential Revision: D34627330
fbshipit-source-id: 1fb8cfc1a4d94349b5290915a0a9f190186f2af3
Summary:
Why Lacrima?
- Throw a JavascriptException in native won't get the js error reported to backend, we need to use reporter APIs
- Lacrima is a new error reporting framework to report crashes and app deaths in android applications at Facebook, and it has APIs for reporting js exceptions.
- ```FbErrorReporterImpl.java``` uses ```Acra``` API to report, and ```Lacrima``` is a rewrite of ```Acra``` https://fb.workplace.com/groups/323014308578038/
- We've been receiving js errors reported via Lacrima https://fburl.com/logview/y1vhc8u8
In this diff all js errors are treated as soft errors during reporting because they don't usually crash the app, crashes will be reported with a different category.
Changelog:
[Android][Chagned] - Change static string to public
Reviewed By: fkgozali
Differential Revision: D34095100
fbshipit-source-id: 73d89647134a197baf5d228d620732781b6bd723
Summary:
changelog: [internal]
I'm looking into JS exceptions in React 18 on Android and I've noticed we use [std::unordered_set::contains]((https://en.cppreference.com/w/cpp/container/unordered_set/contains) that is only supported in C++20. I don't think this causes the exceptions but I would like to eliminate this option.
Reviewed By: javache
Differential Revision: D34547741
fbshipit-source-id: 6cffcff3366e7579a2c0e19bc01ffcb355b9ddb6
Summary:
changelog: [internal]
ViewState is sometimes deleted but not created again because of race condition in C++ pre-allocation and how it checks for revision to determine if createView mount item should be dispatched.
In this diff, we add an option to delay deletion of ViewState until the next commit.
Reviewed By: ShikaSD
Differential Revision: D34553174
fbshipit-source-id: e5213f1d39137fb56fc745321a7c1b28d417ea27
Summary:
changelog: [internal]
Make all feature flags `const` and initialise them as ivars. Also makes the class final and removes virtualised destructor since it is not needed.
Reviewed By: ShikaSD
Differential Revision: D34549013
fbshipit-source-id: 2b326bc5b6c1dd6d89c2fb9c671bda6da669fa76
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33209
Downloads Kotlin compiler JARs with Buck before starting docker build. This solution is slower than checking in JARs directly, but it allows to keep lighter size of the repo for people who want a complete checkout.
Changelog: [Internal] - Update CI build to download Kotlin jars for buck
Reviewed By: cortinico
Differential Revision: D34582932
fbshipit-source-id: 290398579ce2a4d57c7af318c66526689db6073c
Summary:
Per Android docs (https://developer.android.com/reference/android/graphics/Paint#FILTER_BITMAP_FLAG):
> This should generally be on when drawing bitmaps, unless performance-bound (rendering to software canvas) or preferring pixelation artifacts to blurriness when scaling significantly.
Changelog:
[Android][Fixed] - Improve rendering of images when resampled and corner radius applied
Reviewed By: genkikondo
Differential Revision: D34551184
fbshipit-source-id: 649da53af816be829e5b7f8e2a100f07a8b412b7
Summary:
Disables preallocation for cloned nodes, switching to create during commit for those cases instead.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D34555621
fbshipit-source-id: 21fc834cea318ca299aa37a9aab28a2f2a9675b5
Summary:
remove duplicate code in FabricUIManager
The removed line is already called few lines above
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D34553849
fbshipit-source-id: 2e0102a77e7464c9803fb1175385fdafbb88f1fa
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33196
Fixes compilation of Android template and renames the field according to Java guidelines.
Changelog: [Android][Changed] - Rename field with default values for ReactConfig to DEFAULT_CONFIG
Reviewed By: cortinico
Differential Revision: D34523356
fbshipit-source-id: 46ed703e7ca8ab5e05879dd93e8e4d2d816c2d03
Summary:
Introduce DefaultValuesReactNativeConfig constant that represents a ReactNativeConfig that always return default values
I also use this constant in all the apps using fbaric that don't have a custom Mobile Config
changelog: [internal] internal
Reviewed By: RSNara
Differential Revision: D34160478
fbshipit-source-id: 5730054fd4cc87e1fe5dc8247cc237194bf48981
Summary:
Changing fbjni's mapException interface to use `std::exception_ptr`, this enables us to use `std::rethrow_exception` and `catch` statements to check for specific exception types, rather than using dynamic_casts and having an explicit dependency on RTTI.
While generally, we should always be enabling RTTI and exception support simultaneously, we have a number of targets where we don't, or where avoiding RTTI provides significant binary size gains.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D34379950
fbshipit-source-id: 446d105f9fb50ada3526f242f98e163215abd0ab
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33168
Buck Kotlin setup requires jars to be available locally before the build, so we are checking them in.
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D34419817
fbshipit-source-id: 5e75f0f3818fb76b640349c003bd258bc12aa53d
Summary:
With the `MapBuffer`-based props calculated from C++ props, there's no need to keep `rawProps` around for Android views.
This change makes sure that the `rawProps` field is only initialized under the feature flag that is responsible for enabling `MapBuffer` for prop diffing, potentially decreasing memory footprint and speeding up node initialization as JS props don't have to be converted to `folly::dynamic` anymore.
For layout animations, props rely on C++ values, so there's no need to update `rawProps` values either.
Changelog: [Internal][Android] - Do not init `rawProps` when mapbuffer serialization is used for ViewProps.
Reviewed By: mdvacca
Differential Revision: D33793044
fbshipit-source-id: 35873b10d3ca8b152b25344ef2c27aff9641846f
Summary:
Provides an alternative way of diffing props on Android side. Instead of passing dynamic JSON values from JS, the new approach diff C++ props and serializes difference into a `MapBuffer`. Current implementation does this only for `RCTView` component to test performance, correctness and memory impact (as MapBuffers are supposed to be more compact and performant).
This way of passing props allows for additional alignment between platforms, as C++ props are now source of truth for the view state, similar to iOS. At the same time, changing serialization method requires reimplementing `ViewManager` codegen (done manually for now) to account for `MapBuffer`s.
Changelog: [Added][Android] - Added an experimental prop serialization path based on MapBuffer
Reviewed By: mdvacca
Differential Revision: D33735245
fbshipit-source-id: 1515b5fe92f6557ae55abe215ce48277c83974a6
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33160
Enables Kotlin build in RNAndroid, while adjusting the package structure to help with path/package inconsistency.
Moves `react/uimanager/interfaces` files into `react/uimanager` to make sure package name matches with file location and updates Buck configuration to still include it as a separate target.
Changelog: [Android][Changed] - Moved `com/react/facebook/uimanager/interfaces` files into `com/react/facebook/uimanager` to enable Kotlin build
Reviewed By: cortinico
Differential Revision: D34381179
fbshipit-source-id: 252588d9c7f62b8019bdfcce66197628d63e63a1
Summary:
Creates a mapbuffer from two ViewProp objects. This MapBuffer is used later instead of bag of props from JS to set the properties with platform ViewManager.
Changelog: [Internal] - Added MapBuffer diffing for ViewProps
Reviewed By: mdvacca
Differential Revision: D33735246
fbshipit-source-id: 10ad46251ea71aa844586624c888f5223fa44e57
Summary:
Changes `peek`->`poll` on Animated node queues to a single `poll`, accounting for race conditions. In case the queue batch is split at some point, previously polled operation is kept as a "carry-over" inside the queue abstraction.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D34377737
fbshipit-source-id: 78e5ca60c4777576fbf009c3d3ffa53629221910
Summary:
The concurrent queue seems to leak values on some Android 12 devices when interleaving instruction polling and execution. To help with that, we can drain the queue before executing operations, reducing the window when `add` and `poll` happen concurrently.
Changelog: [Internal] - Drain the Animated queue before execution
Reviewed By: JoshuaGross
Differential Revision: D34370056
fbshipit-source-id: 781631dbf1a10e3f1ba9564db985403bd77a2c06
Summary:
I was seeing a SoftException in our apps due to this method being called as part of ReactInstanceManager's init. It seems inconsistent that we would call this from a background thread when all other `onHost*` methods are marked as confined to the UI thread.
Changelog: [Internal]
Reviewed By: ryancat
Differential Revision: D34340210
fbshipit-source-id: 0104eda66b2ca6bb315e64e806e9a30409d0d45c
Summary:
This is a refactor of the logging of Fabric commit statistics to simplify the way we track performance points.
we'll later refactor and iterate on the API to integrate fabric perf point into developer tools
changelog: [internal] internal
Reviewed By: ShikaSD
Differential Revision: D34006700
fbshipit-source-id: 93a01accd90dfacc8b44edd158033b442a843284
Summary:
Was trying out some behaviour when using the MapBuffer experiment and fixed some small issues.
Changelog: [Internal]
Reviewed By: ShikaSD
Differential Revision: D34108859
fbshipit-source-id: 550ca0847419006ec17472cc4b70d38fc8d05396
Summary:
Brings the same fix https://www.internalfb.com/diff/D34015853 (https://github.com/facebook/react-native/commit/be260b9f479a3b55ee43d2959d2c49fd3c1eb4ac) for ReactScrollView to ReactHorizontalScrollView
When setting ScrollView's contentOffset, if the ScrollView hasn't been laid out yet when ReactHorizontalScrollViewManager.setContentOffset is called, then scroll position is never set properly. This is because the actual scroll offset (0, 0) was being passed into setPendingContentOffsets, instead of the desired scroll offset. Thus, when ReactHorizontalScrollView.onLayout gets called, ReactHorizontalScrollView.scrollTo gets called with (0, 0).
Changelog:
[Android][Fixed] - Fix ReactHorizontalScrollView contentOffset
Reviewed By: bvanderhoof
Differential Revision: D34246489
fbshipit-source-id: d923f7c9f136f7275d64bd658ffd5c2cc049d392
Summary:
Found that after backgrounding `mShouldStop` would always remain true, which prevents events from being dispatched / scheduled.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D34247567
fbshipit-source-id: 63876986dc0cee5e2a73cb4f8a35d90379d9f8ea
Summary:
The check implemented in PR https://github.com/facebook/react-native/issues/29089 is flawed as the exception class name and message depends on the OS version as well as the OEM. In OxygenOS running android 11, it comes out as RuntimeException and the check fails and hence the crash occurs again. But on observing closely, its clear that the exception message is consistent across OEMs with similar strings appearing in them that have been included in the if check.
Hence there is a simple fix to this issue, by checking the message instead of the exception class.
Here is the snippet:
```
private Nullable CookieManager getCookieManager() {
if (mCookieManager == null) {
possiblyWorkaroundSyncManager(mContext);
try {
mCookieManager = CookieManager.getInstance();
} catch (IllegalArgumentException ex) {
// https://bugs.chromium.org/p/chromium/issues/detail?id=559720
return null;
} catch (Exception exception) {
String message = exception.getMessage();
// We cannot catch MissingWebViewPackageException as it is in a private / system API
// class. This validates the exception's message to ensure we are only handling this
// specific exception.
// The exception class doesn't always contain the correct name as it depends on the OEM
// and OS version. It is better to check the message for clues regarding the exception
// as that is somewhat consistent across OEMs.
// For instance, the Exception thrown on OxygenOS 11 is a RuntimeException but the message contains the required strings.
// https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/webkit/WebViewFactory.java#348
if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") ||
(message!=null && (message.contains("WebView provider") ||
message.contains("No WebView installed")))){
return null;
} else {
throw exception;
}
}
}
return mCookieManager;
}
```
## Changelog
[General] [Added] - A fail proof check to catch any crash involving webview:
if (exception.getClass().getCanonicalName().contains("MissingWebViewPackageException") || (message!=null && (message.contains("WebView provider") || message.contains("No WebView installed"))))
[General] [Removed] - Flawed check to catch WebViewProvider crash:
if (message != null && exception.getClass().getCanonicalName().contains("MissingWebViewPackageException"))
Pull Request resolved: https://github.com/facebook/react-native/pull/33088
Test Plan:
This code has been tested rigorously on OnePlus Nord CE 5G (Oxygen OS 11.0) and Realme X (Realme UI 2.0) both running on Android 11 and reproducing the crash on a hybrid (Native android + ReactNative) app that showed this crash in production being dependent on WebViews. I have implemented an entire patched fork of 0.67.2 to fix this issue in our app.
How to test:
Launch any app that has a webview.
Go to settings->apps->Android System Webview -> disable.
Resume/Restart the app.
In this fix:
Open a webview and notice that the app will not crash.
In current version:
App crashes on startup as the exception escapes the catch block.
Even if it survives startup (did on Realme X), it will crash once you try to open a webview.
Reviewed By: rh389
Differential Revision: D34240097
Pulled By: cortinico
fbshipit-source-id: 0f1f9a3b078c0ad3074c7841392892cb70b427eb
Summary:
This diff integrates DeviceConfig into ReactNativeConfig used by ReactNativePanelApps. The goal is to be able to control Fabric MCs using GKs and QEs in RN VR apps
I did an audit of the MCs that were used by RNPanelApps:
```
"react_fabric:enabled_android_fabric_logs": -> will get data from GK (disabled by default)
"react_fabric:disable_virtual_node_preallocation": -> does not exist in code anymore
"react_fabric:enable_early_event_emitter_update": -> will get data from MC (disabled using static value)
"react_fabric:enable_background_executor_android": -> does not exist in code anymore
"react_fabric:enable_props_forwarding_android": -> does not exist in code anymore
"react_fabric:remove_outstanding_surfaces_on_destruction_android": -> will get data from MC static value (ENABLED using static value)
"react_fabric:enable_large_text_measure_cache_android": -> will get data from MC default value (ENABLED by default)
```
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D33898210
fbshipit-source-id: 0ea1e0e2fc15929bec328f7dcc9410efa9925b34
Summary:
Certain events (practically always touch events probably?) will not be correctly emitted to JS in Fabric if there is no View underneath the touch - if there is no touch target besides the ReactRootView.
We can just rely on the UIManagerType annotation on the Event, which is correct and reliable.
Instead, what we do today is derive UIManagerType from ViewTag, which is correct UNLESS the viewtag is the same as the SurfaceId, in which case we may incorrectly detect that the touch is on a non-Fabric View when in fact it is on a Fabric ReactRootView.
ViewTag is not a reliable way to detect Fabric vs non-Fabric /when looking at the RootView/, where ViewTag is the same as SurfaceId. Ironically, only Fabric RootViews have a SurfaceId at all.
Practically, this won't change anything since events emitted to ReactRootView don't go anywhere (they don't have EventEmitters). So this is a pretty low-stakes fix, but is still technically correct.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D34149878
fbshipit-source-id: f01da556865eb597a50cd49e9787316a0ed56f70
Summary:
Currently we are using Appcompat in version 1.0.2 which is almost 4 years old now, this PR updates it to version 1.4.1.
Using Appcompat 1.0.2 was also causing a crash on RNTester due to an error where FontFamily's method was not found (Related to https://github.com/facebook/react-native/issues/33065)
Closes https://github.com/facebook/react-native/issues/31620
## Changelog
[Android] [Changed] - Bump android Appcompat to 1.4.1
Pull Request resolved: https://github.com/facebook/react-native/pull/33072
Test Plan: Use `./scripts/test-manual-e2e.sh` to test both RNTester and a new app
Reviewed By: cortinico
Differential Revision: D34107105
Pulled By: ShikaSD
fbshipit-source-id: 966e4687b09ae50a88ee518622f073d72e8c6550
Summary:
The views with touch event props are currently flattened by Fabric core, as we don't take event listeners into account when calculating whether the view should be flattened. This results in a confusing situation when components with touch event listeners (e.g. `<View onTouchStart={() => {}} /> `) or ones using `PanResponder` are either ignored (iOS) or cause a crash (Android).
This change passes touch event props to C++ layer and uses them to calculate whether the view node should be flattened or not. It also refactors events to be kept as a singular bitset with 32 bit (~`uint32_t`).
Changelog: [Changed][General] Avoid flattening nodes with event props
Reviewed By: sammy-SC
Differential Revision: D34005536
fbshipit-source-id: 96255b389a7bfff4aa208a96fd0c173d9edf1512
Genki Kondo
skychx
Nicola Corti
Nikita Lutsenko
Xin Chen
Lulu Wu
Samuel Susla
David Vacca
Andrei Shikov
Pieter De Baets
Joshua Gross
Alfonso Curbelo
fabriziobertoglio1987
Kunal Farmah
Dark Knight
Paige Sun
Gabriel Donadel Dall'Agnol
Ian Childs
