Граф коммитов

94 Коммитов

Автор SHA1 Сообщение Дата
Andres Suarez 3b31e69e28 Tidy up license headers [2/n]
Summary: Changelog: [General] [Fixed] - License header cleanup

Reviewed By: yungsters

Differential Revision: D17952694

fbshipit-source-id: 17c87de7ebb271fa2ac8d00af72a4d1addef8bd0
2019-10-16 10:06:34 -07:00
Janette Cheng 10cc834567 Use fbandroid_labels and fbobjc_labels in xplat targets
Summary:
`xplat` targets add different deps based on what platform the target is being built for.

for anything using `fb_xplat`, we can put all ios supermodules in `fbobjc_labels` and all android sms in `fbandroid_labels`

There's some weirdness with python targets like `thrift_gen` in  `/xplat/mobileconfig/tools/generator/gen-py/BUCK` that don't have platform-specific labels because the except_for list for `fbandroid` doesn't need the `fbsource//` prefix (see changes in `/ios/isolation/infra.mobileconfig.sm`)

Changelog: [Internal]

Reviewed By: shergin, joshleibsly

Differential Revision: D17884952

fbshipit-source-id: e245364cf515b75682990094d24f789d53b1f3f5
2019-10-15 19:32:27 -07:00
Peter Argany 474f12e6fc Fix NSDictionary crash in bridgeless RN
Summary: Changelog: [iOS] [Fixed] Fixed crash in TurboModules when bridge isn't set

Reviewed By: RSNara

Differential Revision: D17939896

fbshipit-source-id: 4c59de00da30fa8ceb32e590d65663ac37ff3ca2
2019-10-15 16:18:21 -07:00
Ramanpreet Nara 63e58a375e Improve method argument RCTConvert logic
Summary:
**Context**
For method calls from JS to Objective C, we have to convert JS values to ObjC objects/primitives. Before we can call our ObjC methods, we need to run both the ObjC primitives and objects through `RCTConvert`. This is necessary, because we sometimes convert `NSDictionary`s to special Objective C objects. Apparently, in `RCTTiming`, we also do the same with `double` (i.e: we convert a `double` arg to another `double` type with different meaning).

**Problem**
`RCTTiming` used `RCTConvert` to convert `double`s into `NSTimeInterval` (also a double). The conversion is defined like this:

```
// i.e: division by 1000
RCT_CUSTOM_CONVERTER(NSTimeInterval, NSTimeInterval, [self double:json] / 1000.0)
```

This diff implements the support necessary to make this work. For completeness, I also implemented the same functionality for `BOOL`s.

Changelog: [iOS][Fixed] Improve method argument RCTConvert logic

Reviewed By: mdvacca

Differential Revision: D17887915

fbshipit-source-id: 3246fdbf4db7e96911f16460d92448b1f1e99444
2019-10-14 15:39:34 -07:00
Ramanpreet Nara bec5b8711f Improve TM main queue initialization
Summary:
In the legacy system, when NativeModules are supposed to be initialized on the main queue, we do the following synchronously on the main thread:
1. Done: Attach bridge on main queue
2. Register the NativeModule for frame updates on main queue
3. Post Notification that NativeModule was created on main queue
4. Attach methodQueue on main queue
5. Call new on main queue

`[RCTModuleData instance]` is the entrypoint for all of this logic.

We probably shouldn't synchronously execute all this initialization on the main queue, because it can lead to deadlocks down the road. Therefore, this diff makes it so that we still call `new` on the same thread. However, we do all other initialization in the main thread, if that's required.

Changelog: [iOS][Fixed] TurboModule initialization on the main queue

Reviewed By: PeteTheHeat

Differential Revision: D17867583

fbshipit-source-id: a88412ee1e3d93a4f9b5ab0b4dd8fc5213fa91f8
2019-10-14 14:19:23 -07:00
Adam Ernst a45e6a8b5f React Native supermodule
Summary: #nocancel

