Summary:
Changelog: [Internal]
A long time ago we experimented with JSC bytecode. We are not experimenting with JSC bytecode any more. This code can be removed.
Reviewed By: mhorowitz
Differential Revision: D22017374
fbshipit-source-id: 6fe3fb7ad7966f92a5cd103605ac5c0bd1f17a8e
Summary:
Fix crashes in LayoutAnimations:
1) It is valid to omit create, update, delete configs
2) When extracting SRT from a matrix, ignoring skew properties
3) Provide valid telemetry from LayoutAnimations transactions
Unrelated to crashes: to help debugging and until onSuccess/onError callbacks are working, log any configuration parsing errors.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22050736
fbshipit-source-id: e59418ecad0f9bfd20a2b4976557e39020c2d101
Summary:
This diff fixes a race condition that caused a flicker during initial rendering of TextInput in Fabric
The root cause is that the TextInput's State is sometimes initialized with no information from the Theme, this causes text input to be rendered with 0 padding. Later ReactTextInput manager updates TextInput state with theme data and the TextInput is re-rendered with the correct padding information.
changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D22034849
fbshipit-source-id: a2fa91f34a8340878f2ec8d231ef6c86cee08f05
Summary:
This pull request updates the Podspecs to reflect recent changes to Fabric, so they build with Fabric enabled again.
In `React-Fabric.podspec`, subspecs for the views `legacyviewmanagerinterop`, `safeareaview`, `textinput`, and `unimplementedview` are added. A minor tweak to the subspec for `rncore` was added blacklisting two codegen-generated files that have issues building, but are not actually needed yet and are empty.
In `React-graphics.podspec`, the header paths for Boost were added.
This is partially a retread of https://github.com/facebook/react-native/issues/26805, which had to be backed out due to issues with the changes to the Yoga podspec.
## Changelog
[Internal] [Fixed] - Fixed building Fabric with the Cocoapods
Pull Request resolved: https://github.com/facebook/react-native/pull/28584
Test Plan: RNTester can now compile.
Reviewed By: shergin
Differential Revision: D20966065
Pulled By: hramos
fbshipit-source-id: 73931be63b72fcb52705643f81c0f787e571eb76
Summary:
Changelog: [Internal]
Round calculated text size to closest larger pixel size.
In Paper we do this as well in https://fburl.com/diffusion/w2pj6ea0
Why do we need this?
For example, we calculate height of the text to be 16.707235, yoga takes this value and assigns text to have height 17 anyway.
I assume Yoga uses this extra 0.3 points of height to move other things around a little bit because Yoga doesn't deal with exact values.
Reviewed By: shergin
Differential Revision: D21999032
fbshipit-source-id: 73923dc3e27fa110adb3f76cfa635e0bfdc672d4
Summary:
There is no good reason why they are `const` and `private`, in the future diffs we will need to mutate them.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21992197
fbshipit-source-id: f8aee8db9ea1ff8282b38fbe3a966911678ee2c4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/29090
Moving the logic for calling into JS to handle errors into ErrorUtils, where it can be reused outside of the bridge.
Reviewed By: RSNara
Differential Revision: D21939254
fbshipit-source-id: 0d8f3bd2503720be7619ed8dc8b2389f544049f3
Summary:
While I was investigated the previous issue (T59430348) I found this place where we manipulate managed pointers manually. So, to make it error-prone this diff changes it to use `wrapManagedObject`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D21943010
fbshipit-source-id: ecaeffb419ae8d4880187027ca7ec9563e0dfd46
Summary:
ManagedObjectWrapper (`wrapManagedObject`) is used to pass pointers managed by Objective-C runtime object thought C++ parts of the framework. The wrapper consists of a shared pointer to an object with a custom deleter that calls `CFRelease`.
Apparently, there is a caveat here: shared ptr implementation always calls a deleter even if the object points to `nullptr`. It's usually fine because in C++ calling `delete` on a nullptr is a no-op, not an error. But it's an error from the `CFRelease` perspective, which checks the pointer and crashes if it's nullptr.
More info:
https://stackoverflow.com/questions/1135214/why-would-cfreleasenull-crashhttps://stackoverflow.com/questions/50201967/why-unique-ptr-with-custom-deleter-wont-work-for-nullptr-while-shared-ptr-does
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21943011
fbshipit-source-id: 442ad5e274a146de112e6bd8f3c2d20f0225bf77
Summary:
Changelog: [Internal]
`CFRelease` crashes app if `cf` is `NULL`, `CGColorRelease` doesn't.
From:
https://developer.apple.com/documentation/coregraphics/1586340-cgcolorrelease?language=objc#
Even though this hasn't happened yet, we had a similar crash in D21943011. Also `CGColorCreate` returns nullable so it could happen.
In order to prevent this in the future, let's switch to `CGColorRelease` which doesn't crash if `cf` is `NULL` and also is semantically correct.
Reviewed By: shergin
Differential Revision: D21953404
fbshipit-source-id: 8b969ee345c2507fcba2442fa20e3fdaf2235d8c
Summary:
Changelog: [Internal]
View gets flattened even though it has `display: none` and therefore it and its children do not get hidden.
Reviewed By: shergin, mdvacca
Differential Revision: D21929033
fbshipit-source-id: 994a79fb64fbe66273a70218ebe8056d92cd3cd4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/29087
D21908523 added an implicit dependency on `jsi.h` to use functions like `asObject`, etc. For some reason this doesn't break the build with BUCK (??) but it does with cocoapods. Adding the dep to the cxxreact podspec and regenerating offline mirrors to unbreak CircleCI. Also adding the BUCK dep and include statement for good measure.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21924592
fbshipit-source-id: 295c0670c6499e1195ba3c3a3320c6aee13bc025
Summary:
Changelog: [Internal]
The crash is caused by dereferencing invalid pointer.
This can happen because `UIManager` can outlive `RCTScheduler` and `Scheduler`.
Here we make sure when `Scheduler` is being deconstructed, UIManager's pointer is invalidated.
I don't think this is ideal solution but it should fix the crash.
Ideally we want the owner of animation delegate to invalidate the pointer.
Reviewed By: JoshuaGross
Differential Revision: D21922910
fbshipit-source-id: b2a56c1104574cecebaffad1bcbcbff82c1fa0cf
Summary:
Call into our existing JS error handling logic from C++ using JSI in the RuntimeExecutor used by the bridge.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D21908523
fbshipit-source-id: ae41196443781b9f2673dcb7bbcb5b5aa8aa2528
Summary:
Changelog: [Internal]
# Problem
`MountingCoordinator` holds a pointer to instance of `MountingOverrideDelegate` which becomes invalid.
# Solution
Use `std::weak_ptr` instead of raw pointer so it is possible to tell whether the pointer is expired.
Reviewed By: JoshuaGross
Differential Revision: D21905351
fbshipit-source-id: c7bf9635742a6ec086a03ba83202e46e1f1f373f
Summary:
This diff adds support elevation prop in Fabric core, additionally it adds this prop as non collapsable on the view flattening algorithm
changelog: [Internal] internal change in fabric to support elevation prop
Reviewed By: JoshuaGross
Differential Revision: D21896465
fbshipit-source-id: e0854acc0b2ac30eaf3f82d615aab1cf378cc530
Summary:
JSError creation can lead to further errors. Make sure these cases
are handled and don't cause weird crashes or other issues.
This solution has a few parts:
* include a ScopedNativeDepthTracker in checkStatus
* If an exception object message or stack property is already a String, don't
call JS String ctor on it
* Verify that a jsi::Value is a String before calling getString on it.
* Add more tests for JSError construction
Changelog: [Internal]
Reviewed By: dulinriley
Differential Revision: D21851645
fbshipit-source-id: 2d10da0e741ad4ede93cd806320f68ad512e5138
Summary:
There are two ways to make a NativeModule method execute synchronously:
- Declare the NativeModule method to be synchronous (i.e: use `RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD`).
- Make the NativeModule synchronous (i.e: make its method queue `RCTJSThread`). This way, all its methods are synchronous.
`RCTNativeModule` executes all synchronous methods on the JS thread:
- Executing an async methods on a sync module: https://git.io/JfRPj
- Executing a sync method: https://git.io/JfRXe
However, in TurboModules we block the JS thread, execute the method on the NativeModule's method queue, and then unblock the JS thread. While this approach is thread-safe, and arguably the correct way to dispatch sync methods, it's also much slower than the alternative. Therefore, this diff migrates the legacy behaviour to the TurboModule system.
## Special Case: getConstants()
When an ObjC NativeModule requires main queue setup, and it exports constants, we execute its `constantsToExport` method on the main queue (see: [RCTModuleData gatherConstants](https://github.com/facebook/react-native/blob/c8d678abcf93fd3f6daf4bebfdf25937995c1fdf/React/Base/RCTModuleData.mm#L392-L402)). I replicated this behaviour with TurboModules.
Changelog:
[iOS][Fixed] - Execute ObjC TurboModule async method calls on JS thread for sync modules
Reviewed By: fkgozali
Differential Revision: D21602096
fbshipit-source-id: 42d07b7ad000abeac27091dc3ec440e3836d2eae
Summary:
If a NativeModule requires main queue setup, its `getConstants()` method must be executed on the main thead. The legacy NativeModule infra takes care of this for us. With TurboModules, however, all synchronous methods, including `getConstants()`, execute on the JS thread. Therefore, if a TurboModule requires main queue setup, and exports constants, we must execute its `getConstants()` method body on the main queue explicitly.
**Notes:**
- The changes in this diff should be a noop when TurboModules is off, because `RCTUnsafeExecuteOnMainQueueSync` synchronously execute its block on the current thread if the current thread is the main thread.
- If a NativeModule doens't have the `requiresMainQueueSetup` method, but has the `constantsToExport` method, both NativeModules and TurboModules assume that it requires main queue setup.
## Script
```
const exec = require("../lib/exec");
const abspath = require("../lib/abspath");
const relpath = require("../lib/relpath");
const readFile = (filename) => require("fs").readFileSync(filename, "utf8");
const writeFile = (filename, content) =>
require("fs").writeFileSync(filename, content);
function main() {
const tmFiles = exec("cd ~/fbsource && xbgs -n 10000 -l constantsToExport")
.split("\n")
.filter(Boolean);
const filesWithoutConstantsToExport = [];
const filesWithConstantsToExportButNotGetConstants = [];
const filesExplicitlyNotRequiringMainQueueSetup = [];
tmFiles
.filter((filename) => {
if (filename.includes("microsoft-fork-of-react-native")) {
return false;
}
return /\.mm?$/.test(filename);
})
.map(abspath)
.forEach((filename) => {
const code = readFile(filename);
const relFilename = relpath(filename);
if (!/constantsToExport\s*{/.test(code)) {
filesWithoutConstantsToExport.push(relFilename);
return;
}
if (!/getConstants\s*{/.test(code)) {
filesWithConstantsToExportButNotGetConstants.push(relFilename);
return;
}
if (/requiresMainQueueSetup\s*{/.test(code)) {
const requiresMainQueueSetupRegex = /requiresMainQueueSetup\s*{\s*return\s+(?<requiresMainQueueSetup>YES|NO)/;
const requiresMainQueueSetupRegexMatch = requiresMainQueueSetupRegex.exec(
code
);
if (!requiresMainQueueSetupRegexMatch) {
throw new Error(
"Detected requiresMainQueueSetup method in file " +
relFilename +
" but was unable to parse the method return value"
);
}
const {
requiresMainQueueSetup,
} = requiresMainQueueSetupRegexMatch.groups;
if (requiresMainQueueSetup == "NO") {
filesExplicitlyNotRequiringMainQueueSetup.push(relFilename);
return;
}
}
const getConstantsTypeRegex = () => /-\s*\((?<type>.*)\)getConstants\s*{/;
const getConstantsTypeRegexMatch = getConstantsTypeRegex().exec(code);
if (!getConstantsTypeRegexMatch) {
throw new Error(
`Failed to parse return type of getConstants method in file ${relFilename}`
);
}
const getConstantsType = getConstantsTypeRegexMatch.groups.type;
const getConstantsBody = code
.split(getConstantsTypeRegex())[2]
.split("\n}")[0];
const newGetConstantsBody = `
__block ${getConstantsType} constants;
RCTUnsafeExecuteOnMainQueueSync(^{${getConstantsBody
.replace(/\n/g, "\n ")
.replace(/_bridge/g, "self->_bridge")
.replace(/return /g, "constants = ")}
});
return constants;
`;
writeFile(
filename,
code
.replace(getConstantsBody, newGetConstantsBody)
.replace("#import", "#import <React/RCTUtils.h>\n#import")
);
});
console.log("Files without constantsToExport: ");
filesWithoutConstantsToExport.forEach((file) => console.log(file));
console.log();
console.log("Files with constantsToExport but no getConstants: ");
filesWithConstantsToExportButNotGetConstants.forEach((file) =>
console.log(file)
);
console.log();
console.log("Files with requiresMainQueueSetup = NO: ");
filesExplicitlyNotRequiringMainQueueSetup.forEach((file) =>
console.log(file)
);
}
if (!module.parent) {
main();
}
```
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D21797048
fbshipit-source-id: a822a858fecdbe976e6197f8339e509dc7cd917f
Summary:
At some early stages of Instance initialization, it does not have a `nativeToJsBridge_`. At the same time, `handleMemoryPressure` can be called at any point in time, so we should check the pointer for not being null before calling on it.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D21798522
fbshipit-source-id: 6384da88784cceb493cf9810408cbb47777d3f4b
Summary:
This diff extends the measurement of Text components in order to support empty strings.
This is required for parity with Paper.
I created a follow up task to analyze support of empty string as part of the Text infrastructure of Fabric in the future.
changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D21761171
fbshipit-source-id: d2aa074052b09732af5d35723f19014090fcabbf
Summary:
`getStackTracesTreeNodeForAlloc` only works for memory that was tracked with `newAlloc`.
For now, that is only memory that goes through GC APIs.
Various places were calling this function on native memory which was allocated
with `malloc`, not GC memory. This led to SIGSEGV on nullptr when trying to take heap profiles
of these objects.
`newAlloc` could, in theory, work with native memory as well. But building out support for that
is outside the scope of this fix.
Changelog: [Internal]
Reviewed By: jbower-fb
Differential Revision: D21667842
fbshipit-source-id: 8403a6668e5ec607972ce6819f78fedb89da3f37
Summary:
Index adjustment doesn't work if virtual views are inserted, because those don't actually result in the view hierarchy being mutated, as such.
Add an Android-specific check to solve the crash.
I don't love adding platform-specific checks here, so I'm considering wrapping this logic in a platform-specific class, so that logic lives outside of the core of LayoutAnimations and entirely in platform code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21727805
fbshipit-source-id: 5af2cf479beaa4d0e9d94ea16ac989c4268920f8
Summary:
property and type are optional params according to Flow, so we should treat as such.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21725746
fbshipit-source-id: 3c48a8cef8fa5911c195f582556de6dad871c4f1
Summary:
A key difference in WeakRefs with the Hades GC is that they are mutable,
in the sense that reading the value of a WeakRef while a GC is active might
clear the WeakRef.
For this reason, attempting to lock the WeakObject might mutate the backing
reference.
I preferred to communicate this change in behavior via the API rather than
`const_cast` it away inside the implementation.
Changelog: [Internal] Change jsi::WeakObject to be mutable in lockWeakObject
Reviewed By: mhorowitz
Differential Revision: D21485758
fbshipit-source-id: 3618928be8f8791aed56cb20673896ff5b786ded
Summary:
Simple diff to add ART components into Catalyst app Android
changelog: [Internal] Internal changes to add support of ART for Fabric
Reviewed By: JoshuaGross
Differential Revision: D21621479
fbshipit-source-id: d957c25f447d19d8f349c69aa20f5f19237d867a
Summary:
Here I'm implementing equality methods for ARTGroup, ARTShape and ARTText and I'm using these methods to update the state only when it is necessary.
This will improve perf in rendering of ART
changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D21695127
fbshipit-source-id: b438ddea4c34bd7a0bdf26a6aac4fd62a9f78b49
Summary:
Element, Shape, Group, Text are too generic, we are renaming these classes to ARTElement, ARTBGroup, ARTShape...
changelog: [Internal] internal changes to support ART in android
Reviewed By: JoshuaGross
Differential Revision: D21681878
fbshipit-source-id: f6b35443486a3db210f61dcaf91bd32df47fbc66
Summary:
This diff adds support for Text in ART Fabric Android
changelog: [Internal] internal changes to fully support ART in Fabric
Reviewed By: JoshuaGross
Differential Revision: D21681877
fbshipit-source-id: c92e642cff56b71f8ee8f4eb9af6eea6c490f6c7
Summary:
This diff implements the serialization of Text components to send data from C++ to java
changelog: [Internal] internal changes to support ART in fabric
Reviewed By: JoshuaGross
Differential Revision: D21681875
fbshipit-source-id: eba31f35c95e0a2d3226ec70421832719083d7fa
Summary:
This diff refactors the types of ART Text classes, this is necessary on the next diffs of the stack
closeoncommit
changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D21681876
fbshipit-source-id: ea438e89df6d860b3ff8bbdae657ca123b417a1b
Summary:
Changelog: [Internal]
`BatchedEventQueue::enqueueUniqueEvent` goes over event queue and deletes previous event of the same type and same target.
This is useful for ScrollView for example where only the latest event is relevant.
This only affects ScrollView scroll event, other events take the original code path.
Reviewed By: mdvacca
Differential Revision: D21648906
fbshipit-source-id: a80ad652058fd50ebb55e24a87229cdc1764b591
Summary:
Changelog: [Internal]
Separates EventQueues from an array into 4 ivars.
EventQueues are of different type, in the future we will want to call different methods on different kind of EventQueue.
Reviewed By: shergin
Differential Revision: D21648905
fbshipit-source-id: 90ae65edb8a9276eecfea9770f554d8c56804797
Summary:
Changelog: [Internal]
# Problem
Yoga internally uses address of owner to determine whether a node should be cloned or it shouldn't.
During layout, it traverses the tree and looks whether current parent is equal to child's owner. If it isn't it means the yoga node is shared between 2+ trees and need to be cloned before mutated. Parent in yoga is stored as reference to the parent.
We can run into an issue where yoga node is shared between 2+ trees, but its current parent is equal to child's owner. This is because we reallocate new parent at address where its previous parent lived. This happens over few iterations, the old parent is first deallocated.
This is known as ABA problem.
# Solution
When we are cloning node, we loop over all children to see whether any of the is not using address of new `yogaNode_` as its owner. At this point we know this is accidental and set its owner to `0xBADC0FFEE0DDF00D`.
We chose `0xBADC0FFEE0DDF00D` because when someone is debugging this in LLDB and prints this address, it will hint them that it was artificially set and can string search for it in the codebase.
Reviewed By: shergin
Differential Revision: D21641096
fbshipit-source-id: c8b1b4487ea02b367f5831c1cdac055bce79c856
Summary:
This diff serializes ART state into folly::dynamic. this is necessary to send this data to Android
changelog: [Internal]
Reviewed By: shergin
Differential Revision: D21657608
fbshipit-source-id: 6c1b69af7d1dbe7de15e509f83c508a38294d89e
Summary:
This diff replaces the usage of int to represent the type of elements for an Enum
changelog: [Internal] Internal change to support ART in fabric
Reviewed By: shergin
Differential Revision: D21657706
fbshipit-source-id: 7bda0210d50136477f0524695d5406e35074f09c
Summary:
This diff integrates ART state into ARTSurfaceViewShadowNode
changelog: [Internal]
Reviewed By: shergin
Differential Revision: D21657611
fbshipit-source-id: 06bd4d610e2c52e0ef3bca423b93c9ad2318e8df
Summary:
This diff creates the internal state of ART based on its shadow nodes
changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D21657607
fbshipit-source-id: 0a15e90ee7465bf3a2b1001ff9d3198eb22fd708
Summary:
This diff introduces a set of classes that are going to be used to represent the internal State of ART nodes
changeLog: [Internal][Android] Internal change to support ART in Fabric
Reviewed By: JoshuaGross, shergin
Differential Revision: D21657612
fbshipit-source-id: ea6d94b06807ff02d222dfa129a1cae384dceeaa
Summary:
## Motivation
We got this crash T67304907, which shows a `EXC_BAD_ACCESS / KERN_INVALID_ADDRESS` when calling this line:
```
NativeModulePerfLogger::getInstance().asyncMethodCallBatchPreprocessStart();
```
There are no arguments in that call, so I figured the only error could be when we try to invoke `getInstance()` or `asyncMethodCallBatchPreprocessStart()`.
This diff:
1. Removes the `NativeModulePerfLogger::getInstance()` bit. Now NativeModulePerfLogger is used via regular static C functions. So, there's no way that simply invoking one of the logging functions crashes the application: there's no vtable lookup.
2. Inside each logging function, when perf-logging is disabled, the global perflogger should be `nullptr`. This diff makes it so that in that case, we won't execute any code in the control group of the perf-logging experiment.
## Changes
**How do we enable NativeModule perf-logging?**
- Previously:
- `NativeModulePerfLogger::setInstance(std::make_shared<FBReactNativeModulePerfLogger>(...))`
- `TurboModulePerfLogger::setInstance(std::make_shared<FBReactNativeModulePerfLogger>(...))`.
- Now:
- `BridgeNativeModulePerfLogger::enableLogging(std::make_unique<FBReactNativeModulePerfLogger>(...))`
- `TurboModulePerfLogger::enableLogging(std::make_unique<FBReactNativeModulePerfLogger>(...))`
**How do we do NativeModule perf-logging now?**
- Previously:
- `NativeModulePerfLogger::getInstance().command(...args)`
- `TurboModulePerfLogger::getInstance().command(...args)`.
- Now:
- `BridgeNativeModulePerfLogger::command(...args)`
- `TurboModulePerfLogger::command(...args)`.
The benefit of this approach is that each method in `BridgeNativeModulePerfLogger` is guarded with an if check. Example:
```
void moduleCreateConstructStart(const char *moduleName, int32_t id) {
NativeModulePerfLogger *logger = g_perfLogger.get();
if (logger != nullptr) {
logger->moduleCreateConstructStart(moduleName, id);
}
}
```
Therefore, we don't actually execute any code when perf-logging is disabled.
Changelog:
[Internal]
Reviewed By: fkgozali
Differential Revision: D21669888
fbshipit-source-id: 80c73754c430ce787404b563878bad146295e01f
Christoph Nakazawa
Joshua Gross
David Vacca
Samuel Susla
empyrical
Valentin Shergin
Emily Janzer
Ryan Tremblay
Sidharth Guglani
Paige Sun
Marc Horowitz
Ramanpreet Nara
Riley Dulin
Christoph Purrer
Kevin Gozali
generatedunixname89002005287564