Commit Graph
11 Commits
Author SHA1 Message Date
generatedunixname89002005287564 3f85b83653 Daily arc lint --take CLANGFORMAT
Reviewed By: zertosh

Differential Revision: D23811656

fbshipit-source-id: 4104948d2e4db786998320bcfdb1598d4a497f2d
2020-09-21 04:38:18 -07:00
Valentin Shergin 57dd48b246 Fabric: Marking all JS function lambdas noexcept in UIManagerBinding
Summary:
Exceptions in C++ work quite differently from exceptions in other languages. To make exceptions actually work **correctly** all the code needs to be written with "exceptions in mind" (e.g., see https://www.stroustrup.com/except.pdf). In short, if the code is not "exceptions ready", throwing an exception causes memory leaks, dangling pointers, and invariant violations all over the place, which will probably cause another crashes down the road (which will be especially hard to investigate and attribute to the original issue).
Fabric Core (Layout, Props parsing, ShadowNodes management, and so on) does not use exceptions because in most (all?) the cases the exception is now recoverable. So, if a program detects some internal state invariant violation or missing some resource, *logically* it's fatal. We also don't want to pay code-size and performance tax for exception support, so that's why we don't use them. It's just not the right fit for Fabric Core.

This does not mean that exceptions don't happen though. C++ standard library can throw them... sometimes. And if our library is compiled with exceptions enabled (still the case, unfortunately), an exception can bubble to JavaScript code and losing all context down the road. And it's hard to investigate such crashes. To isolate those occasional exceptions inside C++ core we are marking all C++/JS boundaries with `noexcept` that stops the bubbling.

I hope that will give us much more informative crash reports.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D23787492

fbshipit-source-id: 0822dbf36fc680c15b02b5cd0f2d87328296b642
2020-09-18 17:07:29 -07:00
Valentin Shergin 1ddc6542c7 Fabric: Removing catching all exceptions in UIManager::completeRoot
Summary:
This is a revert of D23529233 (https://github.com/facebook/react-native/commit/902611f14837752353e919dd1740812ec7260fb8).
It turns out it was a bad idea. With this catch-all thing, we don't get new information. Yeah, we crash earlier now but seems we have even less information about the crash. :(

I think D23754284 (https://github.com/facebook/react-native/commit/04c874bd9c6b15274fd87acf10cb3533b2eabc0d) should fix the issue.

Changelog: [Internal] Fabric-specific internal change.

Original commit changeset: 7ac7fb26ac08

Reviewed By: sammy-SC

Differential Revision: D23786086

fbshipit-source-id: 86784df0193abfb7328c4d5a16a9af4214e9a161
2020-09-18 17:07:29 -07:00
Joshua Gross ee38751975 Layout Events: throttle layout events sent to same node repeatedly
Summary:
Under Fabric only, we can enter an infinite layout loop where the emitted layout event oscillates between two height values that are off by a very small amount.

The cause is, in part, components that use layoutEvents to determine their dimensions: for example, using onLayout event "height" parameters to determine the height of a child. If the onLayout height changes rapidly, the child's height will change, causing another layout, ad infinitum.

This might seem like an extreme case but there are product use-cases where this is done in non-Fabric and layout stabilizes quickly. In Fabric, currently it may never stabilize.

Part of this is due to a longstanding issue that exists in Fabric and non-Fabric, that we cannot immediately fix: If in a single frame, C++ emits 100 layout events to ReactJS, ReactJS may only process 50 before committing the root. That will trigger more layout events, even though product code has only partially processed the layout events. At the next frame, the next 50 events will be processed which may again change the layout, emitting more events... etc. In most cases the tree will converge and layout values will stabilize, but in extreme cases in Fabric, it might not.

Part of this is because Fabric does not drop *stale* layout events. If 10 layout events are dispatched to the same node, it will process all 10 events in older. Non-Fabric does not have this behavior, so we're changing Fabric to drop stale events when they queue up.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D23719494

fbshipit-source-id: e44a3b3e40585b59680299db3a4efdc63cdf0de8
2020-09-17 13:20:23 -07:00
Valentin Shergin 902611f148 Fabric: Catching exceptions in UIManager::completeRoot earlier, in C++
Summary:
Fabric Core does not support exception safety, so technically exceptions are unrecoverable and binaries should be built without exception support.
While it's not the case, some exceptions might bubble to JavaScript realm and be caught as JavaScript exceptions. In this case, we lose the stack trace. To prevent that and to investigate one particular error we have we add this try-catch block in this diff.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: mdvacca

Differential Revision: D23529233

fbshipit-source-id: 7ac7fb26ac08ad26af8790172de471ac178c3a37
2020-09-04 22:53:40 -07:00
Lulu Wu bdbb040382 Fix Logbox Fullscreen View
Summary:
Logbox inside bridgeless surfaces was crashing and not able to open, this diff should fix it.

Changelog: [Internal]

Reviewed By: PeteTheHeat

Differential Revision: D23043773

fbshipit-source-id: 6e584f97e53e626b9bfedd6dc997ba6ba8c08b8f
2020-08-18 12:29:45 -07:00
Joshua Gross 77accf2380 Log more diagnostics when JSI values cannot be cast to Object
Summary:
There are a few places where we cast JSI values to objects without much validation and without proper error logging, and in some places the crashes aren't symbolicated well. To make debugging easier in the short-term, I'm adding some additional logs.

Changelog: [Internal]

Reviewed By: mdvacca, RSNara

Differential Revision: D23033222

fbshipit-source-id: 9343d693a441f0af728e560a0c245bcc4eb97869
2020-08-10 16:08:50 -07:00
Samuel Susla 154ce78972 Take viewport offset into account in UIManager.measureInWindow
Summary:
Changelog: [Internal]

Fabric's UIManager.measureInWindow didn't take viewport's offset into account. This diff fixes it by including viewport's offset in `LayoutContext`.

Reviewed By: JoshuaGross

Differential Revision: D23021903

fbshipit-source-id: 9106a8789d66fe19d8cb0a9378ee5bc8f2c83005
2020-08-10 12:52:23 -07:00
Samuel Susla 6c01f26d27 Fix typo in UIManagerBinding's destructor
Summary: Changelog: [internal]

Reviewed By: JoshuaGross

Differential Revision: D22897454

fbshipit-source-id: 73cc02b9645c04dd0e15e255d7277cd87d56fde3
2020-08-04 11:16:16 -07:00
Joshua Gross e3302eeeab LayoutAnimations: call onSuccess, onFailure callbacks
Summary:
Hook up onSuccess and onFailure callbacks to LayoutAnimations.

Note that in non-Fabric RN, onSuccess is not known to work in Android. I could not find any uses of onFailure and it's not documented, so for now, it is only called if the setup of the animation fails.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D22889352

fbshipit-source-id: 4306debb350388dd2b7a2cbfe295eb99723988e2
2020-08-02 16:37:03 -07:00
David Vacca 3093010ea5 move fabric to ReactCommon/react/renderer
Summary:
This diff moves fabric C++ code from ReactCommon/fabric to ReactCommon/react/renderer
As part of this diff I also refactored components, codegen and callsites on CatalystApp, FB4A and venice

Script: P137350694

changelog: [internal] internal refactor

Reviewed By: fkgozali

Differential Revision: D22852139

fbshipit-source-id: f85310ba858b6afd81abfd9cbe6d70b28eca7415
2020-07-31 13:34:29 -07:00