Summary:
This enables a new state auto repeating mechanism built-in mechanism for all state updates which we already use for CK interop. This experiment is supposed to help with T74769670 and co.
This change is gated with MC.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D23762508
fbshipit-source-id: f535513c724ace9ede570177281324eb507329c5
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
Summary:
LayoutAnimations: at the end of every animation, issue an update mutation - this is so that the props data on the Mounting layer/StubViewTree layer is pointer-identical to the props data on the ShadowTree.
This unblocks iOS debug mode crashes.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D23753606
fbshipit-source-id: 407e0c2746a65e6d3ee29c1cce891cd7c1013593
Summary:
iOS needs this function to be marked as static.
Changelog: [internal]
Reviewed By: shergin
Differential Revision: D23749613
fbshipit-source-id: a8c160646853450fc7d849448bdbb45e02beb964
Summary:
Currently, MountingCoordinator's RN_SHADOW_TREE_INTROSPECTION code will crash often because it assumes there is always a "new" tree to compare the old tree to. In the LayoutAnimations context this is not always the case - in fact, the majority of the time, LayoutAnimations is producing mutations for animation without a "new" tree.
Just check that the tree exists before trying to print it.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23747289
fbshipit-source-id: a1ba22aeae32ed8915a53bc33cdc199e8ce5128a
Summary:
In MountingCoordinator override mode (used in LayoutAnimations) we must set the start and end `diff` time when no real diff happens, otherwise we will hit an assert in telemetry later.
I also ensure that the TransactionNumber is incremented in that case.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23746684
fbshipit-source-id: b1fe3864e453fdba89d43cc827bd37434abf7a4d
Summary:
`adjustDelayedMutationIndicesForMutation` asserts that the mutation is either Remove or Insert. At one callsite, we weren't checking the mutation type before calling `adjustDelayedMutationIndicesForMutation`.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23746617
fbshipit-source-id: 6046fec2eb4821094937b1b16f40347bbc55c20e
Summary:
This change maps the three most used colors (black, white, clear) to corresponding predefined values in UIColor. This should meaningfully reduce the overall amount of allocated UIColor/CGColor objects. In my non-scientific measures, it reduces the number of CGColor objects from ~1500 to ~1000. And... it no much at least in terms of kilobytes. However, I still think it's a good idea to implement this because I hope that can remove some work from memory allocation infra and maybe enable some optimizations that UIKit hopefully does for black and white colors. (I tend to believe that this optimization exists because UIKit even has a classes called UIDeviceWhiteColor and UICachedDeviceWhiteColor.)
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D23753506
fbshipit-source-id: 46e58dc7e6b0dcab3c83d29c7257c90ffbd95246
Summary:
Finally, this diff changes the internal implementation of SharedColor to be `optional<int>`.
Initially, when we started working on the new renderer, it seemed like a good idea to allocated CGColor objects ahead of time and store them with Props. Now, this idea does not look so good, especially because:
* Having `SharedColor` as a `shared_ptr` is a quite big perf overhead for copying this thing. And the size of the object is not small.
* Having `SharedColor` as a `shared_ptr` creates huge interconnectedness among pieces of the core and rendering. E.g. improper releasing a pointer in some component view can cause a crash somewhere in the core (because both places might use the same shared `blackColor`.
On Android, we already use simple `int` as a representation of a color, and this works great. And this diff implements something very similar to Android, but a bit differently: here we use `optional<int>` instead of custom class with a single `int` field and some magic value that represents "empty value".
This approach should fix T75836417 because now we don't have allocation and deallocation when we simply assign color values.
If this solution works fine on iOS, I will do unify all implementations among all platforms.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D23753507
fbshipit-source-id: 42fd6cee6bf7b39c92c88536da06ba9e8cf4d4db
Summary:
This diff changes the implementation of `RCTCreateCGColorRefFromSharedColor` and `RCTUIColorFromSharedColor` in such a way that they don't rely on the fact that SharedColor is actually a `shared_ptr<CGColorRef>`. Instead, the methods just extract color components from SharedColor and create UIColor and CGColorRef objects on demand.
This allows us to change the implementation of SharedColor without worrying much about the rest of the system, which will do in the next diff.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D23753510
fbshipit-source-id: 340127527888776ebd5d241ed60c7e5220564013
Summary:
We don't need a shared_ptr here and without it the code will be faster and simpler.
This change is aligned with any clone-line callbacks we have in the Core which accepts a `const &` and return `shared_ptr<>`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, sammy-SC
Differential Revision: D23725687
fbshipit-source-id: 1cd959f4273913175d342302e2f12752f0114768
Summary:
Previously, to get a current root shadow node for a shadow tree, we called `tryCommit` method and stole a pointer from this. That was not a very straightforward method to get things done, and most importantly we need to do this to change the shape of the ShadowTreeCommitTransaction signature (remove a shared pointer from the callback) to make it simpler, faster and allow future improvements (see the next diff).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, sammy-SC
Differential Revision: D23725689
fbshipit-source-id: 51950b843a0e401828b6c6a38e5e2aaaf21ec166
Summary:
Instead of storing tree separate instance variables, we now store a single object that contains them. We will need it to be able to postpone the mounting stage (we save all info we need for mounting inside the new instance variable).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23725690
fbshipit-source-id: 09dd4a0912c6f5b34d805636b62f73ca12287329
Summary:
The implementation of this class is too complex for the purpose it serves. Making it simpler will make the code simpler and faster.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23725688
fbshipit-source-id: 5e1ecddb0dd3c4c4f94786e2ba0af9b67e7426ce
Summary:
Right now nested Text components are not accessible on Android. This is because we only create a native ReactTextView for the parent component; the styling and touch handling for the child component are handled using spans. In order for TalkBack to announce the link, we need to linkify the text using a ClickableSpan.
This diff adds ReactClickableSpan, which TextLayoutManager uses to linkify a span of text when its corresponding React component has `accessibilityRole="link"`. For example:
<Text>
A paragraph with some
<Text accessible={true} accessibilityRole="link" onPress={onPress} onClick={onClick}>links</Text>
surrounded by other text.
</Text>
With this diff, the child Text component will be announced by TalkBack ('links available') and exposed as an option in the context menu. Clicking on the link in the context menu fires the Text component's onClick, which we're explicitly forwarding to onPress in Text.js (for now - ideally this would probably use a separate event, but that would involve wiring it up in the renderer as well).
ReactClickableSpan also applies text color from React if it exists; this is to override the default Android link styling (teal + underline).
Changelog: [Android][Fixed] Make nested Text components accessible as links
Reviewed By: yungsters, mdvacca
Differential Revision: D23553222
fbshipit-source-id: a962b2833d73ec81047e86cfb41846513c486d87
Summary:
It's more correct and we will rely on this in the future.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D23681508
fbshipit-source-id: 5fef0ea164e8a400e6ca9a67947252b47ce6d44e
Summary:
This fixes TextInput measurement caching. Previously we were not setting the correct Spannable in the cache; we needed to additional Spans to it to indicate font size and other attributes.
This brings Fabric closer to how non-Fabric was measuring Spannables for TextInputs (see ReactTextInputShadowNode.java).
This should fix a few crashes and will be most noticeable with dynamically-sized multiline textinputs where the number of lines changes over time.
This also allows us to transmit less data from C++ to Java in the majority of cases.
Changelog: [Internal]
Differential Revision: D23670779
fbshipit-source-id: cf9b8c848b9e0c2619e01766b72b074248466825
Summary:
Every time we measure a TextInput we allocate a JNI local array and weren't cleaning it up, leading to JNI table exhaustion.
Changelog: [Internal]
Differential Revision: D23670780
fbshipit-source-id: 2ecf9770c8593eeadd70a248be58037fefdca61e
Summary:
Changelog: [Internal]
There was a key mismatch between what Java and C++.
cacheIdMap was incorrectly initialised.
Differential Revision: D23652342
fbshipit-source-id: 66f54dc887a011afeead9420bda093e9a0c9a8ca
Summary:
One of the operations we do in `Scheduler::stopSurface()` is committing an empty tree to free up `ShadowNode` objects and "disable" `EventEmitter`s associated with them. Before this change, we had a gap in time between a moment when we commit an empty tree and remove the tree from the registry. During this time gap, JavaScript (or native, actually) can commit another tree and mount another new state on the screen. To prevent this, we remove the tree from the registry first and only then commit an empty tree to a uniquely owned tree.
Note that the deleted comment says that we actually have to have a tree in the registry for committing an empty tree, I don't think it's true now.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23667882
fbshipit-source-id: 387052e9f3e78e7d4446f36baed50f9caa831133
Summary:
Changelog: [Internal] Log image-requested QPL point for Fabric images
* See D23450649 for the full lifecycle
Reviewed By: fkgozali
Differential Revision: D23448179
fbshipit-source-id: 0a78dae2d4f1e6322bbeee3574b10abe1efb30ef
Summary: Changelog: [Internal] Add IGviewpoint to get image visibility callbacks for when an UIImageView is in or out of view
Reviewed By: fkgozali
Differential Revision: D23428528
fbshipit-source-id: 87e4cee8fbe3c6b7da5153f87bbb530b2f990d96
Summary:
* Create `ImageTelemetry` as a property of ImageRequest to pass image metadata from the ImageShadowNode into RCTImageComponentView
* ImageRequest is a object in the ImageShadowNode::ConcreteState
Reviewed By: fkgozali
Differential Revision: D23239664
fbshipit-source-id: db0f08bb889af8ce63fbe1abe88744edddb86150
Summary:
This function replaces the usage of an Objective-C `#import` statement with `#include` in `react/renderer/scheduler/SynchronousEventBeat.cpp`.
Clang seems to have no problems with `#import` being used in CPP files, but it was causing errors in MSVC. ([it has a different purpose](https://docs.microsoft.com/en-us/cpp/preprocessor/hash-import-directive-cpp?view=vs-2019) on Windows)
## Changelog
Changelog: [Internal][Changed] - Fabric: Replace #import statement in SynchronousEventBeat
Pull Request resolved: https://github.com/facebook/react-native/pull/29885
Test Plan: This CPP file now compiles in MSVC
Reviewed By: sammy-SC
Differential Revision: D23591933
Pulled By: shergin
fbshipit-source-id: ee2cb8152f37e5ee09500af621a57b59d5a18400
Summary:
Changelog: [internal]
# Problem
onTextLayout was called even if there was no change to text layout. This can cause an infinite loop with onTextLayout triggering commit which triggers onTextLayout.
# Fix
Do not call onTextLayout if there is nothing to layout.
Reviewed By: yungsters
Differential Revision: D23648430
fbshipit-source-id: 055dc34a9aca0edf2c78a5812b35b80df32c9e3e
Summary:
This is *another* attempt to solve a failed state update problem.
Unfortunately, some applications are inherently not compatible with the "let's recommit the update on the application side in case it failed" approach. The problem is that if we call `updateState` on the application side, we miss the original event window. E.g. if we need to deliver some state update with AsyncBatched priority and if the update fails, we lose the opportunity to commit it on time. These issues can be critical for some complex use-cases as ComponentKit interop.
This diff adds implementation for `updateState` that does the work a bit differently. For all failed state updates it tres to recommit them asap using `ShadowTree::commit` and calling lambda on every attempt. With this approach the update might fail in two cases:
The node disappeared from the tree, so there is no way to update it.
The lambda returned `nullptr` indicating that the update is no longer needed.
We need this for the ComponentKit interoperability layer that is very sensitive for missing state updates.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23603958
fbshipit-source-id: a3b8c09fb2f1c8302583aa5880b48fc0840224e3
Summary:
We need it to stop repeating to commit new shadow tree in `ShadowTree::commit` when a transaction cancels the commit.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, sammy-SC
Differential Revision: D23603959
fbshipit-source-id: d279fb3bf4190e860740a6450595d6f2fc3117f7
Summary:
This pull request adds a call to `std::move` on the lambda capture in `Element::stateData`.
On Windows/Visual Studio 2017, this fixes a failure in the test `LayoutableShadowNodeTest.contentOriginOffset` where the error `std::bad_function_call` was being thrown. This was narrowed down to the callback being empty when called in `Element::stateData`.
https://github.com/facebook/react-native/blob/7e899348c74238a4a042380f86a8fe0d7e05511b/ReactCommon/react/renderer/element/Element.h#L98
Making sure the callback survives with `std::move` allows that test to pass under Windows.
## Changelog
Changelog: [Internal][Changed] - Fabric: Use std::move on callback in Element::stateData
Pull Request resolved: https://github.com/facebook/react-native/pull/29897
Test Plan: The Fabric test suite passes on Windows after this change is made. I also tested it under macOS and Linux built with Clang and they both pass with this change made.
Reviewed By: sammy-SC
Differential Revision: D23591969
Pulled By: shergin
fbshipit-source-id: e5c88bb0e94641e5128c4d49dd2f9dbfa49e9cfa
Summary:
In `StateReconciliationTest`, the way initializer lists are used to create null `ShadowNode`s causes this error on Visual Studio 2017 on Windows:
```cpp
auto result = (ShadowNode const *){nullptr};
```
---
```
StateReconciliationTest.cpp(35): error C4576: a parenthesized type followed by an init
ializer list is a non-standard explicit type conversion syntax
```
This change allows this test to compile in Visual Studio 2017, and the effected tests successfully compile and pass on Windows. They also compile and pass on Linux and macOS (both built with Clang)
## Changelog
Changelog: [Internal][Changed] - Fabric Tests: Change null ShadowNode creation in StateReconciliationTest
Pull Request resolved: https://github.com/facebook/react-native/pull/29899
Test Plan: The Fabric test suite passes on Windows after this change is made. I also tested it under macOS and Linux built with Clang and they both pass with this change made.
Reviewed By: sammy-SC
Differential Revision: D23592007
Pulled By: shergin
fbshipit-source-id: 7c6131736d478a0bf29d6c9475ef9149b7602dd6
Summary:
This pull request changes the default constructor in the `TestProps` class in `react/renderer/core/tests/TestComponent.h`
`using ViewProps::ViewProps;` was causing problems in Visual Studio 2017 - changing it to ` TestProps() = default;` allowed dependent CPP files to compile on MSVC and caused no regressions in the Fabric test suite.
## Changelog
Changelog: [Internal] [Changed] - Fabric Tests: Change default constructor in TestComponents' TestProps
Pull Request resolved: https://github.com/facebook/react-native/pull/29898
Test Plan: The Fabric test suite passes on Windows after this change is made. I also tested it under macOS and Linux built with Clang and they both pass with this change made.
Reviewed By: sammy-SC
Differential Revision: D23591986
Pulled By: shergin
fbshipit-source-id: 132e1c2e38fa74aa4f2c8746054d6152f30035e9
Summary:
This pull request adds an include statement including the header `<algorithm>` to `react/renderer/graphics/Rect.h`.
It is needed for building with MSVC because of the use of the `std::max` function. It seems that this header is included as a side effect of some other header that's being used when building with Clang, and for some reason it is not included when being built with MSVC.
## Changelog
Changelog: [Internal][Added] - Fabric: Explicitly include algorithm in Rect.h
Pull Request resolved: https://github.com/facebook/react-native/pull/29884
Test Plan: Files that include this header now successfully compile on MSVC.
Reviewed By: sammy-SC
Differential Revision: D23591910
Pulled By: shergin
fbshipit-source-id: d8367bbdd94bc66c05d8fa308ed46c7aca24a68a
Summary:
The standard says that zIndex should only be defined for non-`static` positioned views. This diff implements it.
For now, it actually enables zIndex for all views in RN because there is no way to specify `position: static` but we will give that ability by changing Flow definitions in future diffs in a couple of weeks (to ensure OTA safety).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D23559447
fbshipit-source-id: 20ea10c9349de2c5b1adea5735324a8f57150695
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
Summary:
Shadow tree introspection was disabled for a while, now we need it back working. This diff also restructures the logic of `MountingCoordinator::pullTransaction()` splitting it into two sections, first one for the base case and the second for the overriding case.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23374948
fbshipit-source-id: 0b5f1c598975bceb3dcb6a0eaee67ff58ef9dda1
Summary:
Just renaming, nothing more.
The idea of MountingTelemetry already grown to something bigger than just mounting telemetry, so we are renaming it.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23374947
fbshipit-source-id: f60ce38b75d1ce77498b84688e59598314c69a78
Summary:
This diff fixes the initial render of RN Android TextInput. The problem was that "attributedString" was not serialized until an event was sent from native side.
Changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D23383969
fbshipit-source-id: 86601434b1fbaa9f712bdb79b013a1d004bc55a4
Summary:
My goal in this diff is to make LayoutAnimations more stable, and more resilient to challenging situations. Namely, LayoutAnimations already works fine with:
1) Reorders
2) Flattening/unflattening
3) Deletion and recreation of the same hierarchy
4) Updates conflicting with an existing animation
However, what if /all/ of those things are combined? Handling update conflicts with multiple ongoing animations, repeatedly flattening/unflattening the same layer of hierarchy and reordering both parents and children, etc .
This diff does not make LayoutAnimations perfect, but it does make LayoutAnimations much more resilient to situations it was not able to handle before.
My primary method of testing was to use two Playground examples: one just repeatedly queues up mutations (some animated, some not) that create, update, and delete in the same hierarchy layer. The second, more complex one, mutates between random view hierarchies that involve a lot of flattening and unflattening as well as reordering, over a depth of 5. It also exercises animations over TextInlineViews, which is more challenging.
LayoutAnimations works best with the new "Flattening Differ" for now, because the Flattening Differ produces a much smaller, nearly minimal set of instructions in cases of flattening-unflattening. I would like that to not be a hard requirement for using LayoutAnimations, but it's a good starting-point for now.
As part of this work, I also developed a lot of debugging and logging mechanisms that are handy for detecting inconsistencies and debugging crashes. Some are included in this diff behind `#define` statements that are disabled by default, and the rest will be published separately and likely cannot be landed permanently, as they're more invasive changes that are only helpful in debugging.
# Followups:
- Automate testing: write a suite of C++ tests that mutates between random diffs and guarantees that all mutations in a StubViewTree are sensible
- Construct a set of minimal repros that catalogues all remaining crashes and inconsistency issues (these seem to be extremely marginal cases and are very hard to repro - so I think it's fine to run this in prod for now, but I will follow-up as soon as I'm back to catalogue and fix all remaining issues)
- This diff focuses on *not crashing*, but it is still possible to construct a sequence of complex mutations that results in (for example) views having some opacity between 0 and 1 if animations are interrupted repeatedly. Although this is easy enough to prevent in product code - the types of scenarios I'm running in tests are very unlikely to ever happen in production - it would be nice to be *sure* that LayoutAnimations will always converge to a sensible View hierarchy with up-to-date props.
- In general, the index adjustment logic is complicated. I don't know if there's a great way around it, so I need to at least catalogue and test all edge-cases as mentioned above.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23382975
fbshipit-source-id: f379d9aa2a4b9c33fa2ba8fa07870c9e31fad5e7
Summary:
Now we store a revision number of a Shadow Tree that leads to a transaction for which the concrete instance of MountingTelemetry corresponds. This is useful to understand how many actual transactions were skipped during a mounting phase (a mounting transaction does not directly correspond to a commit operation).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23364663
fbshipit-source-id: 32b86bcdfc1ae97d8fff3b97a8615cc5a5b4d4a9
Summary:
With this change, we now collect the number of text measurements that we perform during the layout phase of the commit. Text measurements are the most expensive layout operations which pretty much responsible for the vast majority of time spent in the layout phase.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23364664
fbshipit-source-id: 19514b93166b4053c2f3be37e79507f2c5248000
Summary:
Microsoft’s RN for macOS fork supports the Hermes engine nowadays https://github.com/microsoft/react-native-macos/pull/473. As a longer term work item, we’ve started moving bits that are not invasive for iOS but _are_ a maintenance burden on us—mostly when merging—upstream. Seeing as this one is a recent addition, it seemed like a good candidate to start with.
As to the actual changes, these include:
* Sharing Android’s Hermes executor with the objc side of the codebase.
* Adding a CocoaPods subspec to build the Hermes inspector source and its dependencies (`Folly/Futures`, `libevent`).
* Adding the bits to the Xcode build phase script that creates the JS bundle for release builds to compile Hermes bytecode and source-maps…
* …coincidentally it turns out that the Xcode build phase script did _not_ by default output source-maps for iOS, which is now fixed too.
All of the Hermes bits are automatically enabled, on macOS, when providing the `hermes-engine-darwin` [npm package](https://www.npmjs.com/package/hermes-engine-darwin) and enabling the Hermes pods.
## Changelog
[General] [Added] - Upstream RN macOS Hermes integration bits
Pull Request resolved: https://github.com/facebook/react-native/pull/29748
Test Plan:
Building RNTester for iOS and Android still works as before.
To test the actual changes themselves, you’ll have to use the macOS target in RNTester in the macOS fork, or create a new application from `master`:
<img width="812" alt="Screenshot 2020-08-18 at 16 55 06" src="https://user-images.githubusercontent.com/2320/90547606-160f6480-e18c-11ea-9a98-edbbaa755800.png">
Reviewed By: TheSavior
Differential Revision: D23304618
Pulled By: fkgozali
fbshipit-source-id: 4ef0e0f60d909f3c59f9cfc87c667189df656a3b