Commit Graph

3700 Commits

Author SHA1 Message Date
Oleksandr Melnykov bdb4278523 Fix Switch measurement on Android
Summary:
Fabric expects the measure method to return the size in density-independent pixels, but getMeasuredWidth and getMeasuredHeight return pixels on Android, so we have to convert these values before returning to C++.

Check the method createUpdateLayoutMountItem in Binding.cpp:

```
local_ref<JMountItem::javaobject> createUpdateLayoutMountItem(
    const jni::global_ref<jobject> &javaUIManager,
    const ShadowViewMutation &mutation) {
     ...

    int x = round(frame.origin.x * pointScaleFactor);
    int y = round(frame.origin.y * pointScaleFactor);
    int w = round(frame.size.width * pointScaleFactor);
    int h = round(frame.size.height * pointScaleFactor);
    auto layoutDirection = toInt(newChildShadowView.layoutMetrics.layoutDirection);
    return updateLayoutInstruction(
        javaUIManager, newChildShadowView.tag, x, y, w, h, layoutDirection);
  }

  return nullptr;
}
```

We are interested in the next two lines:
```
int w = round(frame.size.width * pointScaleFactor);
int h = round(frame.size.height * pointScaleFactor);
```

`frame.size.width` and `frame.size.height` are the values returned from the measure method in Java and they are multiplied by the screen density to get the size in pixels, which means Fabric expects these values to be DIPs.

Reviewed By: shergin

Differential Revision: D17626834

fbshipit-source-id: f9856b5d0796c75c26c84adf11e1652b22a1ddef
2019-10-03 03:15:21 -07:00
Oleksandr Melnykov 299c984964 Pass surface ID to measure function in Java to retrieve themed Context
Summary:
We use `ViewManager.onMeasure` to perform measurements of Android views and pass the measured size back to Yoga. For Android in order to the report correct dimensions of a View, this View must be created using a Context that has a theme associated with it. Before, `onMeasure` only had ReactApplicationContext passed as the first parameter and ReactSwitch, for example, could not be measured correctly (because it uses the size of the thumb drawable, which is extracted from the current theme). This diff adds surfaceId as the first parameter of `FabricUIManager.measure`, so that we can retrieve ThemedReactContext and pass it to `ViewManager.onMeasure`.

The size of the Switch component is still incorrect, but at least the size reported back to Yoga is the same as in Paper. So there is more investigation necessary why this happens in Fabric. I will investigate and publish another diff with the fix.

Reviewed By: JoshuaGross, shergin

Differential Revision: D17625959

fbshipit-source-id: 48197a61240fb13042bef3e9f5d681acacc702fb
2019-10-03 03:15:20 -07:00
Oleksandr Melnykov d0dd1aed29 Integrate AndroidSwitch into Fabric on Android
Summary:
In this diff we integrate the Switch component on Android in Fabric. Since the component has a custom measure function, we need to write some C++ to call the measure method in Java.

The component isn't fully functional yet (setNativeProps isn't supported in Fabric) and has some problems with measuring itself. I will fix the component in the next diffs in this stack.

Reviewed By: JoshuaGross

Differential Revision: D17571258

