Summary:
Changelog: [Internal]
Using `empty()` vs `size() == 0` or `size() > 0`.
It is a more semantic way to check whether container is empty or not.
Reviewed By: JoshuaGross
Differential Revision: D21573183
fbshipit-source-id: b83283f687432a037941852114717a0f014e28db
Summary:
Changelog: [Internal]
For consistency, switching west const to east const.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D21574239
fbshipit-source-id: eb3459c63f731f51b24f40f9f80b574661ffd935
Summary: Changelog: [Internal][Yoga] throw std::logic_error instead of aborting the process and convert to java exception for jni layer
Reviewed By: pasqualeanatriello
Differential Revision: D21301235
fbshipit-source-id: 148b27920e62990a271e1d0df8c85a2cc42f4fd4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/28893
`JSIExecutor::callSerializableNativeHook` converts the arguments from `JSI::Value` to `folly::dynamic`. Then, `RCTNativeModule` converts the arguments from `folly::dynamic` to ObjC data structures in its `static invokeInner` function.
Therefore, I decided to start the sync markers inside `JSIExecutor::callSerializableNativeHook`, which required me to expose these two methode `ModuleRegistry::getModuleName` and `ModuleRegistry::getModuleSyncMethodName`. This shouldn't modify performance because we eagerly generate a NativeModule's methods when it's first required. So, at worst, this is doing a cache lookup.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D21443610
fbshipit-source-id: 67cf563b0b06153e56e63ba7e186eea31eafc853
Summary:
NativeModule async method calls are queued up on the JS side, and flushed to C++ on every Native -> JS call. Before we execute the batch of async NativeModule method calls, we convert it (a JS object) from a `jsi::Value` to a `folly::dynamic` object in `JSIExecutor::callNativeModules`. Then, in `JsToNativeBridge::callNativeModules`, we convert this `folly::dynamic` object into an `std::vector<MethodCall>`, before finally looping over these `MethodCall`s and invoking each NativeModule async method call.
The markers I'm adding in this diff measure this `jsi::Value -> folly::dynamic -> std::vector<MethodCall>` pre-processing.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D21435455
fbshipit-source-id: 4c5a9e2b73c1a2a49d7a8f224a0d30afe3a0c79c
Summary:
This diff instruments two markers:
- JSRequireBeginning: From the start of the JS require to when we start creating the platform NativeModule
- JSRequireEnding: From the end of platform NativeModule create to the end of the JS require
In order to accomplish this, I had modify `ModuleRegistry::ModuleRegistry()` to accept a `std::shared_ptr<NativeModulePerfLogger>`. I also had to implement the public method `ModuleRegistry::getNativeModulePerfLogger()` so that `JSINativeModules` could start logging the JS require beginning and ending.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D21418803
fbshipit-source-id: 53828817ae41f23f3f04a95b1d3ac0012735da48
Summary:
## Description
This diff introduces `NativeModulePerfLogger`, its BUCK, Cocoapod, android-ndk targets. This diff also wires up those targets into the React Native bridge and TurboModules targets, so that we get signal on if the code compiles.
This diff also introduces `TurboModulePerfLogger`, which is a namespace that holds the `NativeModulePerfLogger` that'll do perf-logging for TurboModules.
## How will perflogging work on iOS?
1. Each application will, during React Native initialization, create a NativeModule perf logger.
2. If TurboModules are enabled, we'll call `TurboModulePerfLogger::setInstance(perfLogger)`. If TurboModules are disabled, we'll call `NativeModulePerfLogger::setInstance(perfLogger)`.
3. TurboModules, when they're created and used, will log events via `TurboModulePerfLogger::getInstance()`. NativeModules (i.e: bridge modules), when they're created and used, will log events via the `NativeModulePerfLogger::getInstance()`.
> **Note:** The NativeModule system will log events for non-TurboModules as well. Maybe we should log events for only NativeModules that conform to the `TurboModule` interface, when TurboModules are disabled. This'll ensure a fair comparison between the two systems.
## How will perflogging work on Android?
Please see the subsequent diff.
allow-large-files
Changelog:
[Both][Added] - Introduce `NativeModulePerfLogger`
Reviewed By: PeteTheHeat
Differential Revision: D21318053
fbshipit-source-id: 6ddf5b5a80bdc4076d2dd6588067e2b0ec8c2c6b
Summary:
The interface of css/TextLayoutManager now matches acual platform-specific implementations, so now CXX tests compiles and usable.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21518656
fbshipit-source-id: cece3bea14c70410eea22abafb424f7a2cb201c0
Summary:
Each ComponentDescriptor becomes capable of doing its own interpolation over props for animation purposes.
This new custom interpolator is called by default by the ConcreteComponentDescriptor, but `ComponentDescriptor::interpolateProps` is a virtual function and each ComponentDescriptor can provide custom interpolation if necessary.
For now, only View does any actual interpolation, to support LayoutAnimations.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20965310
fbshipit-source-id: e1c1588107848e94c155efecb0da1cc1619ae544
Summary:
I noticed an issue when the hermes debugger was attached. Console messages would no longer appear in the terminal. This is because the hermes inspector overrides the console object with its own to intercept messages and send them to the debug client.
With this change I made it so that messages are sent to the original console as well instead of completely replacing it.
Changelog:
[General][Fixed] - When Hermes debugger is enabled continue to send log messages to the console
Reviewed By: mhorowitz
Differential Revision: D21509895
fbshipit-source-id: 6d91c4b82682e404679533be14b3e5f12e687e79
Summary:
TextInlineViews in Android was incorrectly converting values to from float to int, this produced to loose precision and to render incomplete texts in some components.
This diff changed the types from int to float, avoiding loose precision.
The impact of this bug is not that high because in the conversion to int we were using Math.ceil(), which was already rounding the result to the next pixel.
changeLog: [Android][Fixed] Fix precision of TextInlineViews in Fabric Android
Reviewed By: JoshuaGross, shergin
Differential Revision: D21541159
fbshipit-source-id: 4741ab96964c35af1c1b7d3e821e505ecef2efce
Summary:
The previous implementation was incorrect causing a memory leak. In the new approach, we use a shared pointer to optional to be able to construct that ahead of time and then fill with actual value.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21516794
fbshipit-source-id: ddcbf8a1ad2b1f629ae4ff2842edeb7bb2a275a6
Summary:
Changelog: [internal]
Fabric asks server for images of their real size and scale, not images of size that they would take up on the screen once rendered. Also scale of the screen is incorrect. This causes images to be twice as large as they have to be.
Look at the size of the first image size here in Paper, it is `{310.5, 207}`.
{F235394066}
If you compare it with Fabric, there it is `{800, 381}`
{F235394115}
It isn't just the size, but scale of request image as well. Fabric always asks for image of scale 1, unlike Paper which takes screen scale into account.
Reviewed By: shergin
Differential Revision: D21255794
fbshipit-source-id: 9db3ccafec1c09cedc5db5ac0a435a28a4c30c85
Summary:
The approach is quite simple: we check the thread id, and if it matches the caller thread id, we unlock mutexes which lead to the normal uninterrupted execution flow.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: RSNara
Differential Revision: D21328313
fbshipit-source-id: 4290b8a0357dbad3563d7da9464c03ecce5ded7c
Summary:
This diff implements iOS-specific `PlatformRunLoopObserver` (and `MainRunLoopObserver`) that then being glued together with `SynchronousEventBeat` replaces `MainRunLoopEventBeat`, and then same thing glued with `AsynchronousEventBeat` replaces `RuntimeEventBeat`.
So, instead of two platform-specific classes we had on iOS (for that needs), now we have only one (that can be reused for a more broad variety of applications).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21341998
fbshipit-source-id: fafde4e678770f7fcf9c1ff87acc02812a37e708
Summary:
We use `EventBeat::OwnerBox` in the `EventBeat` infra to represent weak ownership of classes that contain that, so those classes can ensure own life-time at some critical points.
Before this diff, we stored a pointer to EventDispatcher right after we create it. However, some components that we build require to have a valid pointer from the beginning (even before the EventDispatcher is created). To satisfy this requirement, we create a dummy pointer first and use it as an owner, then we merge (share control block) the pointer to an EventDispatcher into that. It works because the owner pointer never gets dereferenced (it's `void *`), so it does not matter which object it represents while it represents correct ownership.
In the future, this approach will allow us to remove the concept of `OwnerBox` completely and use just `std::weak_ptr<void>` instead.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D21441109
fbshipit-source-id: 7457c77bd31cd577f38a26e28e27eb7e33b6ad24
Summary:
The name of the value was incorrect.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D21496651
fbshipit-source-id: 464c98436bb8d5f2b6275b7eab1c32d187e2b23c
Summary:
We should check for corner cases.
Another interesting detail is behavior. Now (and previously) we return nullptr if case we could not find the most recent node. But maybe we should return the given node instead?
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D21480874
fbshipit-source-id: bb943c4508ec025316a3665d652e4b5e06dd795c
Summary:
We cannot use a raw pointer here because the tree might progress right after we call to `shadowTree.tryCommit` and we will get a dangling pointer as a result.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D21464832
fbshipit-source-id: 3998b39d29db672da54d4adcb6be55cd3d44d862
Summary:
In this method we have to maintain a retaining pointer to the node in order to access it as a raw pointer.
See also a comment in the codee.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D21464834
fbshipit-source-id: 69f6894009509e5feca291fab12c019208933816
Summary:
There is no need to use a raw pointer to a shared pointer as a return value of `UIManager::getNewestCloneOfShadowNode`. If it would be a very hot path, we could you a raw pointer to the node instead but it's not a hot path.
Prooving that using a raw pointer here is safe is quite complex (and I don't know if it's safe or not). So, I changed it to just a shared pointer.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D21464833
fbshipit-source-id: 813a3c9fca0147afb322db6855a0ce8fd2d47909
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/28851
This diff creates a RuntimeExecutor that uses the bridge and exposes it on CatalystInstanceImpl.
Changelog: [Internal]
Reviewed By: mdvacca, RSNara
Differential Revision: D21051949
fbshipit-source-id: b3977fc14fa19089f33e297d29cedba0d067526d
Summary:
This diff adds support for the "beforeScriptWithSourceMapExecution" instrumentation
breakpoint via "Debugger.setInstrumentationBreakpoint".
CDP describes it as a breakpoint, but we just set a flag in the inspector. A
fake breakpoint ID is synthesized for optional removal later.
Changelog: [Internal] Add Debugger.setInstrumentationBreakpoint to Hermes Inspector
Reviewed By: mhorowitz
Differential Revision: D21418218
fbshipit-source-id: 90fa49c0954980993815322d3a7effee416ed5db
Summary:
This removes the Hermes.setPauseOnLoad. It will be replaced by the more standard
Debugger.setInstrumentationBreakpoint's "beforeScriptExecution" event.
ChangeLog: [Internal] Remove Hermes.setPauseOnLoad message (to be replaced)
Reviewed By: mhorowitz
Differential Revision: D21418219
fbshipit-source-id: 93c53801c23487f9336b322c2bd737663ec21b97
Summary:
This diff exposes the Text.includeFontPadding prop to java, then it uses the prop to calculate the height of Text components correctly.
changelog: [Internal][Fabric] Internal change in Fabric to support Text.includeFontPadding prop in fabric
Reviewed By: shergin
Differential Revision: D21446737
fbshipit-source-id: efe73fb6b0d402c3275ac8c012fa8fa06b743bdd
Summary:
This diff extends the ParagraphAttribute class to store the value of the includeFontPadding prop.
Note that this is an Android only prop, I'm not creating android blocks to improve "cleanliness" of the code.
changelog: [Internal][Fabric] Internal change in Fabric to support Text.includeFontPadding prop in fabric
Reviewed By: shergin
Differential Revision: D21446738
fbshipit-source-id: 0543e86aa18ce10f7a56bbaafe111cce0179ea86
Summary:
We consume Hermes through multiple .so's, which means we have multiple (weak) typeinfo definitions of facebook::jsi::JSError. Previously we were using gnustl, which would strcmp typeinfo to decide whether a certain exception handler applies, which meant this didn't cause any major issues. However since this is deprecated, we recently switched to libc++, which does not have this by behaviour (or it does, but behind a flag I'm not sure how to enable). This causes any JS exceptions to fall through from our exception handlers and fatal the app.
This problem is actually documented in the common Android NDK problems page: https://android.googlesource.com/platform/ndk/+/master/docs/user/common_problems.md#rtti_exceptions-not-working-across-library-boundaries
The suggested solution is to ensure that any exception types have a key function defined (a non-pure, out-of-line virtual function). The simplest one to add is a virtual destructor. This makes the object file that holds the implementation of the destructor export a non-weak typeinfo definition which will at load time override the other weak versions.
I'm not sure why we're the first to hit this. RN's JSIExecutor doesn't explicitly reference JSError which probably helps (https://github.com/facebook/react-native/blob/master/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp#L256-L258) and they also don't use unguarded callbacks like we do.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D21426524
fbshipit-source-id: 474284ada1ca2810045dc4402c420879447f9308
Summary:
Move and create an empty rule that redirects as well, to handle //arvr rules
Need to do this way, since ovrsource sync rules are in different repo.
allow_many_files
allow-large-files
Steps:
- [X] Move glog from xplat/third-party to /third-party
- [ ] Update references in ovrsource to translate to //third-party instead of //xplat/third-party
- [ ] Get rid of temporary rule
- [ ] Update fbsource/third-party/glog to 0.3.5 (what we have in ovrsource)
Changelog: [Internal] Update reference for glog from xplat/third-party to /third-party.
Reviewed By: yfeldblum
Differential Revision: D21363584
fbshipit-source-id: c1ffe2dd615077170b03d98dcfb77121537793c9
Summary:
`SynchronousEventBeat` and `AsynchronousEventBeat` are a cross-platform re-implementation of run loop related parts of `MainRunLoopEventBeat` and `RuntimeEventBeat` (iOS specific classes for now). In the future, they will replace iOS- and Android-specifc event beat classes.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21341996
fbshipit-source-id: 8eda9a5df537cd666b7728e32212a8bb5ddb3ab7
Summary:
`RunLoopObserver` is one of the core interfaces that bridge intrinsically platform-specific functionality to cross-platform React Native core. `RunLoopObserver` allows subscribing for notifications about changes in a run loop life cycle. Primarily it supposed to be used for observing UI (aka main) and JavaScript execution thread/run-loop.
Having a `RunLoopObserver` implemented in a platform-specific manner allows building these components in a cross-platform manner:
* Sync and async UI event delivery pipeline;
* Timing for some animation engine;
* Timers (probably additional features are required).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21341997
fbshipit-source-id: 7ef61fb51f550dd0f2e89c64af657e0f0de029aa
Summary:
Before this change, in case of incorrect measurements, Fabric's implementation of `measure` and `measureInWindow` incorrectly returned negative height and width. Now it returns zeros (as classic React Native does).
Fabric:
This does not fix `measureLayout` called for virtual nodes. This is not so trivially to fix and it will be done separately.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, yungsters, mdvacca
Differential Revision: D21433239
fbshipit-source-id: fbaf5ee35c690506822c634daac4426542c2cdcf
Summary:
We are currently very strict about breakpoint location matching. This
diff allows some fuzz in the column, but not in the line.
Changelog: [Internal] Setting Hermes breakpoints no longer requires exact column match
Reviewed By: avp
Differential Revision: D21343198
fbshipit-source-id: a59786a9d63f9fe1ed576835ed660ba3343affe1
Summary:
The core issue solved in D21389371 was that erased elements of a TinyMap were being iterated over, because TinyMap has somewhat-complicated logic around cleaning out the underlying vector. In some very marginal cases, vectors were not being cleaned and an iterator pointing at erased elements was being returned.
The diff prevents some possible crashes in `begin()` and `find()` while making it much less likely to iterate over erased elements.
We also add a unit test to catch the case fixed in D21389371, in particular.
We also are keeping the code added in D21389371 (for now) since it's a cheap check, and will be a safeguard until we have rigorous testing around TinyMap. To be clear that logic should noop currently, but will prevent crashes in case guarantees around TinyMap change in the future.
Currently there is only one line of code that actually uses the TinyMap iterator, so this should be safe.
Reviewed By: shergin
Differential Revision: D21392762
fbshipit-source-id: 36dc998958c230fad01af93338974f8889cbcf55
Summary:
Before, compilation fails with P130281113. After fixing BUCK target, fails with P130281201.
After all changes, the tests succeed.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21390996
fbshipit-source-id: c85aa43b3ef7fc9642d226ae706c937b2a5a408d
Summary:
While #import is common in Objective C, it's a vendor specific extension in C++, only supported by GCC/Clang, and only when -pedantic is off. Its use causes build breaks with MSVC. Replace it with the standard #include.
## Changelog
[Internal] [Fixed] - Don't use #import in C++ Code
Pull Request resolved: https://github.com/facebook/react-native/pull/28825
Test Plan: We've ben running this change within react-native-windows for some time.
Differential Revision: D21391233
Pulled By: shergin
fbshipit-source-id: c0f94f314c46d6ac24067bbdcd5aaaeec9da283f
Summary:
When we call `.erase` on the TinyMap, it sets the key of the element to 0. When we call `.begin()` later, TinyMap will sometimes, but not always, clean up the underlying Vector. This means that *most* of the time, underlying erased elements will be removed from the Vector; but sometimes erased elements will still be there when iterating over it.
This was causing us to generate extra "Create" mutations.
To fix this, for now we just check for zeroed-out elements when iterating over the vector.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21389371
fbshipit-source-id: 1e641050987d40a3f3e31499dcb373cfb28ae6f8
Summary:
In D20551411 we (I) replaced `std::mutex` with `std::recursive_mutex` in `executeSynchronouslyOnSameThread_CAN_DEADLOCK` in order to support synchronous `RuntimeExecutor`. It turned out that the idea was cool but totally incorrect. Quite obviously, double locking recursive mutex on the same thread does nothing, so the whole setup does not work at all.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21328072
fbshipit-source-id: 7f1f81de61c36adb167fe624ada3bd71913bdf7c
Summary:
The change is gated by two QEs (iOS and Android).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D21317797
fbshipit-source-id: 34bfd9546600d9cb3467bddfc61363a16683f81f
Samuel Susla
Sidharth Guglani
Ramanpreet Nara
Valentin Shergin
Joshua Gross
Martin Sherburn
David Vacca
Vitaly Potlov
Kevin Gozali
Manvir Singh
Emily Janzer
Will Holen
Pieter De Baets
Nikita Lutsenko
Nick Gerleman
