Commit Graph

2776 Commits

Author SHA1 Message Date
Héctor Ramos 17862a78db Add Appearance module
Summary:
Android implementation of the Appearance native module. Exposes the user's preferred color scheme: "dark" for Night theme ON, "light" for Night theme OFF.

Emits a `appearanceChanged` event when the current uiMode configuration changes.

To make your app handle Night mode changes, make sure to do the following:

* Declare your Activity can handle uiMode configuration changes (https://developer.android.com/preview/features/darktheme#java):
```
android:configChanges="uiMode"
```
* Make sure to pass the configuration changed activity lifecycle callback from your ReactActivity:
```
Override
protected void onConfigurationChanged(Configuration newConfig) {
    super.onConfigurationChanged();

    if (mReactInstanceManager != null) {
        mReactInstanceManager.onConfigurationChanged(newConfig);
    }
}
```

### RNTester

Adds the AppearanceExample to RNTester on Android.

Changelog:

[Android][Added] - New Appearance module exposes the user's current Night theme preference

Reviewed By: makovkastar

Differential Revision: D16942161

fbshipit-source-id: d24a8ff800a1c5f70f4efdec6891396c2078067e
2019-08-31 11:22:44 -07:00
Joshua Gross 5abe5843e2 Add C++ AndroidTextInput component for backwards-compatible Fabric support of TextInput on Android
Summary: Support existing, backwards-compatible AndroidTextInput component for minimal support of TextInput on Android.

Reviewed By: shergin, mdvacca

Differential Revision: D17086758

fbshipit-source-id: 25726f22229e0d5dfe96eb36b386a5317601283d
2019-08-30 19:04:14 -07:00
Joshua Gross 898124541c Support sendAccessibilityEvent in Fabric
Summary: Support for `sendAccessibilityEvent` in the FabricUIManager.

Reviewed By: shergin

Differential Revision: D17142507

fbshipit-source-id: 5c131d7caa1e4189fd41ecfb558d0027394b6a15
2019-08-30 19:04:14 -07:00
Valentin Shergin c80192c2ab Fabric: EventBeat::Owner to help with crashes
Summary:
The purpose of `EventBeat` is handling an asynchronous callback to itself which is being delivered on some different thread. That brings a challenge of ensuring that the `EventBeat` object stays valid during the timeframe of callback execution. The concept of Owner helps with that.
The owner is a shared pointer that retains (probably indirectly) the `EventBeat` object. To ensure the correctness of the call, `EventBeat` retains the owner (practically creating a retain cycle) during executing the callback. In case if the pointer to the owner already null, `EventBeat` skips executing the callback.
It's impossible to retain itself directly or refer to the shared pointer to itself from a constructor. `OwnerBox` is designed to work around this issue; it allows to store the pointer later, right after the creation of some other object that owns an `EventBeat`.

Reviewed By: JoshuaGross

Differential Revision: D17128549

fbshipit-source-id: 7ed34fd865430975157fd362f51c4a3d64214430
2019-08-30 18:24:27 -07:00
Oleksandr Melnykov 26a8d2e03a Remove RCT prefix from names of generated Java classes
Summary: This diff removes the 'RCT' prefix (if it's present) from the names of the generated Java classes. The motivation is that we don't want to have any Java files having this prefix in the RN Android codebase.

Reviewed By: JoshuaGross

Differential Revision: D17123804

fbshipit-source-id: 31905d3141e0f58ea47cdbdb0cf77d2d105de9a9
2019-08-30 10:03:27 -07:00
Oleksandr Melnykov 3d3db71763 Generate OSS view manager interfaces and delegates
Summary: This diff checks in the OSS view manager interfaces and delegates generated by the JS codegen. This is a temporary workaround since we don't have the JS codegen running in open source.

Reviewed By: fkgozali

Differential Revision: D17104816

fbshipit-source-id: 848afc081785c9a78891d3dc0740ebe858eb8891
2019-08-29 16:08:48 -07:00
Spencer Ahrens ed9ecb5d73 improve UIManager QPL markers
Summary:
The commit and batch points are just showing up as points in scuba relativity which isn't very useful - we want to know how long they take as well, so make them ranges.

Also adds view creation and update counts as annotations which can be handy for debugging regressions.

Reviewed By: JoshuaGross, axe-fb

Differential Revision: D17083058

fbshipit-source-id: dd29d12731734252930c9a4424bddbb98a8acccf
2019-08-28 08:48:48 -07:00
Joshua Gross 44bfc4be44 Support for UpdatePaddingMountItem
Summary: Some views (TextInput!) need padding props.

Reviewed By: mdvacca

Differential Revision: D17081799

fbshipit-source-id: 4f5d6a139bb4dd878f90af0ed4a30fe3810e3429
2019-08-27 19:00:11 -07:00
André Krüger 51aacd5241 Implement fading edges for ScrollView and it's dependent FlatList (#26163)
Summary:
This should add props for enabling horizontal and vertical fading
edges for Scrollview and FlatList.
These fading edges are used to communicate to the user that there is more content to see.

## Changelog

[Android] [Added] - fading edges props to the FlatList and ScrollView components
Pull Request resolved: https://github.com/facebook/react-native/pull/26163

Test Plan:
Open the React Native test app and navigate to the FlatList section.
Enable the `useFadingEdges` switch and insert a number into `Fading edge length`.

![device-2019-08-23-123745](https://user-images.githubusercontent.com/222393/63587150-7385cb00-c5a3-11e9-98dc-bffe8276d30c.png)
![device-2019-08-23-123844](https://user-images.githubusercontent.com/222393/63587156-75e82500-c5a3-11e9-9e9f-66876ac8f506.png)

Differential Revision: D17080676

Pulled By: TheSavior

fbshipit-source-id: 91df629c17052d43c99145672e9084e1379a4113
2019-08-27 18:37:04 -07:00
Peter Laraia db5994980d Android Q related NaN error fix - don't try to do math with NaN values
Summary:
We noticed a repro-able crash in Ride in T52804960 on Android Q due to NaN being passed into setCameraDistance
 on View

see Oleg's related post: https://fb.workplace.com/groups/rn.support/permalink/2682537011794897/

It looks like a generic fix or wrapper around View setCameraDistance might be planned in T48580247

But in the meantime, it kind of maybe seems reasonable-ish to say, ~~if the value of an input node is NaN, don't use it in the math for this node?~~ if a one of the inputs for this node evaluates to NaN, update that input node first? But I'm not super familiar with the Animations library so maybe that's not a good idea, idk.

From what I can tell in our specific error, it's coming from an InterpolatedNode A based off an AdditionNode B which tried to add a ValueNode C + a InterpolatedNode D, but D had only just been created and not had it's first update, so it's value was NaN, and so when B runs it's update value of C + NaN means B's new values is also NaN, and A's subsequent update based on that now comes out to NaN. Atleast that's what it seems like based on Log statements.

Reviewed By: olegbl

Differential Revision: D16960177

fbshipit-source-id: 99c8ca35be4b5e99f7c21db6733ebd622ae39d07
2019-08-27 00:24:45 -07:00
Spencer Ahrens 8e04a1485f Back out "[react-native][PR] [Android] Implement fading edges for ScrollView and it's dependent FlatList"
Summary:
After some thought, we decided we don't need the flexibility of
separate horizontal and vertical props - it would be much nicer
to just have a single prop for the edge length and then the native
code can enable the booleans as appropriate.

Original PR: https://github.com/facebook/react-native/pull/26163

Original commit changeset: f72a9a890d90

Reviewed By: TheSavior

Differential Revision: D16997468

fbshipit-source-id: 7973262287a7ec2cee5957f8dc1806a0f28c1432
2019-08-23 17:11:17 -07:00
Anda Bereczky f8dff0bcb3 Default accessibility hint is read for buttons
Summary: [Android] [Fixed] - Make sure the default accessibility hint is read for buttons on Android.

Reviewed By: PeteTheHeat

Differential Revision: D16974987

fbshipit-source-id: e7b932041f82d41271d9393e651967789c05f38a
2019-08-23 15:20:40 -07:00
André Krüger f8a64f9d61 Implement fading edges for ScrollView and it's dependent FlatList (#26163)
Summary:
This should add props for enabling horizontal and vertical fading
edges for Scrollview and FlatList.
These fading edges are used to communicate to the user that there is more content to see.

## Changelog

[Android] [Added] - fading edges props to the FlatList and ScrollView components
Pull Request resolved: https://github.com/facebook/react-native/pull/26163

Test Plan:
Open the React Native test app and navigate to the FlatList section.
Enable the `useFadingEdges` switch and insert a number into `Fading edge length`.

![device-2019-08-23-123745](https://user-images.githubusercontent.com/222393/63587150-7385cb00-c5a3-11e9-98dc-bffe8276d30c.png)
![device-2019-08-23-123844](https://user-images.githubusercontent.com/222393/63587156-75e82500-c5a3-11e9-9e9f-66876ac8f506.png)

Differential Revision: D16992488

Pulled By: sahrens

fbshipit-source-id: f72a9a890d9056bb017cc5747c6f66b7c35633dd
2019-08-23 13:02:52 -07:00
Joshua Gross 43f042eb7d Work around crashing redbox
Summary: When debugging supporting a Fabric component, a redbox was being shown without a title which resulted in an infinite redbox loop, followed by OOM. That isn't helpful so we just safely work around the null title while indicating what happened to aid in debugging.

Reviewed By: lunaleaps

Differential Revision: D16921053

fbshipit-source-id: df1f4a691df0c10de53c91e4a9356a04a9005a97
2019-08-20 22:28:54 -07:00
Christoph Nakazawa bb625e5238 Remove the native delta client from Android
Summary: This was an experiment to patch individual deltas in development instead of reloading the whole JS bundle. With improvements such as Fast Refresh that reduces the need for reloads and bundle splitting that reduces the number of modules and memory by 10x, we won't be needing this complex optimization that we never properly made work. This diff removes that code and I will be removing the JS side of things in Metro in a follow-up diff.

Reviewed By: fkgozali

Differential Revision: D16832709

fbshipit-source-id: 46596a3126d52d7d74f4b9ffc9a6ee9d82ec9522
2019-08-16 10:02:46 -07:00
Oleksandr Melnykov a5aaca7d4f Migrate RCTAxialGradientView to JS codegen
Summary:
This diff migrates `RCTAxialGradientView` to use generated `RCTAxialGradientViewManagerDelegate` for setting its props. The following base properties have been added to `BaseViewManager`:

```
  protected void setBorderRadius(T view, float borderRadius) {}

  protected void setBorderBottomLeftRadius(T view, float borderRadius) {}

  protected void setBorderBottomRightRadius(T view, float borderRadius) {}

  protected void setBorderTopLeftRadius(T view, float borderRadius) {}

  protected void setBorderTopRightRadius(T view, float borderRadius) {}
```

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D16784173

fbshipit-source-id: f3971985efee2b6e0a5fb248b89c4809305e670c
2019-08-14 04:44:12 -07:00
Oleksandr Melnykov 639da37dc5 Use feature flag to enable view manager delegates for setting props
Summary: This diff adds a feature flag which must be enabled if view managers should be allowed to use a delegate for setting their properties.

Reviewed By: mdvacca

Differential Revision: D16762876

fbshipit-source-id: ae3466d7f02ed02f203dbb79f5e0843e6d9fdd45
2019-08-14 03:30:14 -07:00
Oleksandr Melnykov 50fe8119de Use ViewManagerDelegate if provided instead of $$PropsSetter to update view props
Summary:
This diff introduces an interface `ViewManagerDelegate` and its base implementation `BaseViewManagerDelegate`, which is used as a parent class for all view manager delegates generated by the JS codegen. Before the changes in this diff, generated delegates didn't support setting the base view properties such as background color, rotation, opacity, etc. Now it's possible to do by using `BaseViewManagerDelegate.setProperty(...)`, and since all generated delegates extend BaseViewManagerDelegate, they can just call `super.setProperty(...)` for properties they don't want to handle.

This diff also introduced a new method `ViewManager.getDelegate()`. This will allow view managers to return an instance of the delegate generated by JS and ensure that the view properties are set in a type-safe manner. If this method returns null (it does by default), we fall back to the default implementation of setting view properties using Java-generated `$$PropsSetter`
classes.

This is an example of an interface class generated by JS:

```
public interface RCTAxialGradientViewViewManagerInterface<T extends View> {
  void setColors(T view, Nullable ReadableArray value);
  void setLocations(T view, Nullable ReadableArray value);
  void setEndX(T view, Float value);
  void setEndY(T view, Float value);
  void setStartX(T view, Float value);
  void setStartY(T view, Float value);
}
```

This is an example of a delegate class generated by JS:

```
public class RCTAxialGradientViewManagerDelegate<T extends View, U extends BaseViewManager<T, ? extends LayoutShadowNode> & RCTAxialGradientViewManagerInterface<T>> extends BaseViewManagerDelegate<T, U> {
  public RCTAxialGradientViewManagerDelegate(U viewManager) {
    super(viewManager);
  }
  Override
  public void setProperty(T view, String propName, Nullable Object value) {
    switch (propName) {
      case "colors":
        mViewManager.setColors(view, (ReadableArray) value);
        break;
      case "locations":
        mViewManager.setLocations(view, (ReadableArray) value);
        break;
      case "endX":
        mViewManager.setEndX(view, value == null ? Float.NaN : ((Double) value).floatValue());
        break;
      case "endY":
        mViewManager.setEndY(view, value == null ? Float.NaN : ((Double) value).floatValue());
        break;
      case "startX":
        mViewManager.setStartX(view, value == null ? Float.NaN : ((Double) value).floatValue());
        break;
      case "startY":
        mViewManager.setStartY(view, value == null ? Float.NaN : ((Double) value).floatValue());
        break;
      default:
        super.setProperty(view, propName, value);
    }
  }
}
```

NOTE: What if a view manager, for instance ReactAxialGradientManager, wanted to add support for the borderRadius prop? In the old Java codegen, it would just need to create a method and annotate it with ReactProp (name = ViewProps.BORDER_RADIUS) and $$PropsSetter would call this method when a property with this name must be set. With the new JS codegen, borderRadius is a part of the basic view props, so setBorderRadius is not generated as a part of the ViewManagerInterface, so it’s not possible to set this value. I see two options: 1) add a method boolean setProperty (String propName, Object value) and let the view manager handle it in a non-type safe way (return true if it’s been handled). 2) Generate BaseViewManagerInterface which will include all basic view props and make BaseViewManager implement this interface, leaving all methods empty so that it stays compatible with the current implementation. Override these methods in a view manager that needs to handle a specific property in a custom way (so we would override setBorderRadius in ReactAxialGradientManager).

Reviewed By: mdvacca

Differential Revision: D16667686

fbshipit-source-id: 06a15a92f8af55640b7a53c5a34f40366d1be2a8
2019-08-12 06:36:24 -07:00
Onti Vals c0c5e2276d React edit text changes (#25964)
Summary:
Changing showSoftKeyboard and hideSoftKeyboard to be protected, as we need this change for an internal control that extends ReactEditText.

## Changelog

[Android] [Changed] - part of our react native platform, we have a control that extends ReactEditText and we need to be able to override these 2 methods.
Pull Request resolved: https://github.com/facebook/react-native/pull/25964

Test Plan: The change has been in Microsoft's branch of RN for almost 2 years, and since it's a relatively small change we've done a quick sanity check in RNTester prior to this PR, making sure the TextInput page loads fine and it's functional.

Differential Revision: D16686878

Pulled By: cpojer

fbshipit-source-id: 63035ee9c58e93bc0fa40e5bec318df05322c6c5
2019-08-08 01:43:27 -07:00
Joshua Gross 751d0e75b0 Fabric PerfLogger: prevent ConcurrentModificationException
Summary: Some surfaces throw ConcurrentModificationException when logging detailed perf for Fabric. I've refactored the ReactMarker class to use a threadsafe ArrayList and removed synchronization, which is safer and should improve perf everywhere the markers are used, even if there are zero listeners.

Reviewed By: mdvacca

Differential Revision: D16656139

fbshipit-source-id: 34572f9ad19028a273e0837b0b895c5e8a47976a
2019-08-05 15:38:04 -07:00
Florian Cargoët 28d50189f3 Fixed android bounding box (#25836)
Summary:
This PR fixes https://github.com/facebook/react-native/issues/19637.

Summary of the issue: on Android, transformed touchables have press issues because the touchable's measurements are incorrect in the release phase. `UIManager.measure()` returns an incorrect size and position as explained [here](https://github.com/facebook/react-native/issues/19637#issuecomment-396065914)

This is easily seen with the inspector :

**Screenshot of a { scale: 2 } transform**
The real view (scaled) is in pink, the incorrect touchable area is in blue.
<img src="https://user-images.githubusercontent.com/110431/41190133-8d07ad02-6bd9-11e8-873d-93776a007309.png" width="200"/>

**Screenshot of a { rotateZ: "-45deg" } transform**
The real view (rotated) is in pink, the incorrect touchable area is in blue.
<img src="https://user-images.githubusercontent.com/110431/41190136-a1a079a6-6bd9-11e8-906d-729015bcab6b.png" width="200"/>

## Changelog

[Android] [Fixed] - Fix UIManager.measure()
Pull Request resolved: https://github.com/facebook/react-native/pull/25836

Test Plan:
Android now produces the same results as iOS as seen on these screenshots

| Android without fix | Android with fix | iOS |
| --- | --- | --- |
| ![Screenshot_1564133103](https://user-images.githubusercontent.com/110431/61941632-28729b00-af98-11e9-9706-b13968b79df5.png) | ![Screenshot_1564130198](https://user-images.githubusercontent.com/110431/61941631-28729b00-af98-11e9-9ff3-5f2748077dbe.png) | ![Simulator Screen Shot - iPhone X - 2019-07-26 at 11 19 34](https://user-images.githubusercontent.com/110431/61941633-28729b00-af98-11e9-8dc4-22b61178242e.png) |

Reviewed By: cpojer

Differential Revision: D16598914

Pulled By: makovkastar

fbshipit-source-id: d56b008b717ea17731fb09001cbd395aa1b044fe
2019-08-05 04:42:50 -07:00
Ramanpreet Nara bc7c85f153 Delete jsi::Functions before jsi::Runtime gets deleted
Summary:
## The Problem
1. `CatalystInstanceImpl` indirectly holds on to the `jsi::Runtime`. When you destroy `CatalystInstanceImpl`, you destroy the `jsi::Runtime`. As a part of reloading React Native, we destroy and re-create `CatalystInstanceImpl`, which destroys and re-creates the `jsi::Runtime`.
2. When JS passes in a callback to a TurboModule method, we take that callback (a `jsi::Function`) and wrap it in a Java `Callback` (implemented by `JCxxCallbackImpl`). This Java `Callback`, when executed, schedules the `jsi::Function` to be invoked on a Java thread at a later point in time. **Note:** The Java NativeModule can hold on to the Java `Callback` (and, by transitivity, the `jsi::Function`) for potentially forever.
3. It is a requirement of `jsi::Runtime` that all objects associated with the Runtime (ex: `jsi::Function`) must be destroyed before the Runtime itself is destroyed. See: https://fburl.com/m3mqk6wt

### jsi.h
```
/// .................................................... In addition, to
/// make shutdown safe, destruction of objects associated with the Runtime
/// must be destroyed before the Runtime is destroyed, or from the
/// destructor of a managed HostObject or HostFunction.  Informally, this
/// means that the main source of unsafe behavior is to hold a jsi object
/// in a non-Runtime-managed object, and not clean it up before the Runtime
/// is shut down.  If your lifecycle is such that avoiding this is hard,
/// you will probably need to do use your own locks.
class Runtime {
 public:
  virtual ~Runtime();
```

Therefore, when you delete `CatalystInstanceImpl`, you could end up with a situation where the `jsi::Runtime` is destroyed before all `jsi::Function`s are destroyed. In dev, this leads the program to crash when you reload the app after having used a TurboModule method that uses callbacks.

## The Solution
If the only reference to a `HostObject` or a `HostFunction` is in the JS Heap, then the `HostObject` and `HostFunction` destructors can destroy JSI objects. The TurboModule cache is the only thing, aside from the JS Heap, that holds a reference to all C++ TurboModules. But that cache (and the entire native side of `TurboModuleManager`) is destroyed when we call `mHybridData.resetNative()` in `TurboModuleManager.onCatalystInstanceDestroy()` in D16552730. (I verified this by commenting out `mHybridData.resetNative()` and placing a breakpoint in the destructor of `JavaTurboModule`). So, when we're cleaning up `TurboModuleManager`, the only reference to a Java TurboModule is the JS Heap. Therefore, it's safe and correct for us to destroy all `jsi::Function`s created by the Java TurboModule in `~JavaTurboModule`. So, in this diff, I keep a set of all `CallbackWrappers`, and explicitly call `destroy()` on them in the `JavaTurboModule` destructor. Note that since `~JavaTurboModule` accesses `callbackWrappers_`, it must be executed on the JS Thread, since `createJavaCallbackFromJSIFunction` also accesses `callbackWrappers_` on the JS Thread.

For additional safety, I also eagerly destroyed the `jsi::Function` after it's been invoked once. I'm not yet sure if we only want JS callbacks to only ever be invoked once. So, I've created a Task to document this work: T48128233.

Reviewed By: mdvacca

Differential Revision: D16623340

fbshipit-source-id: 3a4c3efc70b9b3c8d329f19fdf4b4423c489695b
2019-08-02 17:08:19 -07:00
David Vacca e2f82b122e Implement JNI code to invoke Android JSResponder methods from C++
Summary: This diff implements the JNI code required for Android to receive JSResponderHandler calls

Reviewed By: JoshuaGross, makovkastar

Differential Revision: D16543431

fbshipit-source-id: 38cff16a05633fccefa201b189d761d503a9b839
2019-08-02 16:38:45 -07:00
David Vacca 0e067fd23b Implement JS Responder Handler in Fabric Android
Summary: This diff implements the JSResponderHandler in Fabric Android code

Reviewed By: JoshuaGross

Differential Revision: D16543438

fbshipit-source-id: 13680f77a5368e8ba1180383a5f9fb7d7330b90a
2019-08-02 16:38:45 -07:00
David Vacca 08e4537680 Expose JS Responder handler in Scheduler API
Summary: This diff implements the JSResponderHandler methods in the core of RN (scheduler API and friends)

Reviewed By: ejanzer

Differential Revision: D16543437

fbshipit-source-id: dac03e30c4330d182ecf134f3174ba942dbf7289
2019-08-02 16:38:45 -07:00
David Vacca 470ace0932 Expose JSResponderHandler API in Fabric UIManager
Summary:
This diff adds the new API required to implment JSResponderHandler in FabricUIManager
The new API differs from the old API, but since setJSResponder is called ONLY from JS it's not necessary to have this method as part of UIManager interface.

Reviewed By: JoshuaGross

Differential Revision: D16543440

fbshipit-source-id: ca4bd4c1e4df706cda0eb16798e01f3350558d06
2019-08-02 16:38:44 -07:00
Ramanpreet Nara 82e4d28e25 Move ownership of TurboModule jni ref to JavaTurboModule
Summary: When you create a TurboModule from the JS side, we instantiate its Java class and simply make this `javaobject` a `jni::global_ref` in C++. But the reason why we need to make this a global ref is because `JavaTurboModule` needs it to be a global reference for method calls. Making this a `jni::global_ref` from the perspective to TurboModuleManager doesn't really make any sense. So, this diff refactors that bit of code.

Differential Revision: D16622133

fbshipit-source-id: 6a5c20bb405b945c06378a3423d5e7eb38ef244c
2019-08-02 10:08:40 -07:00
Ramanpreet Nara 29ec6401f6 Perform cleanup when ReactApplicationContext is destroyed
Summary:
## Description
When we reload the app, `ReactApplicationContext.destroy()` is called. This method calls `CatalystInstanceImpl.destroy()`, which (on the NativeModules thread) calls `NativeModuleRegistry.notifyJSInstanceDestroy()`, which loops over all its `ModuleHolder`s, and calls `ModuleHolder.destroy()`, each of which call their NativeModule's `onCatalystInstanceDestroy()` method.

TurboModuleManager is a `JSIModule`, so it also has its `onCatalystInstanceDestroy()` method called when the `ReactApplicationContext` is destroyed. But `TurboModuleManager.onCatalystInstanceDestroy()` doesn't do anything. Instead, at the very least, it should call `onCatalystInstanceDestroy()` of all NativeModules.

Differential Revision: D16622132

fbshipit-source-id: da29e698b5217232cfaa01e6f817aab18ceb526f
2019-08-02 10:08:40 -07:00
Ramanpreet Nara 7ab517b5fe Make TurboModules long-lived on Android
Summary: On iOS, calling the `__turboModuleProxy` function with the same name returns the same instance of the TurboModule. Adding this behaviour to Andorid as well.

Differential Revision: D16622131

fbshipit-source-id: 472011ac3356e7c30497f848be0c888596c449b1
2019-08-02 10:08:39 -07:00
Eric Williamson 283a9dcc11 Revert D16553363: [RN][TurboModule] Make TurboModules long-lived on Android
Differential Revision:
D16553363

Original commit changeset: c95e150d6967

fbshipit-source-id: d035724ce131b560abc70e94410d727951a83241
2019-08-01 21:59:45 -07:00
Eric Williamson 5263660c07 Revert D16552730: [RN][TurboModule] Perform cleanup when ReactApplicationContext is destroyed
Differential Revision:
D16552730

Original commit changeset: 04459185262e

fbshipit-source-id: af6ac53f2026385167ded3616926062161cb92aa
2019-08-01 21:59:45 -07:00
Eric Williamson af4862ce32 Revert D16555673: [RN][TurboModule] Move ownership of TurboModule jni ref to JavaTurboModule
Differential Revision:
D16555673

Original commit changeset: 2778fc5a372c

fbshipit-source-id: fac3a6ea185acaa750f58e19d24c194668749636
2019-08-01 21:59:45 -07:00
Eric Williamson 6bc0c108eb Revert D16589168: [RN][TurboModule] Delete jsi::Functions before jsi::Runtime gets deleted
Differential Revision:
D16589168

Original commit changeset: a1c0786999c2

fbshipit-source-id: 8048d62e958c0b58aface00dae8447b8c2d5d2dc
2019-08-01 21:59:45 -07:00
Sensen Chen 2198203ddb - Add null/undefined check for error handling
Summary: Same as title. Changing as per suggestion.

Reviewed By: furdei

Differential Revision: D16615807

fbshipit-source-id: 1c35ae1471beb2460e975841f367ffd49ce34494
2019-08-01 20:50:42 -07:00
David Vacca e3ec8dbe15 Deprecate method UIManagerModule.playTouchSound()
Summary: The method UIManagerModule.playTouchSound() is deprecated, please use SoundManager.playTouchSound() instead.

Reviewed By: JoshuaGross

Differential Revision: D16543436

fbshipit-source-id: 4b391dd1a0af16567ea5574ddfa33484ca488b90
2019-08-01 19:49:12 -07:00
David Vacca 1452acdfff Register Turbo Module SoundManager in Catalyst, FB4A and Workplace
Summary: This diff registers the new Turbo Module SoundManager in Catalyst, FB4A and Workplace

Reviewed By: JoshuaGross

Differential Revision: D16543432

fbshipit-source-id: de3ab175befd5cc11403159e3aa42e5f17d0a280
2019-08-01 19:49:12 -07:00
David Vacca a55ae3a3f0 Implement SoundManager TurboModule in JS and Android
Summary: This diff implements the Turbo Module SoundManager, this will be used by following diffs of the stack

Reviewed By: JoshuaGross

Differential Revision: D16543430

fbshipit-source-id: 34ba545f54b759fe4e49d4e3c5f8867205de907c
2019-08-01 19:49:11 -07:00
Ramanpreet Nara 2a8c188701 Delete jsi::Functions before jsi::Runtime gets deleted
Summary:
## The Problem
1. `CatalystInstanceImpl` indirectly holds on to the `jsi::Runtime`. When you destroy `CatalystInstanceImpl`, you destroy the `jsi::Runtime`. As a part of reloading React Native, we destroy and re-create `CatalystInstanceImpl`, which destroys and re-creates the `jsi::Runtime`.
2. When JS passes in a callback to a TurboModule method, we take that callback (a `jsi::Function`) and wrap it in a Java `Callback` (implemented by `JCxxCallbackImpl`). This Java `Callback`, when executed, schedules the `jsi::Function` to be invoked on a Java thread at a later point in time. **Note:** The Java NativeModule can hold on to the Java `Callback` (and, by transitivity, the `jsi::Function`) for potentially forever.
3. It is a requirement of `jsi::Runtime` that all objects associated with the Runtime (ex: `jsi::Function`) must be destroyed before the Runtime itself is destroyed. See: https://fburl.com/m3mqk6wt

### jsi.h
```
/// .................................................... In addition, to
/// make shutdown safe, destruction of objects associated with the Runtime
/// must be destroyed before the Runtime is destroyed, or from the
/// destructor of a managed HostObject or HostFunction.  Informally, this
/// means that the main source of unsafe behavior is to hold a jsi object
/// in a non-Runtime-managed object, and not clean it up before the Runtime
/// is shut down.  If your lifecycle is such that avoiding this is hard,
/// you will probably need to do use your own locks.
class Runtime {
 public:
  virtual ~Runtime();
```

Therefore, when you delete `CatalystInstanceImpl`, you could end up with a situation where the `jsi::Runtime` is destroyed before all `jsi::Function`s are destroyed. In dev, this leads the program to crash when you reload the app after having used a TurboModule method that uses callbacks.

## The Solution
If the only reference to a `HostObject` or a `HostFunction` is in the JS Heap, then the `HostObject` and `HostFunction` destructors can destroy JSI objects. The TurboModule cache is the only thing, aside from the JS Heap, that holds a reference to all C++ TurboModules. But that cache (and the entire native side of `TurboModuleManager`) is destroyed when we call `mHybridData.resetNative()` in `TurboModuleManager.onCatalystInstanceDestroy()` in D16552730. (I verified this by commenting out `mHybridData.resetNative()` and placing a breakpoint in the destructor of `JavaTurboModule`). So, when we're cleaning up `TurboModuleManager`, the only reference to a Java TurboModule is the JS Heap. Therefore, it's safe and correct for us to destroy all `jsi::Function`s created by the Java TurboModule in `~JavaTurboModule`. So, in this diff, I keep a set of all `CallbackWrappers`, and explicitly call `destroy()` on them in the `JavaTurboModule` destructor. Note that since `~JavaTurboModule` accesses `callbackWrappers_`, it must be executed on the JS Thread, since `createJavaCallbackFromJSIFunction` also accesses `callbackWrappers_` on the JS Thread.

For additional safety, I also eagerly destroyed the `jsi::Function` after it's been invoked once. I'm not yet sure if we only want JS callbacks to only ever be invoked once. So, I've created a Task to document this work: T48128233.

Reviewed By: mhorowitz

Differential Revision: D16589168

fbshipit-source-id: a1c0786999c22bef55d416beb0fc40261447a807
2019-08-01 16:36:47 -07:00
Ramanpreet Nara 88e03fa4db Move ownership of TurboModule jni ref to JavaTurboModule
Summary: When you create a TurboModule from the JS side, we instantiate its Java class and simply make this `javaobject` a `jni::global_ref` in C++. But the reason why we need to make this a global ref is because `JavaTurboModule` needs it to be a global reference for method calls. Making this a `jni::global_ref` from the perspective to TurboModuleManager doesn't really make any sense. So, this diff refactors that bit of code.

Reviewed By: mdvacca

Differential Revision: D16555673

fbshipit-source-id: 2778fc5a372c41847e8296c2e22bb9a8826fcc52
2019-08-01 16:36:47 -07:00
Ramanpreet Nara e6c8ace576 Perform cleanup when ReactApplicationContext is destroyed
Summary:
## Description
When we reload the app, `ReactApplicationContext.destroy()` is called. This method calls `CatalystInstanceImpl.destroy()`, which (on the NativeModules thread) calls `NativeModuleRegistry.notifyJSInstanceDestroy()`, which loops over all its `ModuleHolder`s, and calls `ModuleHolder.destroy()`, each of which call their NativeModule's `onCatalystInstanceDestroy()` method.

TurboModuleManager is a `JSIModule`, so it also has its `onCatalystInstanceDestroy()` method called when the `ReactApplicationContext` is destroyed. But `TurboModuleManager.onCatalystInstanceDestroy()` doesn't do anything. Instead, at the very least, it should call `onCatalystInstanceDestroy()` of all NativeModules.

Reviewed By: mdvacca

Differential Revision: D16552730

fbshipit-source-id: 04459185262e92d69570facb368578a848cc5fdb
2019-08-01 16:36:46 -07:00
Ramanpreet Nara e876cc68e7 Make TurboModules long-lived on Android
Summary: On iOS, calling the `__turboModuleProxy` function with the same name returns the same instance of the TurboModule. Adding this behaviour to Andorid as well.

Reviewed By: mdvacca

Differential Revision: D16553363

fbshipit-source-id: c95e150d6967604a808cfb49877b7a633e33d729
2019-08-01 16:36:46 -07:00
David Vacca a84a62834c Fix lint in MountingManager
Summary: This fixes the lint that was broken by D16543439

Reviewed By: fkgozali

Differential Revision: D16607092

fbshipit-source-id: 70c3989d25f446ffe0877e58253acef5ada8e010
2019-08-01 14:11:32 -07:00
David Vacca 1054930d45 Remove ViewPooling from Fabric Android
Summary:
View Pooling is not currently being used in Fabric Android, this diff removes all the extra abstractions that are being used becuase of the unused ViewPooling.
We might add this in the future when we re-implement view correctly.

Reviewed By: JoshuaGross

Differential Revision: D16543439

fbshipit-source-id: f41b6e02fddc36c7ef7a1052399d2e6b2041fcfb
2019-07-31 23:30:55 -07:00
Ram N bc2b52d22c Enable Starting Sampling profile at App Startup
Summary: In Dev Settings, we used to have an `Start Sampling Profiler on init` option, which was defunct. This diff re-enables that option. We can now start the Sampling Profiler on app start

Reviewed By: yinghuitan

Differential Revision: D7022382

fbshipit-source-id: 1db85d8a324e401c71187ba5871a91abcc18acf9
2019-07-31 13:54:23 -07:00
Spencer Ahrens c46353abe4 Force FbReactInstanceManagerAppJob.IdleExecutor to run in the UIThread
Summary:
`createReactContextInBackground` must be run on the UI Thread, so we update the DI annotation from `DefaultIdleExecutor` to `ForUiThread` so that happens.

Unclear why Infer didn't flag this. Asked [here](https://fb.workplace.com/groups/572076376174315/permalink/2661423100572955/).

Also adds an additional thread assert before setting `mHasStartedCreatingInitialContext = true` so that if a similar bug happens in the future, RN won't be completely hosed.

Reviewed By: mdvacca

Differential Revision: D16574901

fbshipit-source-id: 02ba63979904e9df9ef6d782aa7379cb44702508
2019-07-30 20:28:27 -07:00
Joshua Gross bf5e73e46c Always pass props and state to Create mount item
Summary: For Litho interop and to resolve T47926405, always pass props and state to Create mount item so that any ViewManager can create view instances with knowledge of initial props and state.

Reviewed By: mdvacca

Differential Revision: D16554082

fbshipit-source-id: 3b19a43347b0fa201a054eec60e82fb77cad3625
2019-07-29 18:15:03 -07:00
Joshua Gross 62591ac840 Set scheduler delegate during construction
Summary: I think it's possible that there's a race condition between creating the scheduler and setting the delegate leading to bugs like T47272192.

Reviewed By: mdvacca

Differential Revision: D16537737

fbshipit-source-id: 9c579537658be5a9aeed37c0e4935c997cabb6aa
2019-07-29 10:42:31 -07:00
Joshua Gross 44be1f1516 Better error messages for "unable to find view for viewstate"
Summary: Better error messages for "unable to find view for viewstate"

Reviewed By: shergin

Differential Revision: D16537698

fbshipit-source-id: a8ea681882207cd894e3703c6c0a04346fa4ce06
2019-07-29 10:42:31 -07:00
cpojer d7f5153cd8 Add Hermes support to React Native on Android (#25613)
Summary:
Yesterday we shipped hermesengine.dev as part of the current 0.60 release. This PR brings those changes to master.

## Changelog

[General] [Added] - Added support for Hermes
Pull Request resolved: https://github.com/facebook/react-native/pull/25613

Test Plan:
* CI is green both on GitHub and at FB
* Creating a new app from source can use Hermes on Android

Reviewed By: cpojer

Differential Revision: D16221777

Pulled By: willholen

fbshipit-source-id: aa6be10537863039cb666292465ba2e1d44b64ef
2019-07-25 23:05:53 -07:00
Fred Liu 983ba63025 Back out "[RN][Android] Release underlying resources when JS instance is GC'ed on Android"
Summary: Original commit changeset: 12f14fa4a582

Reviewed By: furdei

Differential Revision: D16494091

fbshipit-source-id: f3080873a11ebb376e819b102fc13efe97146a89
2019-07-25 08:43:07 -07:00