fbshipit-source-id: be4e201495b9b197ddec44ee3484357bfb6225a8
2019-10-03 03:15:20 -07:00
Elisa Lou 8dcb6ee91f android: update dark mode minimum (#26623)
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/26623

I noticed the min version was 10. It is actually 9 if we consider users with developer option turned on.

Changelog:
[Android][Fixed] - Updated Appearance module to allow Android 9/P devices.

Differential Revision: D17632656

fbshipit-source-id: 893699ae37ab1cef64fe2547e0f2d6858bf3c48c
2019-10-01 13:33:39 -07:00
Oleksandr Melnykov 769d51824e Fix setting component padding in Fabric
Summary: This diff fixes the issue with view paddings in Fabric introduced in D17081799. In Paper setting padding on a view was only supported for Text and TextInput components (see https://fburl.com/codesearch/6r6lu5vd). We want to keep Fabric backwards compatible, so we delegate setting the padding to the view manager instead of setting it directly on the view. In this way we can have a no-op implementation in the base view manager and implement this method only in view managers that support setting padding (ReactTextInputManager and ReactTextViewManager at the moment).

Reviewed By: JoshuaGross

Differential Revision: D17665011

fbshipit-source-id: 38bc56278e002bd34881cfcb9ed79df579f79e28
2019-10-01 11:38:21 -07:00
Oleksandr Melnykov b21a941923 Fix order of method execution in Fabric ViewManager
Summary:
Because of the changes made in Fabric, the order of the execution of some methods in ViewManager has changed. In Paper the following order was guaranteed: `createViewInstance -> addEventEmitters -> updateProperties -> onAfterUpdateTransaction`. But in Fabric, the order is the following: `createViewInstance -> updateProperties -> onAfterUpdateTransaction -> addEventEmitters`. This change can actually break some existing view managers, because they rely on the fact that `addEventEmitters` will be called before `onAfterUpdateTransaction`. Check ReactVideoManager:

```
ReactModule(name = ReactVideoManager.REACT_CLASS)
public class ReactVideoManager extends SimpleViewManager<ReactVideoPlayer>
    implements VideoManagerInterface<ReactVideoPlayer> {
...

  Override
  protected void onAfterUpdateTransaction(ReactVideoPlayer view) {
    super.onAfterUpdateTransaction(view);
    view.commitChanges();
  }

 Override
  protected void addEventEmitters(
      final ThemedReactContext reactContext, final ReactVideoPlayer view) {
    view.setStateChangedListener(
        new ReactVideoPlayer.PlayerStateChangedListener() {
...
}
```

As you can see there is a state change listener registered in `addEventEmitters` and `view.commitChanges()` can actually cause a state change. It means that if `onAfterUpdateTransaction` is executed before `addEventEmitters`, the state change listener will be added after a state change has happened and the event will be missed.

Reviewed By: JoshuaGross

Differential Revision: D17600308

fbshipit-source-id: 044e09e0d64973c8237876311d37c057a1ba384e
2019-10-01 03:56:39 -07:00
Oleksandr Melnykov 36b7febe8f Integrate Video into Fabric on Android
Summary: This diff makes the Video component compatible with Fabric on Android.

Reviewed By: JoshuaGross

Differential Revision: D17450815

fbshipit-source-id: 5e47d8cdca96f9fbe72902bac7dd157f7b5fdb1a
2019-10-01 03:56:39 -07:00
Emily Janzer 774502510b Convert SourceCode module to TurboModule
Summary: Converting the SourceCode native module to TurboModules. Checking in the Java class generated from the JS spec and extending it in the module implementation.

Reviewed By: fkgozali

Differential Revision: D17586276

fbshipit-source-id: 3d2080f1280791e81a0366d0aab101d960d11157
2019-09-30 18:03:37 -07:00
Rick Hanlon 2d95668aa8 Add fastRefresh to NativeDevSettings
Summary: This diff adds a method to call whenever a fast refresh happens. Right now this is only useful for reporting.

Reviewed By: cpojer

Differential Revision: D17528033

fbshipit-source-id: 17e82abe7a3e2bab6829de5adecda853fe5335c5
2019-09-30 07:03:51 -07:00
Rick Hanlon 549cac63cb Add reloadWithReason to DevSettings
Summary:
This diff adds reloadWithReason to the NativeDevSettings and updates the exposed DevSettings.reload method to send to it if it's available (setting an 'uncategorized' reason if one isn't set.

[General][Feature] Update DevSettings.reload to accept a reason

Reviewed By: RSNara

Differential Revision: D17499343

fbshipit-source-id: e8c9724800f93d3b9a5d2a8fe9f689d51947d39b
2019-09-30 07:03:50 -07:00
Oleksandr Melnykov 07fe994e26 Fix Slider height in Fabric
Summary: This diff fixes the height of the Slider component in Fabric on Android. Note that the layout is still broken (no padding and the seek bar is misaligned, but it's another issue).

Reviewed By: JoshuaGross

Differential Revision: D17629798

fbshipit-source-id: af8cae909279dc92ee1c80b9be2f5c578972eafc
2019-09-30 03:51:59 -07:00
masaaki1915 231d2f95cd fix typo in comment about package (#26638)
Summary:
This PR only fixes a typo in `ModuleHolder.java`.

## Changelog

[Internal] [Fixed] - Fix typo in comment about package
Pull Request resolved: https://github.com/facebook/react-native/pull/26638

Differential Revision: D17661204

Pulled By: cpojer

fbshipit-source-id: 77ab92b7bfff300961e0a32183188a6e57e99b3d
2019-09-29 20:49:51 -07:00
Marc Mulcahy c7aa6dc827 Add onSlidingComplete callbacks when sliders adjusted via a11y (#26600)
Summary:
When sliders are adjusted via accessibility, no onSlidingComplete callback is
generated. This causes problems for components which perform behavior in this
callback, and means that such components don't behave properly when adjusted via
accessibility. For example, if an app hosting a volume control slider only commits the volume change to the hardware on onSlidingComplete, it is impossible for a screen reader user to ever actually adjust the volume.

Ensure that sliders call the onSlidingComplete callback after adjusted via
accessibility.

## Changelog

[General] [Fix] - Add onSlidingComplete callbacks when sliders adjusted via a11y.

[CATEGORY] [TYPE] - Message
Pull Request resolved: https://github.com/facebook/react-native/pull/26600

Test Plan: Prior to this change, using the RNTester slider example with a screen reader, the onSlidingComplete callback tests never shows any callbacks when the slider is adjusted. With this change applied, the callback test will show a number of callbacks corresponding to the number of times the slider was adjusted via the screen reader.

Differential Revision: D17661157

Pulled By: cpojer

fbshipit-source-id: a6eedef099c6c1b571b290c329059ac9b69b53dd
2019-09-29 19:45:06 -07:00
Vojtech Novak 113c4e229c improve error message in NativeModuleRegistryBuilder.java (#26467)
Summary:
## Motivation

I have seen a spike in users reporting this error. Unfortunately I did not receive any repros that would confirm this, but my hypothesis is that they ran into situation when `new XYZPackage()` was present in `getPackages()` method and then the CLI kicked in with autolinking and they were left with this incomplete error.

someone more knowledgeable of autolinking should review this.
Pull Request resolved: https://github.com/facebook/react-native/pull/26467

Differential Revision: D17661242

Pulled By: cpojer

fbshipit-source-id: 63dfcd85a0d41d85a0dd52f84ab16cb7ceb64ba2
2019-09-29 19:42:08 -07:00
Petter Hesselberg 4b9350061f Petterh/support parcelable array args (#26379)
Summary:
`ReactRootView.startReactApplication` takes a `Bundle` argument called `initialProperties`. This is translated to a `ReadableMap` via `Arguments.fromBundle`. If the bundle contains an array of bundles, this gets translated by `Arguments.fromArray`.

If the bundle was passed from one activity to another via intent extras, however, there is a problem. After the bundle has been marshaled and unmarshaled, the array of `Bundle`s come out the other end as an arrap of `Parcelable`s, although each array element remains a `Bundle`. This results in an "Unknown array type" exception.

This PR fixes this by adding support for `Parcelable` arrays &ndash; provided they contain only members of type `Bundle`.

## Changelog

[Android] [Fixed] - Don't throw "Unknown array type" exception when passing serialized bundle arrays in ReactRootView.startReactApplication's initialProperties parameter
Pull Request resolved: https://github.com/facebook/react-native/pull/26379

Test Plan: Added test class `ArgumentsTest`. The test method `testFromMarshaledBundle` fails when used on the old version of `Arguments`.

Differential Revision: D17661203

Pulled By: cpojer

fbshipit-source-id: 63612d78f49bdf9cc53f6f21ae883dba6cebce84
2019-09-29 19:22:22 -07:00
Ramanpreet Nara 42dcfab2a9 Stop requiring TurboModuleManager JSIModule on CatalystInstance cleanup
Summary:
In `CatalystInstanceImpl.destroy()`, we require the TurboModuleManager using the [following lines](https://fburl.com/diffusion/a4y6wbft):

```
final JSIModule turboModuleManager =
    ReactFeatureFlags.useTurboModules
        ? mJSIModuleRegistry.getModule(JSIModuleType.TurboModuleManager)
        : null;
```

For some strange reason, even though `ReactFeatureFlags.useTurboModules` is true, the TurboModuleManager isn't registered with mJSIModuleRegistry. I spent some time looking through the code, but I couldn't figure out why. These lines actually aren't necessary, so it's possible to fix the issue by simply working around it, which is what this diff does. We shouldn't have been double requiring the TurboModuleManager anyways, since `CatalystInstance.java` has a method to set the TurboModuleManager, which we call in `ReactInstanceManager.createReactContext`.

## Alternative approach
I could push this diff to the next cut, and instead land a diff that adds debug information to the native crash. At the cost of a week, it may help us figure out why we're seeing the crash. Thoughts? cc fkgozali

Reviewed By: fkgozali

Differential Revision: D17636604

fbshipit-source-id: ecfff593dc6eb4ec4d5e331348b308bc7ab37966
2019-09-27 18:20:28 -07:00
Sidharth Guglani 8583d85de1 Add boolean flag to decide whether to use fbjni or jni
Summary:
Adds a flag useVanillaJNI in YogaConfig to determine whether to use FbJNI or JNI.
Currently default is set to false.

We will experiment based on this flag once all code have been migrated.

Reviewed By: Andrey-Mishanin

Differential Revision: D17601703

fbshipit-source-id: 377a5bd2a6f8a7584e84932e87fa7044d8165efd
2019-09-26 17:32:32 -07:00
Sidharth Guglani 34b71e4fb4 Add separate classes to implement JNI methods suing vanilla JNI
Summary:
This diffs adds a separate file YGJNIVanilla.cpp to add jni methods which uses vanilla JNI instead of FBJNI.
In this diff only one method has been added to setup the experiment boolean setup.

At the end of this diff stack , we will be able to experiment between fbjni and vanilla jni in yoga and finally get rid of fbjni which saves us around 300Kb per architecture in yoga binary size.

Reviewed By: Andrey-Mishanin

Differential Revision: D17601591

fbshipit-source-id: a88520c625bd8b5d9ffcf8ab5f02fc71dc800081
2019-09-26 17:32:32 -07:00
Rajdeep Kaur d21f695edf Limit size of video uploaded from camera roll in android
Summary: Details in Task T53266042. AMA users are trying to upload video data of more than 300 MB which is causing spikes of server_err in the web tier. So i added check to retrive videos that have size < 100 MB.

Reviewed By: furdei

Differential Revision: D17544308

fbshipit-source-id: 5a1d1329b6b12656f1617bb8775e303c96d529cb
2019-09-26 09:47:17 -07:00
Ramanpreet Nara 5e68a98c3d Make TurboModuleProviderFunctionType not depend on TurboModuleManager instance
Summary:
## Description
The C++ lambda that JS invokes to create TurboModules uses `TurboModuleManager`. It is possible for this lambda to outlive the `TurboModuleManager` instance because we delete `TurboModuleManager` on the JS Thread before we schedule the deletion of `CatalystInstanceImpl` object on a neutral third-party background thread. `CatalystInstanceImpl` owns the JS VM instance that owns the C++ lambda.

## [CatalystInstanceImpl.java](https://fburl.com/diffusion/vt4pwjwa)
```
public void destroy() {
  // ...

  getReactQueueConfiguration()
    .getJSQueueThread()
    .runOnQueue(
        new Runnable() {
          Override
          public void run() {
            // We need to destroy the TurboModuleManager on the JS Thread
            if (turboModuleManager != null) {
              turboModuleManager.onCatalystInstanceDestroy();
            }

            getReactQueueConfiguration()
                .getUIQueueThread()
                .runOnQueue(
                    new Runnable() {
                      Override
                      public void run() {
                        // AsyncTask.execute must be executed from the UI Thread
                        AsyncTask.execute(
                            new Runnable() {
                              Override
                              public void run() {
                                // Kill non-UI threads from neutral third party
                                // potentially expensive, so don't run on UI thread

                                // contextHolder is used as a lock to guard against
                                // other users of the JS VM having the VM destroyed
                                // underneath them, so notify them before we reset
                                // Native
                                mJavaScriptContextHolder.clear();

                                mHybridData.resetNative();
                                getReactQueueConfiguration().destroy();
                                Log.d(
                                    ReactConstants.TAG,
                                    "CatalystInstanceImpl.destroy() end");
                                ReactMarker.logMarker(
                                    ReactMarkerConstants.DESTROY_CATALYST_INSTANCE_END);
                              }
                            });
                      }
                    });
          }
        });
    }
  });

  // ...
}
```

The JS thread is also terminated in the neutral third-party thread. Therefore, it should be possible for JS to request a `TurboModule` after `TurboModuleManager` has been destroyed (i.e: JS can try to access memory that was freed). This is why I think we're getting a segfault in T54298358.

## Fix
The fix was to wrap all the member variables of TurboModuleManager we use in `TurboModuleProviderFunctionType` in weak references. This way, we can make sure that the memory is valid before using it.

Reviewed By: fkgozali

Differential Revision: D17539761

fbshipit-source-id: fe527383458a019a4cb9107ec5c3ddd6295ae41c
2019-09-26 09:34:41 -07:00
Andrea Cimitan 8d8c3d4e1e Also listen to NFC actions for linking url events (#26553)
Summary:
This PR solves bug https://github.com/facebook/react-native/issues/26552 for Android. Allows an app to receive url events through Linking from NFC tags

## Changelog
[Android] [Fixed] - This branch checks also for `ACTION_NDEF_DISCOVERED` intent matches to send the url events
Pull Request resolved: https://github.com/facebook/react-native/pull/26553

Test Plan: Tested the code multiple times with both NFC tags and normal links

Differential Revision: D17589654

Pulled By: cpojer

fbshipit-source-id: 55e854e765a84da5e22ec2cc51d0fe0972254175
2019-09-25 18:44:43 -07:00
Uts Sikder 94e8ccf9c0 BREAKING: rm YogaNode parameter from YogaLogger#log
Summary:
In D17439957, I noted that YogaLogger#log throws a NoMethodFoundException when called from C++ b/c C++ and Java's signatures of that method don't match. C++ uses YogaNodeJNIBase for the first param, Java uses YogaNode. Both my attempts to fix this failed.

Attempt #1 - Make Java use YogaNodeJNIBase. This doesn't work because the :java-interface target includes YogaLogger but not YogaNodeJNIBase. Moving YogaLogger to the impl target doesn't work either b/c other files in :java-interface reference YogaLogger.

Attempt #2 - Make C++ use YogaNode. This doesn't work b/c we try to call the log method with objects of fbjni type YogaNodeJNIBase. This would be fine in Java since YogaNodeJNIBase extends YogaNode. But fbjni's typing isn't advanced enough to know this, so the Yoga C++ fails to compile.

At this point, I was wondering what the value of having this param in the log function at all was. None of the implementations in our codebase use it today. It might be easier to just remove it all together. This also removes a bug with YGNodePrint where we pass a null layout context that eventually causes a SIG_ABRT when we use it to try to find a YogaNode to pass to this function. (https://fburl.com/diffusion/ssw9h8lv).

Reviewed By: amir-shalem

Differential Revision: D17470379

fbshipit-source-id: 8fc2d95505971a52af2399a9fbb60b63f27f0ec2
2019-09-25 09:12:43 -07:00
Emily Janzer 6464ef0a92 Add @DoNotStrip to JavaScriptModule interfaces
Summary: Adding `DoNotStrip` to all the interfaces that extend `JavaScriptModule` to ensure they don't get stripped from release builds (because they have no Java implementors).

Reviewed By: emma0303

Differential Revision: D17534719

fbshipit-source-id: a793764caf17040bf1252be7ec4c72176d6989d4
2019-09-24 09:36:16 -07:00
Emily Janzer 990c9ea5ec Remove bridge access from JavaTimerManager, again
Summary: Another attempt at D17282188, which got partially reverted in D17505827 due to a crash in release builds.

Reviewed By: RSNara

Differential Revision: D17512419

fbshipit-source-id: a1b0abfed2c4a1f3f02da85e84abee0127b1a7e2
2019-09-23 19:14:43 -07:00
Janic Duplessis 211ea485cd Fix includeFontPadding for TextInput placeholder (#26432)
Summary:
The custom font I'm using requires using `includeFontPadding={false}` to be correctly centered vertically. The only case where this is not working is with the placeholder of `TextInput`. To fix it we call `setIncludeFontPadding` on the `EditText` instance, like we do for `Text`.

## Changelog

[Android] [Fixed] - Fix `includeFontPadding` for `TextInput` placeholder
Pull Request resolved: https://github.com/facebook/react-native/pull/26432

Test Plan:
Tested the fix in an app.

Before

![image](https://user-images.githubusercontent.com/2677334/64898120-f1de0600-d653-11e9-97b3-f53416d5f9fe.png)

After

![image](https://user-images.githubusercontent.com/2677334/64897961-5b114980-d653-11e9-8897-baa14fc0f56c.png)

Reviewed By: mdvacca, mmmulani

Differential Revision: D17468767

Pulled By: JoshuaGross

fbshipit-source-id: ae29debf9a57198a636a24ec8ed9ba3d77f0a73e
2019-09-23 10:28:05 -07:00
Janic Duplessis 8b9f790069 Fix medium font weights for TextInput on Android (#26434)
Summary:
When using a medium (500) font weight on Android the wrong weight is used for the placeholder and for the first few seconds of input (before it gets text back from JS). To fix it I refactored the way we handle text styles (family, weight, style) to create a typeface to be more like the `Text` component.

Since all these 3 props are linked and used to create the typeface object it makes more sense to do it at the end of setting props instead of in each prop handler and trying to recreate the object without losing styles set by other prop handlers. Do do that we now store fontFamily, fontStyle and fontWeight as ivar of the ReactEditText class. At the end of updating prop if any of those changed we recreate the typeface object.

This doesn't actually fix the bug but was a first step towards it. There were a bunch of TODOs in the code to remove duplication between `Text` and `TextInput` for parsing and creating the typeface object. To do that I simply moved the code to util functions in a static class. Once the duplication was removed the bug was fixed! I assume proper support for medium font weights was added for `Text` but not in the duplicated code for `TextInput`.

## Changelog

[Android] [Fixed] - Fix medium font weights for TextInput on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/26434

Test Plan:
Tested in my app and in RNTester that custom styles for both text and textinput all seem to work.

Repro in RNTester:

```js
function Bug() {
  const [value, setValue] = React.useState('');
  return (
    <TextInput
      style={[
        styles.singleLine,
        {fontFamily: 'sans-serif', fontWeight: '500', fontSize: 32},
      ]}
      placeholder="Sans-Serif 500"
      value={value}
      onChangeText={setValue}
    />
  );
}
```

Before:

![font-bug-1](https://user-images.githubusercontent.com/2677334/64902280-6889fc00-d672-11e9-8f44-9e524d844a6c.gif)

After:

![font-bug-2](https://user-images.githubusercontent.com/2677334/64902282-6cb61980-d672-11e9-8163-ace0f23070b6.gif)

Reviewed By: mmmulani

Differential Revision: D17468825

Pulled By: JoshuaGross

fbshipit-source-id: bc2219facb94668551a06a68b0ee4690e5474d40
2019-09-23 10:23:29 -07:00
Oleksandr Melnykov 5cfe588993 Use generated Java delegate for setting properties on ReactSwitchManager
Summary: This diff migrates `ReactSwtichManager` to use the generated `ReactSwtichManagerDelegate` for setting its properties.

Reviewed By: TheSavior

Differential Revision: D17395067

fbshipit-source-id: 1489c5d08cef860030ecbd23ef19bd8de1328d71
2019-09-23 07:18:10 -07:00
Oleksandr Melnykov 81f567d4aa Use generated Java delegate for setting properties on ReactDrawerLayoutManager
Summary: This diff migrates `ReactDrawerLayoutManager` to use the generated `AndroidDrawerLayoutManagerDelegate` for setting its properties.

Reviewed By: mdvacca

Differential Revision: D17343383

fbshipit-source-id: 85cd7ee3531b152da2601048f5e458f5dad73ad6
2019-09-23 07:18:10 -07:00
Oleksandr Melnykov 2e7545cf7e Use generated Java delegate for setting properties on ReactProgressBarViewManager
Summary: This diff migrates `ReactProgressBarViewManager` to use the generated `AndroidProgressBarManagerDelegate` for setting its properties.

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D17315619

fbshipit-source-id: 6293c6fc18567a934b6f3dce9b77abcc408052d8
2019-09-23 07:18:09 -07:00
Oleksandr Melnykov 23557d1f9a Flow type AndroidSwitch and generate Android OSS classes
Summary: This diff adds Flow types to `AndroidSwitchNativeComponent`.

Reviewed By: TheSavior

Differential Revision: D17205083

fbshipit-source-id: 3d11f11e269388b78a5f0ed528be94df04f9719d
2019-09-23 07:18:09 -07:00
Oleksandr Melnykov ef3b16ef6d Use generated Java delegate for setting properties on SwipeRefreshLayoutManager
Summary: This diff migrates `SwipeRefreshLayoutManager` to use the generated `AndroidSwipeRefreshLayoutManagerDelegate`.

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D17225894

fbshipit-source-id: e659d2a9cb5dba42c589559f61a0e98330e21612
2019-09-23 07:18:08 -07:00
Oleksandr Melnykov 1eb8ef59ad Use generated Java delegate for setting properties on ReactModalHostManager
Summary: This diff migrates `ReactModalHostManager` to use the generated `ModalHostViewManagerDelegate` for setting its properties.

Reviewed By: mdvacca

Differential Revision: D17205817

fbshipit-source-id: 6724302fd4301f9df92df04fcfb41f0c2c939d9f
2019-09-23 07:18:08 -07:00
Oleksandr Melnykov 5925b3d408 Use generated Java delegate for setting properties on ReactSliderManager
Summary: This diff migrates `ReactSliderManager` to use the generated `SliderManagerDelegate` for setting its properties.

Reviewed By: mdvacca

Differential Revision: D17203078

fbshipit-source-id: 726736ef275074ecb799b334342ac64976153e2b
2019-09-23 07:18:07 -07:00
Amir Shalem c205b1d391 Use direct access to YogaConfig mNativePointer parameter
Summary:
Use direct access to YogaConfig mNativePointer parameter

Results:
```
The following primary metrics showed statistically significant changes at the 95% confidence level:
javaFullLifecycleAllocateCalculateReadLayout	-1.25%
javaLayoutReading	0.44%
javaYogaNodeAllocateAndSetProps	-1.92%
javaYogaNodeAllocation	-2.11%
javaYogaNodeStylePropAssignment	-0.89%
```

Differential Revision: D17519542

fbshipit-source-id: c39bfe1b0ecae9149dc6da2a0a7e936df215ec5b
2019-09-22 13:48:52 -07:00
Emily Janzer ed6a3a6c74 Remove timer proxy
Summary: Fix for crash introduced by D17282188; doesn't cleanly revert, so just reverting this part

Reviewed By: JoshuaGross, RSNara

Differential Revision: D17505827

fbshipit-source-id: 3232285c0f15dabeb819f8806ad35d4ec83a1e53
2019-09-20 13:55:45 -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 10d89b1eff Make TurboModuleManager accept a CallInvoker for the NativeModules thread
Summary:
In the previous diffs, I:
1. D17422165: Created a `CallInvoker` for the NativeModules thread.
2. D17422164: Exposed this `CallInvoker` via `CatalystInstance.getNativeCallInvokerHolder()`.

In this diff, I:
1. Make TurboModuleManager accept the NativeModules thread `CallInvoker` as a constructor argument.

Reviewed By: PeteTheHeat

Differential Revision: D17422163

fbshipit-source-id: cbaac2fc06f7f726d89e0c545154123ad7410e62
2019-09-20 10:52:57 -07:00
Ramanpreet Nara 77fe4f087d Introduce CatalystInstance.getNativeCallInvokerHolder()
Summary: CatalystInstanceImpl is responsible for creating the NativeModules thread. We therefore expose a method `getNativeCallInvokerHolder()` on this hybrid class to create and give us access to the `CallInvokerHolder` for the NativeModules thread.

Reviewed By: PeteTheHeat

Differential Revision: D17422164

fbshipit-source-id: 316423847518124115643549fa73a8533d493cd0
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
Dulmandakh 60b57bad54 cleanup DrawerLayoutAndroid (#26497)
Summary:
This PR removes Java reflection use in ReactDrawerLayoutManager.java because we all use AndroidX and setDrawerElevation is available. Also adds NonNull annotations

## Changelog

[Android] [Changed] - cleanup DrawerLayoutAndroid
Pull Request resolved: https://github.com/facebook/react-native/pull/26497

Test Plan: RNTester is working as expected.

Differential Revision: D17488727

Pulled By: mdvacca

fbshipit-source-id: fd626e3e7aca3965f62c4c82a55c69e81facc61d
2019-09-19 23:42:02 -07:00
Dulmandakh 70b735c04a extract and reuse ANDROIDX_TEST_VERSION (#26487)
Summary:
This PR is extracts and reuses ANDROIDX_TEST_VERSION, and is part of Gradle script refactoring effort.

## Changelog

[Android] [Changed] - extract and reuse ANDROIDX_TEST_VERSION
Pull Request resolved: https://github.com/facebook/react-native/pull/26487

Differential Revision: D17488766

Pulled By: mdvacca

fbshipit-source-id: f1968ffc403074d78d792eb5cc773cc6366ad2d1
2019-09-19 15:35:29 -07:00
Emily Janzer f054928124 Split up createTimer into two methods, createTimer and createAndMaybeCallTimer
Summary:
This diff splits up the current `createTimer` method (which is used for setTimeout, setInterval, etc.) into two methods, `createTimer` and `createAndMaybeCallTimer`. The latter is what's used by the existing Timing native module, and it preserves the existing behavior of this function.

What's the difference? The current implementation of createTimer makes some assumptions about how it's called - namely, that it's called from JS asynchronously (using the bridge). Right now when you create a timer from JS, the JSTimers module passes in the timestamp for when the timer was created; in the native `createTimer`, we compare this timestamp to the current time, and if we find the timer has already expired, we immediately invoke the callback.

Presumably this is done because we don't know how much time has elapsed since when the timer was scheduled in JS, and we want to make sure that it's called as soon as possible. Of course, this also means that `setTimeout(0)` will be immediately invoked, too, without waiting for the next frame.

This all sounds fine, until we take a look at immediates. Immediates are currently implemented entirely in JS, and are called by the JS bridge; before returning control to native, we flush the immediates queue.

This means that the current behavior is: 1) `setImmediate()` is always invoked before `setTimeout(0)`; 2) `setTimeout(0)` is invoked as soon as possible, before the next frame.

However, this changes with bridgeless RN. With bridgeless RN, the native module methods are being replaced by C++ host functions, which are called synchronously. So if we keep the current logic in JavaTimerManager (where it checks if the timer has already expired), then `setTimeout(0)` will be invoked **before** immediates are called.

So the change that I'm making for bridgeless RN is to always wait until the next frame before calling timers. This preserves the order of immediates/timers, although it does mean that `setTimeout(0)` will no longer be called as soon as possible. Of the two options, this seems preferable.

Reviewed By: PeteTheHeat

Differential Revision: D17403144

fbshipit-source-id: 8230f6ebe56aa20bfcf2325177c7812bc8e9c2ec
2019-09-19 15:16:56 -07:00
Krzysztof Magiera 9f0dede1c9 Allow again for injecting custom root view via ReactActivityDelegate (#26495)
Summary:
This change restores the possibility of injecting custom root views via ReactAcitivtyDelegate. It has been used by react-native-gesture-handler library in order to replace default root view with a one that'd route touch events to gesture-handler internal pipeline.

The regression happened in d0792d4b8a where new `ReactDelegate` was introduced to provide support for rendering react native views in both Android fragments and activities. As a part of that change the logic responsible for creating root view has been moved from `ReactActivityDelegate` to `ReactDelegate` rendering `ReactActivityDelegate.createRootView` unused – that is there is no code path that leads to this method being called. Instead `ReactDelegate.createRootView` method has been added which now plays the same role. The custom root view injection was relying on overwriting that method and hence the method is no longer referenced overwriting it has no effect. Following the logic migration out of `ReactActivityDelegate` into `ReactDelegate` we could potentially now start overwriting methods of `ReactDelegate`. However when working with Android's activities in React Native we can only provide an instance of `ReactActivityDelegate` and in my opinion it does not make too much sense to expose also a way to provide own instance of `ReactDelegate`.

The proposed fix was to route `ReactDelegate.createRootView` to call `ReactActivityDelegate.createRootView` and this way regaining control over root view creation to `ReactActivityDelgate`. The change of the behavior has been implemented by subclassing `ReactDelegate` directly from `ReactActivityDelegate` and hooking the aforementioned methods together. Thanks to this approach, the change has no effect on `ReactDelegate` being used directly from fragments or in other scenarios where it is not being instantiated from `ReactActivityDelegate`.

This fixes an issue reported in https://github.com/kmagiera/react-native-gesture-handler/issues/745 and discussed on 0.61 release thread: https://github.com/react-native-community/releases/issues/140#issuecomment-532235945

## Changelog

[Internal] [Fixed] - Allow for custom root view to be injected via ReactActivityDelegate
Pull Request resolved: https://github.com/facebook/react-native/pull/26495

Test Plan:
1. Run RNTester, take layout snapshot, see the react root view being on the top of view hierarchy.
2. Run gesture-handler Example app (or some other app that overwrites ReactActivityDelegate.createRootView method), on layout snapshot see custom root view being used.

Differential Revision: D17482966

Pulled By: mdvacca

fbshipit-source-id: 866f551b8b077bafe1eb9e34e5dccb1240fa935e
2019-09-19 12:23:08 -07:00
Emily Janzer cc36b89b17 Remove bridge access from JavaTimerManager
Summary:
The next step to making our existing timers logic usable outside of the context of the native module. This diff removes the bridge access through ReactContext in JavaTimerManager, and instead relies on a delegate that implements the JSTimers interface. I'm reusing the JSTimers interface for convenience even though it extends JavaScriptModule, which I don't plan on using for bridgeless RN.

Also changing from ReadableArray to ReadableNativeArray so that it can be directly accessed from C++.

Reviewed By: PeteTheHeat

Differential Revision: D17282188

fbshipit-source-id: 5c5e0b12a2250334e96885c220feb52146e57c83
2019-09-19 11:47:18 -07:00
Emily Janzer 4d774bdc0d Moving timing logic out of TimingModule and into a new class
Summary:
Decoupling the logic for managing timers from the native module interface in TimingModule. I'm doing this so we can more easily share this logic with bridgeless RN, which won't use a native module for timers but instead will define the bindings with JSI.

This diff just moves things around and doesn't change any of the behavior.

Reviewed By: PeteTheHeat

Differential Revision: D17282187

fbshipit-source-id: ef54254dd0c7e2f6e294e9ae5a7d4b010f98de2e
2019-09-19 11:47:18 -07:00
Emily Janzer e9b50fa4ee Renaming Timing to TimingModule
Summary: Most of our other native modules end in 'module', so let's update Timing to match.

Reviewed By: RSNara

Differential Revision: D17260848

fbshipit-source-id: 808b4d370a7036a247724fda5ab7210ac985476b
2019-09-19 11:47:17 -07:00
Uts Sikder 9b246d6185 fix type mismatches in YogaLogger#log function
Summary: This diff fixes two issues with the JNI integration of YogaLogger#log. (1) The YogaLogger class descriptor was missing a semicolon. This causes a ClassNotFoundException whenever you try to call the log method from C++. (2) The C++ signature for the class was using YogaNodeJNIBase as an arg but the Java signature was using YogaNode. This causes a MethodNotFoundException whenever you try to call the method after fixing problem 1.

Reviewed By: astreet

Differential Revision: D17439957

fbshipit-source-id: be3c16512558050265565b3688fb09a7da31b9b2
2019-09-18 21:31:10 -07:00
David Vacca 2c87cf5594 Easy constant refactor in ReactEventEmitter
Summary:
This is a easy change in ReactEventEmitter that refactors the TAG used by this class to log events.
This is a follow up of D17422611

Reviewed By: makovkastar

Differential Revision: D17437034

fbshipit-source-id: 9974a244b24b04315c6a328696954b7acf7efbfd
2019-09-18 08:56:02 -07:00
Amir Shalem d898574fb2 Remove dead code of mYogaNodeCloneFunction
Summary: Code cleanup, remove mYogaNodeCloneFunction completely and its `YogaNodeCloneFunction`

Reviewed By: SidharthGuglani

Differential Revision: D17284191

fbshipit-source-id: 36dae0c0548cfdd3e85182a8e3c6ff9a2d3a5011
2019-09-18 05:53:21 -07:00
Janic Duplessis 9b2374b542 Release underlying resources when JS instance is GC'ed on Android try 2 (#26155)
Summary:
Reland https://github.com/facebook/react-native/pull/24767

The commit had to be reverted because it caused a crash when using remote debugging in chrome. This is normal since jsi is not available in that environment. The crash was caused by `jsContext.get()` being 0, then being dereferenced later in c++. We can simply skip initializing the blob collector in this case.

This also includes the fix from https://github.com/facebook/react-native/issues/25720 to fix a crash when using hermes.

## Changelog

[Android] [Fixed] - Release underlying resources when JS instance is GC'ed on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/26155

Test Plan:
Test using RN tester with jsc and hermes
Test remote debugging

Reviewed By: mdvacca, fred2028

Differential Revision: D17072644

Pulled By: makovkastar

fbshipit-source-id: 079d1d43501e854297fbbe586ba229920c892584
2019-09-18 04:52:43 -07:00