Reviewed By: fkgozali

Differential Revision: D17747685

fbshipit-source-id: 9bad072d3549959528612c2f0329799853d4b675
2019-10-03 15:43:38 -07:00
Emily Janzer 7a6478465d Throw pending JNI errors after Java method discovery in TM
Summary: I was trying to debug an issue with a method signature mismatch last week, but was having trouble figuring out the problem because the error I was getting was unrelated - apparently JNI will sometimes swallow the error and just fail mysteriously later on. Ramanpreet showed me this macro that will throw any pending exceptions, so let's do that after we try to lookup the Java method in case it fails.

Reviewed By: RSNara

Differential Revision: D17680121

fbshipit-source-id: 1f23e49014f7cc1616e111386d440637e6a74677
2019-10-02 12:03:13 -07:00
Kevin Gozali 1452954c4c TM: Add mutex to access LongLivedObjectCollection - making it thread safe
Summary:
There are cases where the CallbackWrapper instances were added from different thread, potentially crashing the inner std::unordered_set<> we're using to keep the wrappers alive for extended time.

To avoid it, let's just use std::mutex.

Reviewed By: shergin

Differential Revision: D17631233

fbshipit-source-id: e8f98004e45a68be31f8f0cda118fb67dcb06d45
2019-09-27 13:44:49 -07:00
Ramanpreet Nara 689233b018 Implement async method dispatch
Summary: Now that all the plumbing is done, this diff finally implements async method dispatch on the NativeModule thread.

Reviewed By: mdvacca

Differential Revision: D17480605

fbshipit-source-id: 992aab99954c488a0327144d84a1668a2b158d04
2019-09-20 10:52:58 -07:00
Ramanpreet Nara 43807f04fe Give TurboModules access to the native CallInvoker
Summary:
Self explanatory.
1. **Existing:** We create the NativeModules thread in CatalystInstanceImpl.cpp.
2. D17422165: We wrap this thread in a `BridgeNativeCallInvoker`.
3. D17422164: We use `CatalystInstanceImpl::getNativeCallInvokerHolder()` to get a hold of the `BridgeNitiveCallInvoker` in Java.
4. D17422163: From Java, we pass this `CallInvokerHolder` to `TurboModuleManager`'s constructor.
5. **This diff:** `TurboModuleManager` then unwraps the `CallInvoker` from `CallInvokerHolder`, and passes it to all TurboModules in their constructor.

Reviewed By: PeteTheHeat

Differential Revision: D17422160

fbshipit-source-id: c0a76dfe5fdedac2e0e21f7a562bc7588dc190fb
2019-09-20 10:52:57 -07:00
Ramanpreet Nara 4c998fd05d Rename JSCallInvoker{,Holder} to CallInvoker{,Holder}
Summary:
## Motivation
The concept behind JSCallInvoker doesn't necessarily have to apply only to the JS thread. On Android, we need to re-use this abstraction to allow execution of async method calls on the NativeModules thread.

Reviewed By: PeteTheHeat

Differential Revision: D17377313

fbshipit-source-id: 3d9075cbfce0b908d800a366947cfd16a3013d1c
2019-09-20 10:52:56 -07:00
Ramanpreet Nara 15343863b0 Ensure callbacks can only be called once on the native side
Summary: This is just a minor fix to D17380360. Basically, we want to make sure that callbacks can only be called once on the ObjC side.

Reviewed By: fkgozali

Differential Revision: D17403852

fbshipit-source-id: b0d2bbd539daef4837a345bc2953dd9687b3147b
2019-09-16 17:30:15 -07:00
Kevin Gozali 3809f2795a TM iOS: Properly clean up CallbackWrapper during reload
Summary: There is a race condition between tearing down the bridge vs CallbackWrapper invoking the wrapped jsi::Function. This means the wrapper can be stale and unsafe to access after the bridge dies. To avoid unsafe access, let's clean it up properly using weak ref.

Reviewed By: RSNara

