Summary:
This diff adds more enforcement for consistency of `ShadowNodeMutation`s, including:
* `Props` object for newly created or updated view must not be nullptr;
* `oldShadowView` must describe the previous state of the view for `Update` instruction;
* `ignoreDuplicateCreates` option was removed.
I suspect some of the crashes we see in Fabric are caused by a violation of one of these constraints. If one of these fails in debug builds, we will get an early signal.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D24880821
fbshipit-source-id: 8c8a3d8e205ce34f6e0335e8a2b0cf676930c284
Summary:
The `state_` member variable was getting destroyed before the `executor_` (because `state_` is declared after `executor_` https://stackoverflow.com/questions/2254263/order-of-member-constructor-and-destructor-calls). This lead to a race condition where items still pending on the `executor_` thread could try to run and access `state_` after it had already been reset.
Changelog:
[General][Fixed] - Fix race condition in Debug Inspector shutdown
Reviewed By: mhorowitz
Differential Revision: D24705062
fbshipit-source-id: e575d66322ab980b1a8c3e6083a0b3d40b9c944b
Summary:
This is the final diff in the JS TurboModule codegen stack for Android. It implements method dispatch using the TurboModuleSchema object.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D22837486
fbshipit-source-id: f91b03f064941457d4b8c5e37e011468559dee71
Summary:
JavaTurboModule will use instances of this class to perform method invocation. TurboModuleSchema is created by parsing the `jsi::Value` that represents the TurboModule's schema.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D22832729
fbshipit-source-id: 792736e176c33a302f6a41c6f63a4812b09af585
Summary:
## Android API
```
// Before we initialize TurboModuleManager
ReactFeatureFlags.useTurboModuleJSCodegen = true
```
## iOS API
```
// Before we initialize RCTBridge
RCTEnableTurboModuleJSCodegen(true);
```
## How is the JS Codegen actually enabled?
The above native flags are translated to the following global variable in JavaScript:
```
global.RN$JSTurboModuleCodegenEnabled = true;
```
Then, all our NativeModule specs are transpiled to contain this logic:
```
interface Foo extends TurboModule {
// ...
}
function __getModuleSchema() {
if (!global.RN$JSTurboModuleCodegenEnabled) {
return undefined;
}
// Return the schema of this spec.
return {...};
}
export default TurboModuleRegistry.get<Foo>('foo', __getModuleSchema());
```
Then, in our C++ JavaTurboModule, and ObjCTurboModule classes, we use the TurboModule JS codegen when the jsi::Object schema is provided from JavaScript in the TurboModuleRegistry.get call.
Changelog: [Internal]
Reviewed By: PeteTheHeat
Differential Revision: D24636307
fbshipit-source-id: 80dcd604cc1121b8a69df875bbfc87e9bb8e4814
Summary:
This change should make all type-safe NativeModules TurboModule-compatible.
Changelog: [Internal]
Differential Revision: D24729493
fbshipit-source-id: 7712708a24d675ca567225797016a7ff66a2920e
Summary:
This change introducing using `updateStateWithAutorepeat` for state updates in RCTSafeAreaViewComponentView. This way we can reduce the number of active commits and reduce jumps & relayout passes.
The approach with a callback is better than using `_lastPaddingStateWasUpdatedWith` because:
* When we compare the values, we can compare them with actual previous padding numbers stored in Shadow Tree.
* The value stored in a UIView instance can go away because of a view being remounted because of flattening.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D24719345
fbshipit-source-id: 9bf1ae284875b4c99cf23be2fcc9a829eb8a895e
Summary:
This implements a new ShadowNode trait that helps to propagate Yoga node `isDirty` flag down the root of the tree and clone siblings appropriately.
Several Fabric components mutate its Yoga styles after the node was cloned. In such cases, we need to mark the node as dirty after doing so. The problem with this is that the parent node and its siblings were already updated (cloned or not) based on the previous value of the `isDirty` flag. This happens because this logic is implemented in YogaLayoutableShadowNode which is a base constructor that must be called before any other logic from a subclass can run.
For now, this change enables that for SafeAreaView only (which seems to help with some junkiness issues), later we can extend the usage of this for other components if needed.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D24719347
fbshipit-source-id: b0d050afea5de9c470e05e1b4c9e7052e00ae949
Summary:
For some interrupted animations we will execute a "final" mutation associated with the animation, if it exists. For example, "UPDATE" animations always have a final Update animation associated with them.
Some, however, do not. For example: INSERT animations do not have a final mutation associated by default. In these cases (before this diff) if the animation from opacity 0 to 1 was interrupted, the View will
appear "stuck" at some intermediate opacity. To mitigate that, we generate a synthetic "final" mutation at 100% progress through the animation if it is interrupted.
Changelog: [Internal]
Reviewed By: fred2028
Differential Revision: D24691151
fbshipit-source-id: d9730b8a3493a5eeac4de325e7e0a7a64f73c8a0
Summary:
These are new markers that will be placed around initializing an RCTInstance.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24607905
fbshipit-source-id: 8e83a2476e2ae878c523217aeb5a3b4bfc5bf911
Summary:
When shouldEnableLoggingForRequestUrl is false, ImageTelemetry is not initialized, and no logging is done.
* Replace `- (NSString *)loaderModuleNameForRequestUrl:(NSURL *)url` with `- (BOOL)shouldEnableLoggingForRequestUrl:(NSURL *)url`
* Rename RCTImageLoaderInstrumentableProtocol.h -> RCTImageLoaderLoggableProtocol.h
Reviewed By: fkgozali
Differential Revision: D24523984
fbshipit-source-id: a5463eceea1c40f9452b0ad2ee6bf047f71a02c1
Summary:
The callback `dispatchEvent` is called asynchronously on the JavaScript thread, so all data it uses must be copied to the lambda. To work around this constraint we use a shared pointer for the counter.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24598840
fbshipit-source-id: fb5581858d54dc806863caf0c7c4f612ed6046e2
Summary:
Changelog: [internal]
Background executor performs unnecessary operations when second `completeRoot` message from React arrives before first `completeRoot` was finished. This produces unnecessary `ShadowViewMutations`.
Mechanism:
Everytime `completeRoot` is received, before the call is dispatched on the background queue, `completeRootEventCounter_DO_NOT_USE_` is incremented.
Inside `ShadowTree::tryCommit` we check if the value has been incremented to determine if another `completeRoot` is queued.
Reviewed By: JoshuaGross
Differential Revision: D24419160
fbshipit-source-id: 11e19026feca01db6c8981b093a691a6b58a006f
Summary:
Changelog: [internal]
There are two exceptions inside `TelemetryController::pullTransaction`:
- Empty Optional cannot be unwrapped
- mutex lock failed: Invalid argument
By marking this method `noexcept`, stack trace is lost and it makes it more difficult to track down the issue.
What does compiler do if a method is marked `noexcept`?
```
void f() noexcept {
try {
// do work
}
catch (...) {
std::terminate(); // This is the std::terminate() we are seeing in stack traces.
}
}
```
Removing noexcept specifier might give us more information about the exception.
Reviewed By: JoshuaGross
Differential Revision: D24477861
fbshipit-source-id: 80f26e9ab160a5330c2848b89a01d60bfc0a4611
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:
Changelog:
[General][Fixed] - Fix handling of very deeply nested data across the bridge
fixesfacebook/hermes#339
Reviewed By: sammy-SC
Differential Revision: D24182938
fbshipit-source-id: b674283a112b98cc63f20e436c538e3789ddf6dd
Summary:
In Chrome, there's a garbage can icon in the memory profiler page that you can
click to force a garbage collection.
Hermes was previously not responding to that button. Add support for this inspector
message type.
The collection it starts is fully synchronous and will block JS from running. It typically
completes quite fast for small heaps, and about 1 second for heaps over 100 MB big.
Changelog:
[Internal][Added] Add support for garbage collection during heap profiling with Hermes
Reviewed By: neildhar
Differential Revision: D24349262
fbshipit-source-id: fe62b8df4d2b67ab3930e5d57f94478b2a88a549
Summary:
#changelog: [internal]
When I built ThreadStorage I didn't know about existence of `thread_local` keyword. Because it achieves the same goal, using built in c++ features is preferred over building our own.
Reviewed By: JoshuaGross, shergin
Differential Revision: D24380680
fbshipit-source-id: e961fc34c6d3f085fc9b918b20bb4827de0d5624
Summary:
changelog: [internal]
Prevents 2 type converions:
1. int <-> size_t
2. int <-> int32_t
# Why is using size_t better when working with indexes.
## 1. Type conversion isn't for free.
Take this example
```
size_t calculate(int number) {
return number + 1;
}
```
It generates following assembly (generated with armv8-a clang 10.0.0):
```
calculate(int): // calculate(int)
sub sp, sp, #16 // =16
str w0, [sp, #12]
ldr w8, [sp, #12]
add w9, w8, #1 // =1
mov w8, w9
sxtw x0, w8
add sp, sp, #16 // =16
ret
```
That's 9 instructions.
If we get rid of type conversion:
```
size_t calculate(size_t number) {
return number + 1;
}
```
Assembly (generated with armv8-a clang 10.0.0):
```
calculate(unsigned long): // calculate(unsigned long)
sub sp, sp, #16 // =16
str x0, [sp, #8]
ldr x8, [sp, #8]
add x0, x8, #1 // =1
add sp, sp, #16 // =16
ret
```
Compiler now produces only 7 instructions.
## Semantics
When using int for indexing, the type doesn't say much. By using `size_t`, just by looking at the type, it gives the reader more information about where it is coming from.
Reviewed By: JoshuaGross
Differential Revision: D24332248
fbshipit-source-id: 87ef982829ec14906ed9e002ea2e875fda4a0cd8
Summary:
We will need it soon.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24290775
fbshipit-source-id: a312e537a3c3954e709a10c8792b3462b574054a
Summary:
Changelog: [internal]
Fabric uses view commands instead of setNativeProps. This diff removes what's left of setNativeProps from the core.
Reviewed By: JoshuaGross
Differential Revision: D24309999
fbshipit-source-id: 70e54f0a984f8c36f77ba2cd59f59fc6923bc832
Summary:
This diff ended up being a bit more complicated than I anticipated, since the source files in `ReactInternal` were depending on `RCTEventDispatcher`. I made the following changes:
1. Make `RCTEventDispatcher` a `protocol`, keep it in `ReactInternal`.
2. Rename the `RCTEventDispatcher` NativeModule to `RCTEventDispatcherModule`, make it conform to the `RCTEventEmitter` `protocol`, and move it to `CoreModules`.
3. Where necessary, replace categories of `RCTEventDispatcher` with functions.
Changelog:
[iOS][Added] - Make RCTEventDispatcher TurboModule-comaptible
Reviewed By: fkgozali
Differential Revision: D18439488
fbshipit-source-id: b3da15c29459fddf884519f33b0c3b8c036b5539
Summary:
There are a few places where we have JNI table ref leaks, and more places where we can aggressively clean up smart pointers immediately instead of waiting for them to be cleaned up at some later point.
In theory these smart pointers should be cleaned up immediately, but in cases where many components are being measured at once, the JNI table could grow until all measure calls are done. In extreme cases this
could cause a crash, which I want to avoid. At the very least, freeing memory more aggressively in this case can't hurt.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24293775
fbshipit-source-id: 159741ba955e5a6fe02caf6e65d1e4d6d4afadee
Summary:
This adds support for `<Picker style={}/>` prop for text styling. It reuses most of conversion logic from BaseText. This means that it actually supports more styles than Paper Picker supported. (Paper picker only supported ~4 styles, this supports everything that Text supports, so 10+ styles).
The only tricky thing is that Picker supports multiple ways of setting text color. Both
<Picker
itemStyle={{color: '#008BD0'}} >
<Picker.Item label="Java" value="java" />
<Picker.Item label="JavaScript" value="js" />
</Picker>
and
<Picker>
<Picker.Item label="Java" value="java" color={'#008BD0'} />
<Picker.Item label="JavaScript" value="js" />
</Picker>
technically work in Paper. I've decided to maintain this behaviour (since there's lots of product code callsites to both options).
Changelog: [iOS][Fabric] Fabric Picker support
Reviewed By: sammy-SC
Differential Revision: D23980319
fbshipit-source-id: e469a837e28af0ad97cf0e171df26ee19adff3ab
Summary:
This builds on previous diff to properly parse `<Picker.Item/>` into a cpp struct.
This fixes the dummy text and text color TODOs.
Changelog: [iOS][Fabric] Fabric Picker support
Reviewed By: sammy-SC
Differential Revision: D23964557
fbshipit-source-id: f42c6c9cf410bfc5e66ff078645b6378548481de
Summary:
This is a starting point for the handwritten Fabric Picker component. It is incomplete, and needs to be landed with the rest of the stack above it.
In general, this creates a new `ComponentView`, `ComponentDescriptor`, `ShadowNode`, `Props` and a few other boilerplate classes for Picker. A bunch of the logic in `ComponentView` was copied over from the Paper `RCTPicker` and `RCTPickerManager`.
What works in this diff:
- A `<Picker>` with items can be created in JS, and a corresponding `UIPicker` is created in native with placeholder text, default styling and the correct amount of items
What doesn't work yet (implemented in later diffs):
- Parsing items to use correct text and styling in native
- Events/commands
Reviewed By: sammy-SC
Differential Revision: D23941821
fbshipit-source-id: e049ca6004757fbd1361985644d5dbb8f53e1ce6
Summary:
When taking a heap timeline, Hermes wasn't showing any data until the timeline
was written to disk and then reloaded.
Turns out we were missing support for two events:
* `HeapProfiler.lastSeenObjectId`: This event reports the most recently
allocated object ID. Used to know when objects were allocated in the
timeline.
* `HeapProfiler.heapStatsUpdate`: Report how many objects and bytes
exist for a "time fragment", represented by a fragment index. Later updates
to the same index can decrease the amount of live memory, which show up
as grey spikes instead of blue spikes
Previously, we only supported these by writing out to a file, and they didn't work
with a "live" profiling view. To fix this, I changed the periodic sampling thread
to instead be a periodic flush of a sample every few allocations. The performance
impact is tucked away only when profiling is turned on, and it's very non-invasive to
the rest of the GC. The flush calls a callback with the relevant information if the
inspector is on, and the inspector sends a message back to the browser.
Changelog: [Internal] Fix for Hermes heap timeline profiling
Reviewed By: neildhar
Differential Revision: D23993363
fbshipit-source-id: 8e0b571130cbb7e839dfb009b04f584f5179085d
Summary:
## Previously
- When TurboModules system was on, we'd only log events from the TurboModules system.
- When TurboModules system was off, we'd only log events from the NativeModule system.
This ultimately gives us less data to analyze both systems in production.
## Changes in This Diff
When perf. logging is on, we'll log events from both systems. Each QPL event now include an annotation of which system the event is coming from. Concretely, this will allow us to see how much of the NativeModule system is being exercised in the TurboModule test group.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D24232594
fbshipit-source-id: 7dff57bd74fc7ef744d3e06ff174304f25790456
Summary:
Index adjustment is tricky. Seems more reliable to adjust each immediate mutation, and then immediately adjust delayed mutations based on it, rinse and repeat.
Previously it was possible to construct examples where the UI would get into a weird state because index adjustment caused items to be inserted in the wrong location.
Changelog: [Internal]
Reviewed By: kacieb
Differential Revision: D24232926
fbshipit-source-id: f8c445213528c2d2aedacf3e0c73c5bbeb62bc3d
Summary:
The captured `instance_` will result in a call to JVM code to release the reference when the lambda is destroyed. However, Hermes may destroy the lambda in a non-JVM background thread.
Since `instance_` is a member of the class, and `this` is already captured, there is no need to capture it.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D24141179
fbshipit-source-id: e3b268485ed229ee933281a80e5dbe2feba55719
Summary:
We're seeing that GCs caused by memory warnings on Android are barely collecting
any memory and are operating on much smaller heaps than natural GCs.
This is likely due to some of the TRIM_MEMORY_* events firing too often.
Log which event name it was, instead of the generic "memory warning", to narrow
down the cases where the memory warning was helpful. Unhelpful warnings will
later be moved to not force a GC.
Note that this diff changes Venice, but we don't see any Venice heaps in production,
so it won't matter much other than making the code match up.
Also note that iOS has a similar memory warning in `RCTCxxBridge.mm`, but it hasn't
been called in production. Perhaps iOS is less trigger-happy for memory warnings than
Android.
Changelog: [Internal]
Reviewed By: neildhar
Differential Revision: D24093394
fbshipit-source-id: 03304f0f79083133c4d9b730559aef291319b6eb
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:
This was causing a crash in babel:
```
$ babel src --out-dir bin --source-maps
Error: Cannot find module 'babel-plugin-idx' from
'~/fbsource/xplat/js/react-native-github/ReactCommon/hermes/inspector/tools/msggen'
- If you want to resolve "idx", use "module:idx" {
code: 'MODULE_NOT_FOUND'
}
```
It didn't appear that this module was used, so I deleted it.
Changelog: [Internal]
Reviewed By: neildhar
Differential Revision: D23993272
fbshipit-source-id: dd34f0fc652cb27c87c891ca37d0eba66a19a6cf
Summary:
Changelog: [internal]
Components can update state multiple times before the state update queue is flushed. This causes unnecessary layout/diff and mount passes. To solve this, drop stale state updates inside `stateUpdateQueue_ ` for specific `ShadowNodeFamily`.
Delivering stale status updates is redundant. Let's take SafeAreaView as an example. It schedules 5-6 state updates before `stateUpdateQueue_` is flushed. That's unnecessary work blocking JS thread. We only care about the latest state update. Same for TextInput and other components using state updates.
Reviewed By: JoshuaGross
Differential Revision: D23987707
fbshipit-source-id: 2e3f92cc93af61d78ac564aa40aef165af64b8c1
Summary:
This compiles SampleTurboModule into RNTester Android. It also adds the NativeModule playground to show case TurboModule system to RNTester examples, just like in iOS.
{F337854369}
Changelog: [Android][TurboModule] Added TurboModule example to RNTester when `USE_CODEGEN` is set
Reviewed By: hramos
Differential Revision: D24004711
fbshipit-source-id: b682dd51fa998ee2e60f8d6ffd8c39220d13a7fe
Summary:
This is the Java/JNI impl of the NativeSampleTurboModule.js, just like on iOS. The files here are supposed to be generated by the react-native-codegen, but they are checked in to the repo for easier build integration with RNTester.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D23985746
fbshipit-source-id: 46340d778f3d964efe5b538d15ebe0f2cab04862
Summary:
The sorting function currently forms a partial ordering, not a total ordering. This can cause problems with certain sequences of immediate or conflicting mutations, leading to UI corruption or crashes.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24002668
fbshipit-source-id: edc9b4c1e3104897cb0c5fd6da563ec43d800494
Summary:
Remove the older implementation of image instrumentation in Fabric by removing, RCTImageInstrumentationProxy, ImageInstrumentation from ImageRequest, and trackURLImageContentDidSetForRequest from RCTImageLoaderWithAttributionProtocol.
Changelog: [RN][Fabric][Image] Remove unused Fabric image instrumentation
Reviewed By: fkgozali
Differential Revision: D23990606
fbshipit-source-id: 004d04025d031af11377a73e5bfb64b1e0449962
Summary:
Refs: [0.62 release](https://reactnative.dev/blog/#moving-apple-tv-to-react-native-tvos), https://github.com/facebook/react-native/issues/28706, https://github.com/facebook/react-native/issues/28743, https://github.com/facebook/react-native/issues/29018
This PR removes most of the tvOS remnants in the code. Most of the changes are related to the tvOS platform removal from `.podspec` files, tvOS specific conditionals removal (Obj-C + JS) or tvOS CI/testing pipeline related code.
In addition to the changes listed above I have removed the deprecated `Platform.isTVOS` method. I'm not sure how `Platform.isTV` method is correlated with Android TV devices support which is technically not deprecated in the core so I left this method untouched for now.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
* **[Internal] [Removed]** - remove most of tvOS remnants from the code:
* `TVEventHandler`, `TVTouchable`, `RCTTVView`, `RCTTVRemoteHandler` and `RCTTVNavigationEventEmitter`
* **[Internal] [Removed]** - remove `TARGET_TV_OS` flag and all the usages
* **[iOS] [Removed]** - remove deprecated `Platform.isTVOS` method
* **[iOS] [Removed]** - remove deprecated and TV related props from View:
* `isTVSelectable`, `hasTVPreferredFocus` and `tvParallaxProperties`
* **[iOS] [Removed]** - remove `BackHandler` utility implementation
Pull Request resolved: https://github.com/facebook/react-native/pull/29407
Test Plan: Local tests (and iOS CI run) do not yield any errors, but I'm not sure how the CI pipeline would react to those changes. That is the reason why this PR is being posted as Draft. Some tweaks and code adjustment could be required.
Reviewed By: PeteTheHeat
Differential Revision: D22619441
Pulled By: shergin
fbshipit-source-id: 9aaf3840c5e8bd469c2cfcfa7c5b441ef71b30b6
Summary:
Without this thing some stuff does not compile.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: fkgozali
Differential Revision: D23948622
fbshipit-source-id: f066ada183c0fd6a7b5eec542395d44bbbfe80a3
Summary:
This fixes a recently introduced crash in `colorComponentsFromColor()` (iOS implementation) caused by dereferencing a null pointer.
The fix is just a copy of a code fragment from a previous implementation.
Reviewed By: fkgozali
Differential Revision: D23944812
fbshipit-source-id: 977135dd75c4375affddfd75183e4890618ae819
Summary:
When an animation is completed or a conflicting animation is detected, force props, state, layout, etc to update.
Currently, when a final animation mutation is queued, some attributes can be updated but not others, causing unexpected visual glitches at least on Android.
Some of these are arguably component bugs, but it's easier to just flush all attributes by tricking the platforms into updating all attributes. This will also prevent us from having to track down more of these bugs, potentially.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D23886519
fbshipit-source-id: 8e5081bbe3b7843c16c0f283fa07fdec0e211aa8
Summary:
Bitcode is turned on by default in React Native and so, setting it here as well.
Changelog: [iOS] [Changed] - Upgraded JSI with a new HERMES_ENABLE_BITCODE flag
Pull Request resolved: https://github.com/facebook/hermes/pull/365
Reviewed By: tmikov
Differential Revision: D23823228
Pulled By: Huxpro
fbshipit-source-id: d43638818a733f6a87b2f4a1ecadad8ea9c7a419