Summary:
Changelog: [Android] - Change StatusBar style handling strategy
Previously Android status bar can set to `dark-content` or `default`, I made the following changes:
- Added `light-content` to get align with iOS
- Changed the behavior of `default` from setting status bar with 'SYSTEM_UI_FLAG_LIGHT_STATUS_BAR' to not doing anything, I did this because 1, `setStyle('default')` is found called even without explicitly declared <StatusBar> on that surface, which I think should fail silently 2, my idea is that user should set status bar style to `dark-content` or `light-content` explicitly instead of using `default`.
- Fixed the bug found in Dating Settings's Second Look.
Reviewed By: RSNara
Differential Revision: D24714152
fbshipit-source-id: 76e7d0d45fd3b8c3733efaee81426f5f449cc7d8
Summary:
For over a month I've been searching for a crash that occurs during Android's native `dispatchDraw` method on View. The stack traces didn't show anything useful - everything in the stack trace was native Android code,
not React Native code.
This also seems to only repro on certain vendors, and only on a very few React Native screens - I'm still not sure why either of those are the case, but from my repro, a *very* specific set of interactions needs to happen
to trigger this crash. See comments inline and an example stack trace.
Luckily, the fix is trivial. Since this code is used for animations, accessibility, and a number of other important interactions, I'm gating this change for now.
In general we must be careful to only mutate the View hierarchy only when we /know/ for certain it is safe to do so. Indirectly mutating the View hierarchy during measure, onMeasure, layout, dispatchDraw, etc, can all be
very dangerous. This is one of the last "escape hatches" that can cause view hierarchy modifications unexpectedly, so I think it's a very good idea to "secure" this further, and only update props synchronously here - and
ensure that other MountItems like `Delete` are definitely /not/ executed here.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24639793
fbshipit-source-id: b6219ce882e8d2204b4d10bf99f6a1120a33cb5a
Summary:
I've had my eyes on this optimization for a while: during TTRC especially, but really during any heavy render path, Fabric will create hundreds to thousands of MountItem class instances in order to construct a BatchMountItem.
This results in: hundreds/thousands of round-trip JNI calls, hundreds/thousands of Object allocations, etc. This will also result in increased memory and GC pressure.
Theoretically, by reducing the number of JNI calls and reducing allocations, we may be able to get some small wins in memory and CPU usage during very hot paths.
I am motivated to do this, in part, to indirectly measure the cost of JNI calls as well as allocating many objects vs an int buffer. I am unaware of such a measurement that we can use to make architectural decisions for React Native Android overall.
The other reason this could be a positive change: if it's successful and we can delete the old path, we'll be able to delete a bunch of Java classes entirely which is great for APK size wins.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24631230
fbshipit-source-id: 86a46ffcaef4ecbec2e608ed226aed0b5c4b832e
Summary:
This re-uses the same logic as `logTaggedMarker` for `logTaggedMarkerWithInstanceKey`. When no instanceKey specified, use 0.
Changelog: [Internal]
Differential Revision: D24607919
fbshipit-source-id: 4a29e5ece9a5462eb1163185d26370ee873f1412
Summary:
Changelog:
Fix the cloneWithChildren implementation that was not copying the list of children on the java object.
We were missing on copying the list of children when cloning. This is pretty bad as it means that the clone operation was mutating the old node as well as the new. When multiple threads were involved this could cause crashes.
Reviewed By: SidharthGuglani
Differential Revision: D24565307
fbshipit-source-id: 4e2e111db389e25c315ce7603b4018ac695bb0f1
Summary: We support setting the text color in the ReactPicker component, but we don't apply the text color to the little arrow icon that appears next to it. This diff applies the color tint from the picker's primary text color (set with a style prop on the main picker component, *not* the 'color' prop on the Picker.Item) to the background of the picker, which tints the arrow icon.
Reviewed By: makovkastar
Differential Revision: D24480642
fbshipit-source-id: 7ce84d616ae677da8975be9444428392020c57dc
Summary:
Bump Okio to 1.17.5, which includes fixes for many bugs and crashes since current version. Also removed android.enableR8=false from gradle.properties because it's deprecated. And moved FEST_ASSERT_CORE_VERSION from gradle.properties to build.gradle because it's used in single line.
## Changelog
[Android] [Changed] - bump Okio to 1.17.5
Pull Request resolved: https://github.com/facebook/react-native/pull/30204
Test Plan: RNTester builds and runs as expected.
Reviewed By: hramos
Differential Revision: D24560711
Pulled By: fkgozali
fbshipit-source-id: 433075293ca2dc41869dbb272d674625639c8b83
Summary:
When working with RN installed from npm and a regular project structure `$rootDir` won't be at the react-native package root. Instead we can use `$projectRoot` which will always be the ReactAndroid folder.
## Changelog
[Android] [Internal] - Use $projectDir instead of $rootDir for ReactAndroid codegen
Pull Request resolved: https://github.com/facebook/react-native/pull/30220
Test Plan: Test building an app with RN as a regular dep with codegen enabled
Reviewed By: hramos
Differential Revision: D24560634
Pulled By: fkgozali
fbshipit-source-id: 434d32f37e6f9d48a8c562655ceff7249bd056ce
Summary:
Pass the ReactAndroid project build directory as a variable to the ndk build so it can be used instead of assuming that the build directory is under ReactAndroid/build.
## Changelog
[Internal]
Pull Request resolved: https://github.com/facebook/react-native/pull/30222
Test Plan: Tested in an app with a custom build directory
Reviewed By: yungsters
Differential Revision: D24560643
Pulled By: fkgozali
fbshipit-source-id: cc65a70582f546ca2e2ca9fb6a2ff03ea70ca9d8
Summary:
This does a few things:
* Remove USE_CODEGEN flag so that TurboModule is enabled by default for RNTester
* Use the codegen output for Java/JNI spec files
* Remove the checked in com.facebook.fbreact.specs Java/JNI files
Changelog: [Changed][Android] RNTester now enables TurboModule by default using codegen.
Reviewed By: RSNara
Differential Revision: D24382083
fbshipit-source-id: 87e3e0581bac3287ef01c1a0deb070c1d7d40f2d
Summary:
This moves all deps on the checked in fbreact/specs files to use the generated output (during build time) instead.
TLDR:
`react_native_target("java/com/facebook/fbreact/specs:FBReactNativeSpec")` ==> `react_native_root_target("Libraries:generated_java_modules-FBReactNativeSpec")`
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D24520293
fbshipit-source-id: 3fb34f172f1ab89b7198dfb4fccf605fbc32d660
Summary:
Sometimes stopSurface crashes when unmountReactApplication is called under Fabric; knowing the stack trace up to this point might assist in debugging.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24532027
fbshipit-source-id: f350e77fb1a2de52eb146b449f1d2f6e960fa017
Summary:
Fabric should be inserting Views into the hierarchy in the correct order based on z-index already, so there should be no reason to enable this mechanism.
At best it's a perf pessimisation and at worst it could be causing consistency issues or crashing (TBD). Most likely this is a noop.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D24512203
fbshipit-source-id: b9336240ef8506742bcbd8d08fc8b830f82cdfe2
Summary:
I want to see if the child count changes before/after this method execute; that would help pinpoint the cause of these crashes.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D24510064
fbshipit-source-id: 11b4baf15bc5e0beb23d65546605b378d84d1b20
Summary:
The value of the `ScrollView.snapToOffsets` property can be an empty array (most likely an issue in the product code), which will crash the app. This diff adds a check to prevent crashing in this scenario and falling back to the default snap behaviour.
Changelog:
[Android][Fixed] - Do not crash when ScrollView snapToOffsets is empty
Reviewed By: sammy-SC
Differential Revision: D24502365
fbshipit-source-id: c63b8e3b8f2fb323ebd6c962ee628015934d8e11
Summary:
Proxy is now enabled by default in hermes 0.7 (https://github.com/facebook/hermes/releases/tag/v0.7.0). However we currently disable it because of the config we pass.
This removes the config so proxy is now enabled.
## Changelog
[Android] [Changed] - Use default for hermes es6 proxy enabled
Pull Request resolved: https://github.com/facebook/react-native/pull/30142
Test Plan: Tested that proxy is now enabled (typeof Proxy !== 'undefined') with hermes 0.7.
Reviewed By: cpojer
Differential Revision: D24494182
Pulled By: mhorowitz
fbshipit-source-id: 7f8a506e2c436f2f1611e183ca22d33dc763643c
Summary:
Added a few FB vs OSS polyfills:
* react_native_root_target() to refer to the root FB react-native-github/ dir or repo dir in OSS
* react_native_xplat_synced_target() for anything xplat
* a few others
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D24437245
fbshipit-source-id: ee290a87a98a8e9be67b102a96f2faac2a2cb92b
Summary:
There's a crash for a small number of users that looks like it is happening when cutting the text via a context menu, or deleting content near the end.
This is only happening because we cache the Spannable and it detects changes due to the cache mechanism itself. I'm making a minor change that will hopefully result
in Spannables being copied instead of the same Spannable instances being used for display on the View and measurement; and also swallowing this error, since it should
not be considered as a fatal, unrecoverable error for now. Hopefully we can delete entirely in the future.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24430622
fbshipit-source-id: 495458b3d85733e46a7e62b5c954b7cb6b00470b
Summary:
We've deprecated API 20 and below. This is just a cleanup to remove code that assumes API level <21.
Changelog: [Android][Deprecated] Deprecate support of Android API levels 19 and 20.
Reviewed By: fkgozali
Differential Revision: D24380233
fbshipit-source-id: d8f98d7cb19446a60ba36137f1f9290e35f27c02
Summary:
This diff updates the minsdkversion of RN OSS template to API level 21.
Changelog: [Android][Deprecated] Deprecate support of Android API levels 19 and 20. The new minSDK version will be 21+ moving forward.
Reviewed By: fkgozali
Differential Revision: D24379607
fbshipit-source-id: 6801cdcd363065807cdc11006bd94217f914fac7
Summary:
Fixes a NPE in debug mode. This will only impact developers who have explicitly turned this debug flag on, so it's a very low-pri fix.
Changelog: [Internal]
Reviewed By: makovkastar
Differential Revision: D24410825
fbshipit-source-id: 08c8a0c6d0e0fb7c132725ad6af9460b91a7edf3
Summary:
So that it's easier for C++ targets to depend on libglog.so for debugging purpose.
Changelog: [Internal]
Differential Revision: D24382033
fbshipit-source-id: 00ad6b2365d571583d6d1aaa40fac2c96974abf1
Summary:
Although the interface for both NativeModules is the same, we'd like to enforce 1 `TurboModuleRegistry.get` call per NativeModule spec file. Therefore this diff splits the one spec into two.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24325260
fbshipit-source-id: f18718e4235b7b8ccbfc44a7e48571ecf483a36c
Summary:
The iOS and Android NativeModules are very different. It's better to split the two interfaces, than to have one merged interface.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24324247
fbshipit-source-id: 097273829ffc719eff006ed2dde55f0dd6bd7d95
Summary:
This NativeModule is actualy not used! Removing this now.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24324362
fbshipit-source-id: 1322c5e072961f1c6c54bfc6dbd562d42f9e5b3f
Summary:
When switching between non-Fabric and Fabric screens, I believe that `initializeEventListenerForUIManagerType` is not always being called on the NativeAnimatedNodesManager if
`NativeAnimatedModule.initializeLifecycleEventListenersForViewTag` is being called before the NativeAnimatedNodesManager ivar has been set. This should occur very infrequently, but logs
indicate that it /does/ happen in some marginal cases. Protecting against these cases should be trivial, by using the `getNodesManager` method which is responsible for returning a nodes manager or creating a new one.
The existing uses of the `NativeAnimatedNodesManager` ivar also occur on different threads and we were not protecting against this, so I'm changing it to an atomic. It's very likely that
the inconsistency issues in the past were caused not by ordering errors, but thread races.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24267118
fbshipit-source-id: 68abbff7ef3d0b2ecc9aa9977165663ad9447ab8
Summary:
There are cases under investigation where unmountReactApplication is called before the ReactRootView gets an ID; in some or all of those cases, UIManagerBinding.stopSurface cannot get the ReactFabric JSI module and crashes there.
It's still unclear why `unmountReactApplication` is being called in these circumstances - maybe another crash causing React Native teardown?
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24214712
fbshipit-source-id: 796594653f4ff6d87088c4841b89f06cc873b46f
Summary:
In D23640968 (https://github.com/facebook/react-native/commit/78b42d7fb7180102c1e8ec917dcccd2d9d4076db) I introduced a mechanism to update offsetX/offsetY whenever onMeasure/onLayout were called, to ensure that `measureInWindow` had the correct metrics and would work properly.
However, now `uiManager.updateRootLayoutSpecs` gets spammed and is called too often. For example, whenever a TextInput is focused/blurred, `uiManager.updateRootLayoutSpecs` may be called 5+ times even though
the measure specs/offsets may only change once.
Thus, we just compare with previous values before calling into the UIManager. This should give us a very small perf improvement.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24176867
fbshipit-source-id: f0dcc816e651a843607e9e5d40d8f3489894d4ba
Summary:
dispatchDraw, dispatchGetDisplayList, updateDisplayListIfDirty, recreateChildDisplayList, etc, can all crash internally for a variety of reasons and it can be very tricky to track down the root cause. This isn't a fix, this just adds extra logging to hopefully make debugging easier.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24166149
fbshipit-source-id: 1bbaf34a92a9bcac5a594a25522c66e6e0cc80ca
Summary:
This PR changes ReadableNativeMap.getNullableValue to return null if key not found, instead of throwing exception. This matches method signature and nullable annotation.
## Changelog
[Android] [Changed] - fix ReadableNativeMap.getNullableValue to match signature
Pull Request resolved: https://github.com/facebook/react-native/pull/30121
Test Plan: RNTester app builds and runs as expected, and getNullableValue will return null instead of throwing exception.
Reviewed By: JoshuaGross
Differential Revision: D24164302
Pulled By: fkgozali
fbshipit-source-id: 572c1d4ae5fd493aa0018c2df1dfc7fc91cb4b6b
Summary:
Fix ReadableArray annotations, because these methods throw ArrayIndexOutOfBoundsException instead of null if index is not found.
## Changelog
[Android] [Changed] - fix ReadableArray null annotations. Possibly breaking change for Kotlin apps.
Pull Request resolved: https://github.com/facebook/react-native/pull/30122
Test Plan: RNTester app builds and runs as expected, and show correct type in when used with Kotlin code.
Reviewed By: JoshuaGross
Differential Revision: D24164326
Pulled By: fkgozali
fbshipit-source-id: 0c3f8fa9accbd32cc71c50befe9330e5201643f6
Summary:
When I landed D24042677 (https://github.com/facebook/react-native/commit/030d2c1931fb9ff97f682343914503a1c359e1c4), I had the right idea in spirit but forgot to negate the if statement. Oops.
This means that in non-Fabric, the cached spannable will be updated (potentially causing a crash) and the cached spannable will *never* be updated, meaning that most TextInputs will be measured as 0 in Fabric.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24119952
fbshipit-source-id: dc86137956535e1f2b147bb432d050b3561e2658
Summary:
This is a resubmit of D21499015. It resolves https://github.com/facebook/react-native/pull/28852 and https://github.com/facebook/react-native/issues/27787.
From Harry Yu's original PR (too old to merge now):
Text in TextInputs can't be selected by long press. This happens only when they're inside of FlatLists that are rendered with removeClippedSubview prop set to true on Android.
Fixes https://github.com/facebook/react-native/issues/27787
Issue https://github.com/facebook/react-native/issues/6085 2 years ago had fixed this issue with a quick fix, but the code has since disappeared in another change. It has a longer explanation for why it's fixed, but essentially - the text is assumed to be not selectable since the TextInput is initialized without being attached to the window. We need to explicitly set the text to be selectable on attachment. This change redoes that change with a 1-line fix.
Changelog: [Android] [Fixed] - Fixed TextInput not being selectable in removeClippedSubviews FlatLists
Pull Request resolved: https://github.com/facebook/react-native/pull/28852
Test Plan:
This can be tested with a new toggle in RNTesterApp.
Go to the FlatList in RNTesterApp
Toggle on "removeClippedSubviews"
Try selecting some text in the list header. It should fail without this comment but succeed with it
Reviewed By: sammy-SC
Differential Revision: D24043533
Pulled By: JoshuaGross
fbshipit-source-id: c8e60f8131ccc5f6af31ed976c4184d0a16eb3af
Summary:
iOS Fabric actually ignores the `index` property and just uses parent and child tags to remove the child from a parent. This brings Android slightly closer to iOS: we try to use the index, but if the index is incorrect, we either (1) throw if the child isn't contained in the parent, or (2) find the correct index, and continue.
In debug, this will still crash, so we'll get more signal about why this happens.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24056375
fbshipit-source-id: 07507cc32ad02505d3271fc95ecb45d080109078
Summary:
Adding another method to ReactMarker to log a marker with both a tag and an instanceKey. The instanceKey is used to attach the event to the correct marker instance - this is used already in Java, but not in C++ yet.
The way that ReactMarker is currently set up makes this change a little more complex/confusing. For some reason I'm not totally clear on, we're using C-style exports with some platforms-specific ifdefs in ReactMarker.h (even though the impl is .cpp?). And we swap out the implementation for `logTaggedMarker` at runtime in platform-specific code (JReactMarker and RCTCxxBridge).
In this diff, I just add a new function alongside `logTaggedMarker`, `logTaggedMarkerWithInstanceKey`. I did it this way because I figured modifying `logTaggedMarker` to add an argument would be a breaking change.
Reviewed By: PeteTheHeat
Differential Revision: D23831533
fbshipit-source-id: f5b3eba1f43a80f7723fdb64cfc0a792548db2ba
Summary:
Flow will soon stop allowing generic types to "escape" out of the scope in which they were defined. The fix will be to add annotations to currently-unannotated variables, parameters, and function returns, so that generics don't become inputs to type inference for those positions. This diff adds new type annotations to xplat where possible to minimize the impact of this change.
This diff was generated by running
```
buck run //flow/src/facebook/komodo/binaries:annotate_escaped_generics -- --write ../../xplat/js
```
from within the flow directory, and then reverting changes that led to new errors. Most changes were reverted by running:
```
facebook/flowd check --json --json-version=2 ../../xplat/js &> post-json
jq -f j.jq < post-json | xargs hg revert
```
where `j.jq` is
```
def locs: [.primaryLoc.source, (select(.rootLoc.source != null) | .rootLoc.source), .referenceLocs[].source ] | unique;
[.errors[] | locs[]] | unique | .[]
```
Changelog: [Internal]
Reviewed By: panagosg7
Differential Revision: D24006427
fbshipit-source-id: 0cd6ec8a9611d8b1e9b14c54f9fffd2d7de2fd9e
Summary:
This is a check to execute code only in Fabric, and... it's just wrong. This object is *always* present, for Fabric and non-Fabric. We instead need to check if there's actually a state object, as other parts of the code check for.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24042677
fbshipit-source-id: 5cf6ebc8f07987d917fdf11042d1715876fa8229
Summary:
If removeViewAt crashes, log the children of the parent view, and all of the parent's ancestors.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24019515
fbshipit-source-id: c5b1ca0948ebc47f2648e161770affa8542ca5dd
Summary:
It was recently upgraded to 4.4, so we don't need the 4.3.1 anymore.
Changelog: [Android][Removed] Removed Robolectric 4.3.1 setup
Reviewed By: jselbo
Differential Revision: D24030822
fbshipit-source-id: 09b3c577d32028723e7bbc02f13459a7ae69b749
Lulu Wu
generatedunixname89002005325676
Kevin Gozali
Joshua Gross
Peter Argany
Pasquale Anatriello
Andrei Shikov
generatedunixname89002005325674
Emily Janzer
Dulmandakh
Janic Duplessis
Jason Safaiyeh
Oleksandr Melnykov
Ramanpreet Nara
hy.harry.yu@gmail.com
Mike Vitousek
Joshua Selbo