Differential Revision: D17380360

fbshipit-source-id: f91ce75d945bf8ed0b141c593bcc674ff465aa8c
2019-09-13 19:20:55 -07:00
Ramanpreet Nara bd8e6cd2e4 Fix JNI local reference table overflow problem
Summary:
This diff improves on D17244061 by moving the `Jni::JniLocalScope` to earlier on in `JavaTurboModule::invokeJavaMethod`. The problem with D17244061 was two-fold:
1. It completely ignored the local references created by just calling `getConstants()`. So, you could spam `getConstants()` and still get an overflow.
2. The capacity was incorrect. Basically, two things happen when you call `PushLocalFrame`. One, we push an actual Jni frame onto a stack somewhere in the dvm: https://fburl.com/f16wfrxi, https://fburl.com/jhpha563. Popping off this stack is  how the local references are cleared when we call `PopLocalFrame`. Two, we do a check to make sure that we can support at least `capacity` more local references: https://fburl.com/jucvew8j. The max is 512. The problem before was that, I was just using the argument count for the capacity. This was inaccurate because we create many `jclass` objects, and several misc. intermediate java objects when we invoke `JavaTurboModule::invokeJavaMethod`. So, I've refined the calculation of capacity to be a bit more accurate. This should make sure that capacity check works, which should help us fail faster if our local reference count approaches 512.

Reviewed By: shergin

Differential Revision: D17337354

fbshipit-source-id: 45574bae4748c52d8f919c1480b9a0936d970628
2019-09-13 18:37:22 -07:00
Ramanpreet Nara fc9c53d621 Refactor Promise returns
Summary:
Because we use the `PromiseWrapper` struct, we need to explicitly manage its lifecycle to ensure that it doesn't clear before the promise methods are invoked by the ObjC Runtime. This `PromiseWrapper` struct is unnecessary. We could just not have it and create the CallbackWrappers for resolve and reject within the `createPromise` function. Therefore, I moved all the logic from `PromiseWrapper` to the `RCTTurboModule::createPromise` function.

In the next diff, I'm going to keep a track of all the CallbackWrappers we create in instances of RCTTurboModule, and `destroy()` them in the destructor of RCTTurboModule. This should make sure that all `jsi::Function`s are released before we delete the `jsi::Runtime`, which should prevent Marketplace from crashing when we hit CMD + R. See: https://fb.workplace.com/groups/rn.support/permalink/2761112713937326/.

Reviewed By: fkgozali

Differential Revision: D17208729

fbshipit-source-id: ce80c9c01088f0e3dc47c7c29397b7a197d699ce
2019-09-11 14:58:17 -07:00
Ramanpreet Nara 1c2671c8c9 Clean up local references created for method calls
Summary: Whenever we invoke a method, we convert JS arguments into JNI objects. These JNI objects are all local references that need to be manually destroyed after the method call happens. Therefore, I'm using `JniLocalScope` to automatically do this whenever the stack is cleared after the call to `JavaTurboModule::invokeJavaMethod`. This should hopefully get rid of the JNI table overflow we're seeing in T52817336.

Reviewed By: mdvacca

Differential Revision: D17244061

fbshipit-source-id: 92ca78cdb23ad8dfe2425db46e086c10c9662fe2
2019-09-09 13:21:44 -07:00
Ramanpreet Nara 337ec88137 Convert NSNull and nil return value to JS null
Summary: When you return null or [NSNull null] from a TurboModule method, we should just return null to JS.

Reviewed By: fkgozali

Differential Revision: D17141170

fbshipit-source-id: a73410b7a4a765750a8dd9c4e904046ffe1c0fc1
2019-09-06 17:10:03 -07:00
Peter Argany e94f2c3625 Adhere to main queue requirements of native modules
Summary:
Certain turbomodules set `requiresMainQueueSetup` to true. This is b/c they use some fancy APIs in setup that need main queue.

TurboModuleManager mostly adhered to this restriction, the only case it didn't is when setting bridge. There is possibility that this happens on JS thread, which would crash the app for these certain TM. This diff fixes that.

Reviewed By: RSNara

Differential Revision: D16921644

fbshipit-source-id: 69b2410550360d3ccb03c0b71fb7dfccb889eda4
2019-08-21 14:31:02 -07:00
Ramanpreet Nara c237794236 Format code in ReactCommon/turbomodule/core
Summary: Just ran `arc f ReactCommon/turbomodule/core/**/*`.

Reviewed By: ejanzer

Differential Revision: D16691807

fbshipit-source-id: 3f499ffeffaae47bda550c0071c93cd7f48e2a23
2019-08-08 10:52:26 -07:00
Kevin Gozali d2e18a1c5c iOS: Revert RCT->RN prefix renaming to avoid confusion
Summary: The previous rename from RCT->RN prefix ended up causing some confusions on which prefix to use for which files and under what circumstances. To avoid further confusion before we're done with the re-architecture project, let's keep them as RCT.

Reviewed By: mdvacca

Differential Revision: D16705566

fbshipit-source-id: 395bff771c84e5ded6b2261a84c7549df1e6c5e5
2019-08-08 07:21:25 -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
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 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 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
Kevin Gozali 9420de6860 iOS: codemod react-native-github: RCT->RN prefix for Fabric
Summary: Fabric ObjC(++) files will be prefixed by RN* for the time being, this codemod is a simple rename. This includes `interface` and `protocol` definition

Reviewed By: PeteTheHeat, yungsters

Differential Revision: D16611524

fbshipit-source-id: 868d2571ea2414dde4cbb3b75b1334b779b5d832
2019-08-01 20:06:04 -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 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
Championrunner 9115b61083 Update RCTTurboModule.mm (#25831)
Summary:
## Changelog

[CATEGORY] [TYPE] - Typo Error
Pull Request resolved: https://github.com/facebook/react-native/pull/25831

Differential Revision: D16514794

Pulled By: cpojer

fbshipit-source-id: 36a267fe53f78b93de418d1f0b04f04353bf2195
2019-07-26 01:36:32 -07:00
Min ho Kim 84f5ebe4f9 Fix typos (#25770)
Summary:
Fix typos mostly in comments and some string literals.

## Changelog

[General] [Fixed] - Fix typos
Pull Request resolved: https://github.com/facebook/react-native/pull/25770

Differential Revision: D16437857

Pulled By: cpojer

fbshipit-source-id: ffeb4d6b175e341381352091134f7c97d78c679f
2019-07-23 03:23:11 -07:00
Peter Argany 97d3d59077 Comments & category cleanup
Summary: Been reading a lot of code comments getting familiar with Fabric & TM, just fixing a few typos and removing an unused bridge category method.

Reviewed By: shergin

Differential Revision: D16371581

fbshipit-source-id: bf0cc9c873c60e37124dc715c92d7f105e54e42f
2019-07-20 10:41:46 -07:00
Ramanpreet Nara ff9323a0b0 Type-check JS args when converting them to Java args
Summary:
When you call a TurboModule method with JS arguments, we necessarily convert these JS args to Java objects/primitives before passing them to the Java implementation of the method. The problem is that we let the type of the JS arg dictate the type of the Java object/primitive to convert said arg to. This means that if a JS developer passes in an `number` to a function that expects an `Array`, we'll convert the number to a `double` and try to call the Java method with that `double`, when it actually expects a `ReadableArray`. This will trigger a JNI error, and crash the program. Ideally, we should be able to catch these type mismatches early on.

In this diff, on every TurboModule method call, I parse the method signature to determine the Java type of each JS argument. Then, for any argument, if the JS arg and the Java arg types aren't compatible, I raise an exception, which gets displayed as a RedBox in development. This diff also implements support for `?number` and `?boolean` argument and return types in TurboModules.

Reviewed By: mdvacca

Differential Revision: D16214814

fbshipit-source-id: 4399bb88c5344cff50aa8fe8d54eb2000990a852
2019-07-12 22:54:55 -07:00
Kevin Gozali c1b0f398e6 TM iOS: move jscallinvoker under ReactCommon podspec
Summary:
This essentially changes the header namespace to `<ReactCommon/`
Relevant efforts:
https://github.com/facebook/react-native/pull/25619
https://github.com/facebook/react-native/pull/25393

Reviewed By: PeteTheHeat

Differential Revision: D16233125

fbshipit-source-id: 83eda4cc50ebb01efd1ce3eb18f47c97a049cffa
2019-07-12 22:44:20 -07:00
Kevin Gozali 394c9a55be TM iOS: deprecate RN_TURBO_MODULE_ENABLED compiler flag
Summary:
It was added due to missing TM support in .xcodeproj, but since .xcodeproj has been deprecated, we don't need this flag anymore.

Relevant efforts:
https://github.com/facebook/react-native/pull/25619
https://github.com/facebook/react-native/pull/25393

Reviewed By: hramos

Differential Revision: D16231698

fbshipit-source-id: b3276d1fc64394624e5110f2ecd31acc2bb2d240
2019-07-12 22:44:20 -07:00
Kevin Gozali 6e7ce9c082 TM iOS: refactor header dir for TM
Summary:
For better cocoapods compatibility, refactored TM podspec to be a subspec of ReactCommon, then use `<ReactCommon/` header prefix for all TM files.

Relevant efforts:
https://github.com/facebook/react-native/pull/25619
https://github.com/facebook/react-native/pull/25393

Reviewed By: hramos

Differential Revision: D16231697

fbshipit-source-id: 38d3418b19978ff54aa0c61b064ac45ac0e1c36c
2019-07-12 22:44:20 -07:00
James Treanor 6ef91061e8 CocoaPods frameworks compatibility: Step 1 (#25496)
Summary:
This is the first step towards fixing https://github.com/facebook/react-native/issues/25349. These are the changes to the podspec to correctly update dependencies and build config that will cause any breaking change for users or libraries.

I am breaking these changes out from https://github.com/facebook/react-native/pull/25393 as suggested by fkgozali in https://github.com/facebook/react-native/pull/25393#issuecomment-508322884.

These are the changes:

- Made C++ headers in `React-Core` private by default so that ObjC files can import the module without failures.
- Reduced the number of `yoga` headers that are exposed for the same reason as above. As far as I can see this doesn't cause issues but we can find another solution if it does.
- Adding some missing dependencies to fix undefined symbols errors.
- Added `DoubleConversion` to `HEADER_SEARCH_PATHS` where it was missing.

## Changelog

[iOS] [Fixed] - Updated podspecs for improved compatibility with different install types.
Pull Request resolved: https://github.com/facebook/react-native/pull/25496

Test Plan:
Everything should work exactly as before. I have a branch on my [sample project](https://github.com/jtreanor/react-native-cocoapods-frameworks) here which points at this branch to show that it is still working `Podfile` to demonstrate this is fixed.

You can see that it works with these steps:

1. `git clone git@github.com:jtreanor/react-native-cocoapods-frameworks.git`
2. `git checkout podspec-updates`
3. `cd ios && pod install`
4. `cd .. && react-native run-ios`

The sample app will build and run successfully.

Reviewed By: mmmulani

Differential Revision: D16167346

Pulled By: fkgozali

fbshipit-source-id: 1917b2f8779cb172362a457fb3fce686c55056d3
2019-07-10 10:21:38 -07:00
Kevin Gozali 01a42bc368 TM iOS: Prevent module cache invalidation race
Summary: There are still race condition during bridge invalidation. Some modules may be accessing other modules during invalidation, but it's racing with the TM manager clearing the cache.

Reviewed By: JoshuaGross

Differential Revision: D15947488

fbshipit-source-id: 3bd51382264f538a03ca565b0f099da40c3daadf
2019-06-21 15:54:01 -07:00
zhongwuzw af59323d81 TM: Backward compatibility of calling invalidate on module's method queue (#25264)
Summary:
Backward compatibility for `invalidate` method, because for the old module system, we ensure call all methods of modules on `methodQueue`, we also need to keep this rule to avoid some race condition in module.

## Changelog

[iOS] [Fixed] - Backward compatibility of calling invalidate on module's method queue
Pull Request resolved: https://github.com/facebook/react-native/pull/25264

Test Plan: Ensure `invalidate` method of `module` be called on `methodQueue`.

Reviewed By: PeteTheHeat, JoshuaGross

Differential Revision: D15944532

Pulled By: fkgozali

fbshipit-source-id: e260792bc6b86a48cdf376282063cbabccbf26f0
2019-06-21 15:54:01 -07:00
Kevin Gozali 72e276a0f0 TM iOS: Keep LongLivedObject longer past invalidation
Summary:
There can be a race condition between bridge invalidation (hence TM binding invalidation) with pending Promise reject/resolve invocation. If invalidation happens first, invoking the resolve/reject from ObjC class might end up accessing destroyed PromiseWrapper, causing hard crash randomly.

The proper fix is to switch the objc promise resolve/reject block (objc block) to use C++ PromiseWrapper directly, such that the lifecycle of the shared_ptr<> can be correct.

Reviewed By: RSNara

Differential Revision: D15801403

fbshipit-source-id: 9b0c7d323b18d94e920ea3eafc3a6916dadba247
2019-06-13 09:13:51 -07:00
Ramanpreet Nara e908f7a3fe Fix TurboCxxModules::get
Summary: Make `TurboCxxModule::get` return `undefined` when trying to access methods that don't exist. This is what `TurboModule::get` does. So, it makes sense that this behaviour is preserved in TurboCxxModule.

Reviewed By: mdvacca

Differential Revision: D15780044

fbshipit-source-id: 13aeae081655735ef634f1dc09c0dc70a3a3a893
2019-06-13 09:07:24 -07:00
Kevin Gozali c0bf53b716 TM iOS: attempt to convert number args using RCTConvert as well
Summary: Some native modules methods expects number-based args like `NSDate`. For backward compatibility, the incoming numbers should be converted using RCTConvert, just like object args.

Reviewed By: mdvacca

Differential Revision: D15748968

fbshipit-source-id: 4db2cb0c41eda1bbe8cde7b0365d9c3d675f5fb5
2019-06-10 18:41:22 -07:00
Kevin Gozali 7225daf98d TM iOS: Support @synthesize methodQueue auto queue creation
Summary: Some native modules defined `synthesize methodQueue` which will ask the bridge to create a new serial queue for them. TM system needs to preserve this behavior for backward compatibility. In the future though, this magic shall be removed.

Reviewed By: mdvacca

Differential Revision: D15748198

fbshipit-source-id: 66a4b60a2769ac967a8d3bb00c4c635a68daebbc
2019-06-10 18:41:22 -07:00
Kevin Gozali 360e999937 TM iOS: reduce the scope of cache access lock
Summary: We just need to protect access to the cache, we don't need to protect the entire module lookup, because a module initialization may try to lookup another module, causing deadlocks.

Reviewed By: RSNara

Differential Revision: D15690645

fbshipit-source-id: cbb780db8699a94f2c9a2e121b35ddad2b125b65
2019-06-06 09:40:21 -07:00
Ramanpreet Nara a44ab2957c Protect access to RCTTurboModuleCache
Summary: The `_rctTurboModuleCache` `std::unordered_map` can be accessed by multiple threads at the same time via the `provideRCTTurboModule` method. Since `provideRCTTurboModule` both reads and writes to `_rctTurboModuleCache`, this is really bad because we could end up reading from `_rctTurboModuleCache` while it's in an invalid state. Therefore, in this diff, I'm making it so that only one thread at a time can enter `provideRCTTurboModule`.

Reviewed By: fkgozali

Differential Revision: D15609987

fbshipit-source-id: e24e1f5cc2351d8cbb820b7a97074aacd06eec9d
2019-06-03 16:16:09 -07:00
Ramanpreet Nara e1102b43ff Implement Android Cxx TurboModule support
Summary:
## Summary
This diff does a bunch of things:
1. The TurboModule resolution algorithm on Android now supports C++ TurboModules.
2. `SampleTurboCxxModule` is moved from `ReactCommon/turbomodule/samples/platform/ios/` to `ReactCommon/turbomodule/samples` so that both iOS and Android can share it.
3. `CatalystTurboModuleManagerDelegate::getTurboModule(std::string, JSCallInvoker)` now understands and returns `SampleTurboCxxModule`.

Reviewed By: mdvacca

Differential Revision: D15253477

fbshipit-source-id: 3def91911b091f8cf93be17decd245a0499ed718
2019-05-22 13:16:13 -07:00
zhongwuzw a352ecfabd TM iOS: Remove retainArguments && do retain by us explicitly (#24849)
Summary:
This is a TODO, we met crash if we don't call `retainArguments` when return type is like `NSDictionary`, the reason is `getReturnValue` don't retain the return value, so we need to using `__bridge` to transfer ownership to OC type.
Also add `resolveBlock` and `rejectBlock` to `retainedObjectsForInvocation`.

cc. cpojer

## Changelog

[iOS] [Fixed] - Remove retainArguments && do retain by us explicitly
Pull Request resolved: https://github.com/facebook/react-native/pull/24849

Differential Revision: D15369209

Pulled By: fkgozali

fbshipit-source-id: 8431d03705d8476f38c8b5d29630489a545d373a
2019-05-15 21:50:40 -07:00
Kevin Gozali 8662d9d3b0 TM iOS: Added backward-compatible ObjC invalidation logic
Summary:
Some ObjC NativeModules conform to `RCTInvalidating` protocol and implements `invalidate` method. This is typically used to clean things up during bridge teardown or reload. In TurboModule system the invalidate method can be replaced by pure destructors, but for backward compatibility, we call existing invalidate method on each module during teardown.

This also cleans up all existing LongLivedObject's.

Reviewed By: mdvacca, RSNara

Differential Revision: D15365655

fbshipit-source-id: 802844b39b5b7adb54970ea541f4d744bbf9e480
2019-05-15 17:36:05 -07:00
RCiesielczuk cd2f8c567b Introduce TurboModule Setup Metric (#24732)
Summary:
With the introduction of TurboModules, it would be beneficial to measure the setup time of these modules, as we currently have it in place for NativeModules.

The instantiation of the TMs occurs in the `RCTTurboModuleManager`. In order to successfully measure the time it took to setup the module, we need to ensure that we don't take into account cached modules. As such, we need to:

1. Check if module is in `_turboModuleCache`
  a. Start mark for `RCTPLTurboModuleSetup` tag if not found
2. Get the TM via `[self provideTurboModule:]`
3. Check if module is in `_turboModuleCache`
   a. Stop mark for `RCTPLTurboModuleSetup` if we did not find module in cache prior to **step 2** and if it's now present in the cache.
   b. Notify about setup time if the above is true.
4. Return TM

## Changelog

[iOS] [Added] - Gain insights on the the turbo module setup times by observing `RCTDidSetupModuleNotification`. The userInfo dictionary will contain the module name and setup time in milliseconds. These values can be extracted via `RCTDidSetupModuleNotificationModuleNameKey` and `RCTDidSetupModuleNotificationSetupTimeKey`.
Pull Request resolved: https://github.com/facebook/react-native/pull/24732

Differential Revision: D15362088

Pulled By: RSNara

fbshipit-source-id: e6a8044e4aba5a12ae63e9c7dbf707a17ec00180
2019-05-15 15:28:10 -07:00