Commit Graph

4064 Commits

Author SHA1 Message Date
Ramanpreet Nara 61dda3242d Refactor: Delete ExceptionLogger interface in DevSupportManagerBase
Summary:
## Rationale
- ExceptionLogger [is a private interface](https://www.internalfb.com/code/fbsource/[0fa3b362177087a2b5c4544c9d65a01f7f09b0e7]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java?lines=256-258) is [only implemented by JSExceptionLogger](https://www.internalfb.com/code/search?q=repo%3Afbsource%20regex%3Aon%20implements%20ExceptionLogger&tab=all)
- You can only have one ExceptionLogger, which is [a JSExceptionLogger created in DevLoadingViewBase](https://www.internalfb.com/code/fbsource/[0fa3b362177087a2b5c4544c9d65a01f7f09b0e7]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java?lines=227).

So, we could remove this generalized ExceptionLogger interface and just use replace it with an instance method on DevSupportManagerBase. This makes DevSupportManagerBase easier to reason about.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D29007764

fbshipit-source-id: 357f0e2e07f381f43b4b5e0c5f1415f87af7fd88
2021-06-10 16:47:20 -07:00
Ramanpreet Nara 1d14df217e Refactor: Stop having DevSupportManager conform to PackagerCommandListener
Summary:
## Rationale
The public methods of PackagerCommandListener should not be a part of the public API of DevSupportManagerBase. It's cleaner to just create a new PackagerCommandListener object inline, when we need it. This way, there're are less methods to think about when reasoning about the public API of DevSupportManagerBase.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D29006835

fbshipit-source-id: 0eea86c22e7d676c94d29b240a1f39955d8216ab
2021-06-10 16:47:20 -07:00
Ramanpreet Nara 675e480fb9 Refactor: Make DevSupportManager not conform to DevInternalSettings.Listener
Summary:
## Rationale
This reduces the surface area of DevSupportManager's public API.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D29006836

fbshipit-source-id: 579364f6c556b8588f3b6bac79df3deb244a92e5
2021-06-10 16:47:20 -07:00
Ramanpreet Nara 39d9122445 Delete unnecessary DevSupportManager constructor
Summary:
## Rationale
This DevSupportManagerBase constructor isn't used anywhere, aside from VeniceDevSupportManager. Even in VeniceDevSupportManager, we should use the more specific constructor, because it informs readers of the class what specifically is being nullified in the base class. So, this diff gets rid of the smaller arity constructor.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D29004542

fbshipit-source-id: 74e20db93cf1871a364bf228dc089cd7d124297f
2021-06-10 16:47:19 -07:00
Ramanpreet Nara 7c6eb1fecb Move handleReloadJS() from DevSupportManagerBase to BridgeDevSupportManager
Summary:
## Rationale
DevSupportManagerBase.handleReloadJS() implements reloads for the bridge. Therefore, it's best to move this method to BridgeDevSupportManager.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D29004541

fbshipit-source-id: f77244e9c44cd442e7e0ab2845f78d699b143e66
2021-06-10 16:47:19 -07:00
Ramanpreet Nara 2c88459e24 Refactor: Introduce methods to show/hide DevLoadingView in DevSupportManagerBase
Summary:
## Rationale

Throughout DevSupportManagerBase, we show/hide the DevLoadingView and simultaneously write to the `mDevLoadingViewVisible` boolean. This diff pulls all those boolean writes into methods, so that subclasses of DevSupportManagerBase can show/hide the DevLoadingView without accessing the boolean directly.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D29004062

fbshipit-source-id: d54d79701e31f9ac8d1d1d44019199718fe1c8c0
2021-06-10 16:47:19 -07:00
Ramanpreet Nara 30340890dc Move loadSplitBundleFromServer from DevSupportManagerBase to BridgeDevSupportManager
Summary:
## Rationale
DevSupportManagerBase.loadSplitBundleFromServer() uses the bridge. So, it's best to move it to DevSupportManagerBase.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D29004064

fbshipit-source-id: 9acfb769d57a44ee4871e1997ff0950c3d66b757
2021-06-10 16:47:19 -07:00
Ramanpreet Nara 3feaecd473 Rename DevSupportManagerImpl to BridgeDevSupportManager
Summary:
## Rationale
See the description of D28978217.

Changelog: [Internal]

Reviewed By: sshic

Differential Revision: D28978218

fbshipit-source-id: 29b9269b3d0e25171abaf6b74cacd0439f5c8ab5
2021-06-10 16:47:19 -07:00
Joshua Gross f2e0b2f45f Fix crash in PreAllocation optimization
Summary:
PreAllocation currently always happens at revision 0 (after ShadowNode creation), and all CREATE mutations are triggered for ShadowNodes at revision 1 or higher (since CREATE mutations are generated by the differ, it means that all ShadowNodes have revision 1 or higher when CompleteRoot is called). This means that between PreAllocation and CREATE, we /always/ expect at least one clone.

It is possible for a node to be "non-view-forming" at revision 0, causing view preallocation to be skipped, and "view-forming" at revision 1 (causing the CREATE mutation to be thrown away, since all CREATE mutations of revision 0 or 1 are thrown away). This causes a crash. It is extremely marginal, but there are repros in the wild.

Thus, I'm introducing one new UIManager and Scheduler delegate method that allows the mounting layer to be notified of clones. If a clone from rev 0->1 results in a node going from non-view-forming to view-forming, we can preallocate then, as well.

This resolves this crash, and allows us to keep experimenting safely with this View PreAllocation optimization. I believe all edge-cases are accounted for.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D29043426

fbshipit-source-id: dff11d3140ded1cbb02a5518a3aeb52dc812cc50
2021-06-10 15:50:14 -07:00
Joshua Gross 25e8fbe8ff Ensure that EventEmitterWrapper is cleaned up as soon as a View is deleted
Summary:
In T92179998, there is a crash when an EventEmitterWrapper is deallocated after the JS VM is torn down, and the EventEmitterWrapper tries to free its JSI::Pointer reference (double-free).

To make sure this happens less/not at all, free EventEmitterWrappers on ViewState when a view is deleted, instead of waiting for all of RN to be torn down.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D29020768

fbshipit-source-id: 9d72a23bc9966992ef56c1e3ee523405d4333194
2021-06-09 23:48:04 -07:00
Maxime Bertheau ba387b91d3 Fix setSnapToOffsets crashing on Android if snapToOffsets is null (#31681)
Summary:
Fixes https://github.com/facebook/react-native/issues/30502.
This PR fixes a crash caused by a `NullPointerException` when updating the `snapToOffsets` property.
I noticed this crash on production, you'll find the stack trace below.

Cause: `snapToOffsets` is annotated `Nullable` but there are no nullability check before calling `snapToOffsets.size()`.

## Changelog

[Android] [Fixed] - Fixed a crash when updating `snapToOffsets` to a null value

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

Pull Request resolved: https://github.com/facebook/react-native/pull/31681

Test Plan:
Passing `snapToOffsets={undefined}` to a `FlatList` used to crash on Android. Not anymore.

## Stacktrace

```
com.facebook.react.bridge.JSApplicationIllegalArgumentException: Error while updating property 'snapToOffsets' of a view managed by: RCTScrollView
        at com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateViewProp(ViewManagersPropertyCache.java:102)
        at com.facebook.react.uimanager.ViewManagerPropertyUpdater$FallbackViewManagerSetter.setProperty(ViewManagerPropertyUpdater.java:136)
        at com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps(ViewManagerPropertyUpdater.java:56)
        at com.facebook.react.uimanager.ViewManager.updateProperties(ViewManager.java:49)
        at com.facebook.react.uimanager.NativeViewHierarchyManager.updateProperties(NativeViewHierarchyManager.java:143)
        at com.facebook.react.uimanager.UIViewOperationQueue$UpdatePropertiesOperation.execute(UIViewOperationQueue.java:93)
        at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:908)
        at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1019)
        at com.facebook.react.uimanager.UIViewOperationQueue.access$2600(UIViewOperationQueue.java:47)
        at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1079)
        at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
        at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
        at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1008)
        at android.view.Choreographer.doCallbacks(Choreographer.java:809)
        at android.view.Choreographer.doFrame(Choreographer.java:740)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:995)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:246)
        at android.app.ActivityThread.main(ActivityThread.java:8512)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:602)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1130)

Caused by: java.lang.reflect.InvocationTargetException
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateViewProp(ViewManagersPropertyCache.java:87)
        at com.facebook.react.uimanager.ViewManagerPropertyUpdater$FallbackViewManagerSetter.setProperty(ViewManagerPropertyUpdater.java:136)
        at com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps(ViewManagerPropertyUpdater.java:56)
        at com.facebook.react.uimanager.ViewManager.updateProperties(ViewManager.java:49)
        at com.facebook.react.uimanager.NativeViewHierarchyManager.updateProperties(NativeViewHierarchyManager.java:143)
        at com.facebook.react.uimanager.UIViewOperationQueue$UpdatePropertiesOperation.execute(UIViewOperationQueue.java:93)
        at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:908)
        at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1019)
        at com.facebook.react.uimanager.UIViewOperationQueue.access$2600(UIViewOperationQueue.java:47)
        at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1079)
        at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
        at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
        at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1008)
        at android.view.Choreographer.doCallbacks(Choreographer.java:809)
        at android.view.Choreographer.doFrame(Choreographer.java:740)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:995)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:246)
        at android.app.ActivityThread.main(ActivityThread.java:8512)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:602)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1130)

Caused by: java.lang.NullPointerException: Attempt to invoke interface method 'int com.facebook.react.bridge.ReadableArray.size()' on a null object reference
        at com.facebook.react.views.scroll.ReactScrollViewManager.setSnapToOffsets(ReactScrollViewManager.java:107)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.facebook.react.uimanager.ViewManagersPropertyCache$PropSetter.updateViewProp(ViewManagersPropertyCache.java:87)
        at com.facebook.react.uimanager.ViewManagerPropertyUpdater$FallbackViewManagerSetter.setProperty(ViewManagerPropertyUpdater.java:136)
        at com.facebook.react.uimanager.ViewManagerPropertyUpdater.updateProps(ViewManagerPropertyUpdater.java:56)
        at com.facebook.react.uimanager.ViewManager.updateProperties(ViewManager.java:49)
        at com.facebook.react.uimanager.NativeViewHierarchyManager.updateProperties(NativeViewHierarchyManager.java:143)
        at com.facebook.react.uimanager.UIViewOperationQueue$UpdatePropertiesOperation.execute(UIViewOperationQueue.java:93)
        at com.facebook.react.uimanager.UIViewOperationQueue$1.run(UIViewOperationQueue.java:908)
        at com.facebook.react.uimanager.UIViewOperationQueue.flushPendingBatches(UIViewOperationQueue.java:1019)
        at com.facebook.react.uimanager.UIViewOperationQueue.access$2600(UIViewOperationQueue.java:47)
        at com.facebook.react.uimanager.UIViewOperationQueue$DispatchUIFrameCallback.doFrameGuarded(UIViewOperationQueue.java:1079)
        at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
        at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
        at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1008)
        at android.view.Choreographer.doCallbacks(Choreographer.java:809)
        at android.view.Choreographer.doFrame(Choreographer.java:740)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:995)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:246)
        at android.app.ActivityThread.main(ActivityThread.java:8512)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:602)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1130)
```

Reviewed By: ShikaSD

Differential Revision: D28965864

Pulled By: JoshuaGross

fbshipit-source-id: f08f1df371c1892935e1290806cc67d34394203b
2021-06-09 11:30:16 -07:00
Joshua Gross ac6d1982f4 Re-land: Pass in EventEmitter during View CREATE and Preallocation
Summary:
This is a re-land of D28810022 (https://github.com/facebook/react-native/commit/7e9c741146e1b3aa542ecfe138765e95ddddac3f), which was reverted due to T92179998. The fix is in D28938530. This issue could also be resolved by preventing more view preallocations (D28811419 (https://github.com/facebook/react-native/commit/8ca18f0b602a08cd9a5c9f6681bb5dc74e0d34f7)).

 ---

EventEmitter is not transmitted from C++ to Java until an UPDATE operation is enqueued.

Practically this usually happens "right away", but in the case of an Image component, especially, the EventEmitter could be missing while events are being fired from the native side (for example, loading events).

The fix is just to pass EventEmitter in sooner, in both Create and Preallocate. There should be no ill effect since EventEmitter is nullable anyway.

One potential side-effect: since Views can be PreAllocated and potentially never deallocated until StopSurface is called, this could result in more EventEmitter objects being leaked and retained from Java. I believe the fix is to remove PreAllocated Views more aggressively.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D28938637

fbshipit-source-id: c9e290a24ed15c28881e3eead4a5f580f66b288f
2021-06-07 12:16:12 -07:00
Joshua Gross 792acff149 Clean up EventEmitterWrapper on surface deallocation
Summary:
clean up EventEmitterWrapper on surface deallocation. Will resolve T92179998.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D28938530

fbshipit-source-id: f718945f52a652a4b79a7a376f052035e4f41edb
2021-06-07 12:16:12 -07:00
Joshua Gross e15f171080 Back out "Pass in EventEmitter during View CREATE and Preallocation"
Summary:
Original commit changeset: ae4c8b4eefe6

Reverting temporarily to resolve T92179998.

Changelog: [Internal]

Reviewed By: fkgozali, sammy-SC

Differential Revision: D28938467

fbshipit-source-id: a855c8f08f4826e0bb4b918f2dd3269f200787f5
2021-06-07 12:16:12 -07:00
fabriziobertoglio1987 3827ca6171 Fix font weight numeric values (#29117)
Summary:
This issue fixes https://github.com/facebook/react-native/issues/25696 fixes https://github.com/facebook/react-native/issues/28854 fixes https://github.com/facebook/react-native/issues/26193
Since Android API 28 it is possible to specify fontWeight with numerical values ranging from 100 to 900

This pr uses the new Typeface.create() method available on Android API 28+ to set font weight value ranging from 100 to 900, while still keeping existing functionalities (custom fonts, bold/italic and other styles).
https://developer.android.com/reference/android/graphics/Typeface#create(android.graphics.Typeface,%20int,%20boolean)

## Changelog

[Android] [Fixed] - Fix font weight numeric values

Pull Request resolved: https://github.com/facebook/react-native/pull/29117

Test Plan:
Works in all scenarios.

**<details><summary>CLICK TO OPEN TESTS RESULTS</summary>**
<p>

| **BEFORE** | **AFTER** |
|:-------------------------:|:-------------------------:|
|  <img src="https://user-images.githubusercontent.com/24992535/84420949-1daa0e80-ac1b-11ea-9a2e-eaac03dc4533.png"  width="300" height="" />| <img src="https://user-images.githubusercontent.com/24992535/84490766-edf31900-aca3-11ea-90d8-7c52d2e2be59.png" width="300" height="" /> |

| **AFTER** | **AFTER** |
|:-------------------------:|:-------------------------:|
|  <img src="https://user-images.githubusercontent.com/24992535/84490768-ee8baf80-aca3-11ea-8d3e-937d87b3c56a.png"  width="300" height="" />| <img src="https://user-images.githubusercontent.com/24992535/84490769-ef244600-aca3-11ea-9dec-5eb70358834b.png" width="300" height="" /> |

| **AFTER** |
|:-------------------------:|
|  <img src="https://user-images.githubusercontent.com/24992535/84490772-f0557300-aca3-11ea-851a-5befc900192c.png"  width="300" height="" />|

</p>
</details>

Reviewed By: lunaleaps

Differential Revision: D28917328

Pulled By: yungsters

fbshipit-source-id: 8b84e855b3a8b87960cb79b9237d452b26974c36
2021-06-05 00:47:40 -07:00
Joshua Gross 3ba815228e Ship setJSResponder in code
Summary:
setJSResponder/clearJSResponder have been in use in prod for a while and are stable. Ship them in code.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D28889894

fbshipit-source-id: 1c42526cd890d528062eeb50761fc49cc6109d76
2021-06-04 11:56:13 -07:00
Kacie Bawiec 399285f91c Localize "search", "button", and "togglebutton" accessibility roles by using the platform roles
Summary:
Several accessibilityRole strings are hardcoded to only English on Android. Swap them to just use the platform constants. This way, TalkBack will handle translations.

This change swaps roles "search", "button", and "togglebutton" roles to use the platform description.

Changelog:
[Android][Changed] Localize "search", "button", and "togglebutton" accessibility roles by using the platform roles

Reviewed By: lunaleaps

Differential Revision: D28077246

fbshipit-source-id: 5b88a6fd7e78b3426506f253b823ecca0608c4bc
2021-06-03 14:44:01 -07:00
Ramanpreet Nara 281daf1222 Ship bridge RuntimeExecutor JSIExecutor flushing
Summary:
The RuntimeExecutor that Fabric gets from the bridge doesn't call JSIExecutor::flush(). In the legacy NativeModule system, we're supposed to flush the queue of NativeModule calls after every call into JavaScript. The lack of this flushing means that we execute NativeModule calls less frequently with Fabric enabled, and TurboModules disabled. It also means that [the microtask checkpoints we placed inside JSIExecutor::flush()](https://www.internalfb.com/code/fbsource/[62f69606ae81530f7d6f0cba8466ac604934c901]/xplat/js/react-native-github/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp?lines=427%2C445) won't be executed as frequently, with Fabric enabled.

Changelog: [Android][Fixed] - Flush NativeModule calls with Fabric on Android, on every Native -> JS call.

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D28620982

fbshipit-source-id: ae4d1c16c62b6d4a5089e63104ad97f4ed44c440
2021-06-02 16:54:33 -07:00
Joshua Gross 8ca18f0b60 Don't PreAllocate virtual views
Summary:
Virtual views that are flattened and don't "FormsView" on-screen should not be preallocated.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28811419

fbshipit-source-id: 949dcbf4cf3791355c58af785603b35fa50f3f02
2021-06-02 12:49:49 -07:00
Joshua Gross fc2153cc43 Remove mounting-layer hack
Summary:
This hack should not be necessary. It should be fixed at Differ or LayoutAnimations level if there are existing issues there.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28810021

fbshipit-source-id: 98b8d2ae9991ad527a3b3e90943d75063b2a4496
2021-06-02 12:49:49 -07:00
Dulmandakh 70da640946 remove jcenter (#31609)
Summary:
jcenter is read-only now, and newer versions of dependencies will be published to either MavenCentral or Jitpack. This PR removes jcenter to avoid future issues, then uses MavenCentral and Jitpack as replacement. Current flipper depends on Stetho version that is not available on MavenCentral, so had to exclude and bump the version.

Both Gradle and Buck successfully download all the dependencies.

## Changelog

[Android] [Changed] - Remove jcenter

Pull Request resolved: https://github.com/facebook/react-native/pull/31609

Test Plan: rn-tester builds and runs as expected.

Reviewed By: mdvacca

Differential Revision: D28802444

Pulled By: ShikaSD

fbshipit-source-id: 043ef079d0cda77a1f8dd732678452ed712741a4
2021-06-02 09:57:28 -07:00
David Vacca 286fac5ad0 Delete eager initialization of Fabric classes
Summary:
I run an experiment and I verified that eager initialization of fabric classes is neutral and it's showing a regression in "marketplace:interface:metrics"
See: https://www.internalfb.com/intern/qe2/react_fabric_marketplace_home_android_universe/react_fabric_marketplace_home_eager_init_v1/analysis?control=test_no_eager_init&test=test_eager_init_classes

This diff removed the optimization and it cleans up the code

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28815647

fbshipit-source-id: 2c9fe3875b1797d9a7def61e5ab97c2df2a462dd
2021-06-01 18:27:04 -07:00
Joshua Gross 7e9c741146 Pass in EventEmitter during View CREATE and Preallocation
Summary:
EventEmitter is not transmitted from C++ to Java until an UPDATE operation is enqueued. Practically this usually happens "right away", but in the case of an Image component, especially, the EventEmitter could be missing while events are being fired from the native side (for example, loading events).

The fix is just to pass EventEmitter in sooner, in both Create and Preallocate. There should be no ill effect since EventEmitter is nullable anyway.

One potential side-effect: since Views can be PreAllocated and potentially never deallocated until StopSurface is called, this could result in more EventEmitter objects being leaked and retained from Java. I believe the fix is to remove PreAllocated Views more aggressively.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28810022

fbshipit-source-id: ae4c8b4eefe619d9a99fa5f90f612d6dd4880da5
2021-06-01 15:15:47 -07:00
Andrei Shikov d1ab03235c Remove feature flag for layout transition tag cleanup
Summary:
Removes stale feature flag that was in production for a couple of months. Fix helped to decrease number of crashes significantly, so we can remove it.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28757995

fbshipit-source-id: 375da09c11f265e8bbe03cd99de1b83f168420ce
2021-05-27 14:43:53 -07:00
Andrei Shikov 712bc6769b Log when item execution is delayed during mounting
Summary:
Adds logs to indicate that MountItem was queued instead of executed.
Also moves logging of preMount items to dispatcher for consistent logging with other items.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28690203

fbshipit-source-id: 14d24f0bebb97a810127f5187b4d46f66b6c836f
2021-05-27 04:44:30 -07:00
David Vacca 4efd603b26 Update customDirectEvent types for ReactVitoImageManager
Summary:
This diff updates the customDirectEvent types for ReactVitoImageManager

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28682669

fbshipit-source-id: 3341a7d6dbd5ec90f274f0770c6e883bd82f0a2d
2021-05-25 11:31:47 -07:00
Joshua Gross 5a9b6fc49b Simplify LockFreeEventDispatcherImpl further
Summary:
TL;DR: simplify and delete a bunch of stuff that shouldn't be necessary in Fabric.

I discovered that this event dispatcher (and the older one this is based on) is triple-queueing: we queue events into "staging", and then post "dispatch events" to only run /on the JS thread/. Even in Fabric. Then, each of these events is emitted into C++ where they are queued /again/! This refactor eliminates one more level of queueing - instead of scheduling dispatch for the JS thread, we just emit them directly to C++ when they're received in Java.

Unfortunately, the EventDispatcher is also used to drive AsyncEventBeat in C++:

1. EventBeatManager.onBatchEventDispatched: https://fburl.com/diffusion/qf6dyhsw
   In the C++ impl, it indirectly will drive the AsyncEventBeat/AsyncEventBeatV2.
2. onBatchEventDispatched is ONLY called from EventDispatcherImpl: https://fburl.com/codesearch/mxk8ifyj
3. Which is queued and only runs on the JS thread: https://fburl.com/codesearch/czvbst4u

This means the AsyncEventBeat is only ticked when the JS thread is free, and ticks will be skipped when the JS thread is occupied for long periods.

Now, in this refactor, when this class is used it will drive AsyncEventBeat on every UI tick. This is also potentially not correct. On iOS (Fabric), AsyncEventBeat is driven when the UI thread is "about to sleep".

For now I'm not going to worry about that detail - it is significant, but Fabric+Android is currently /not doing the right thing/ and it's not clear that we want to maintain iOS behavior. This is something we need to discuss further and figure out.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28654033

fbshipit-source-id: b3cb9b706343c8dd3c4cf84f24388908c57e2138
2021-05-24 20:14:46 -07:00
Joshua Gross 900fed576b Events: Remove default getCoalescingKey impls
Summary:
Event.java's default implementation of `getCoalescingKey` returns 0. Most overrides of getCoalescingKey just return 0. Delete them.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28650792

fbshipit-source-id: 374591fb4ba2305df934de412790b3397ca2fa1c
2021-05-24 13:41:19 -07:00
Joshua Gross 0510821170 Replace EventDispatcherImpl with LockFreeEventDispatcherImpl
Summary:
EventDispatcherImpl uses synchronized blocks all over to make it thread-safe. I'm concerned about the perf implications of this and creating contention between JS and UI threads.

This is locked behind a feature flag.

Enabled only for Fabric in StaticViewConfig mode, and a feature flag, for now.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28591331

fbshipit-source-id: ea8f93a2e1343ce37fa78690dcb62fe03594120f
2021-05-21 17:22:41 -07:00
Joshua Gross cd4bebb6c6 Make sure that touch events have a SurfaceId only in Fabric
Summary:
Could fix some infrequent crashes in non-Fabric.

Changelog: [Internal]

Reviewed By: ShikaSD

Differential Revision: D28605773

fbshipit-source-id: 57555e013657e61cfb02b25d9fd14c9c15774e0e
2021-05-21 11:32:02 -07:00
Joshua Gross b161241db2 Proposed fix for ScrollView race condition between C++ state update and onScroll
Summary:
FlatList relies heavily on onScroll events + the measure API. In Fabric, usage of `measure` relies on C++ having an accurate view of the current scroll position of the ScrollView.

We already have a mechanism for updating the scroll position in C++ using UpdateState. But, it is only used currently at the /beginning/ and /end/ of scrolling, and UpdateState is not called /during/ scrolling at all.

This means that we will see a series of events like this while scrolling:

```
Scrolling begins
UPDATE C++ STATE: scrollLeft = 0, scrollTop = 0
JS event: onScroll x=0, y=0
JS event: onScroll x=0, y=100
JS event: onScroll x=0, y=200
...
JS event: onScroll x=0, y=1000
UPDATE C++ STATE: scrollLeft = 0, scrollTop = 1000
```

Notably, not many C++ state updates are queued; and the last one is queued AFTER the JS event is sent. The last JS event and UpdateState will race, which means that sometimes the C++ update will /lose/ and C++ will have an inaccurate view of the world when FlatList receives the onScroll event and calls `measure`.

My proposed solution, gated behind a feature flag, is to delay /some/ onScroll events until the C++ UpdateState has made its way back to Java, and send UpdateStates more frequently. The balance here is that UpdateState is a relatively expensive operation, so we probably still want to call it /less/ than we call onScroll. This means that `measure` will still return some incorrect results but will return correct results more frequently. Win?

Changelog: [Internal[

Reviewed By: mdvacca

Differential Revision: D28558380

fbshipit-source-id: 11c7cd714fae67ee5a94c4413be988481413ec03
2021-05-20 12:53:39 -07:00
Joshua Gross d670381fac Reduce FabricUIManager soft errors further
Summary:
Reduce soft error volume; see comments.

Changelog: [Internal]

Reviewed By: ShikaSD

Differential Revision: D28551214

fbshipit-source-id: 7e6aa1ee7cad812650bd6219d0cbe568c3cdd3de
2021-05-20 09:49:11 -07:00
David Vacca 0b371304aa Create ReactFeatureFlag to initialize MapBufferSo file during Fabric initialization
Summary:
This diff creates a ReactFeatureFlag to initialize MapBufferSo file during Fabric initialization.

This is necessary to be able to compare properly Mapbuffer vs ReadableNativeMap (because ReadableNativeMap c++ files is already included in the bridge so file)

changelog: [internal]

Reviewed By: JoshuaGross

Differential Revision: D28436044

fbshipit-source-id: 338e1bb72b5313dc29a309e1b0e979e7c8bd1c18
2021-05-14 12:55:24 -07:00
Samuel Susla 831e2d41ab Funnel All Fabric calls to RuntimeExecutor to RuntimeScheduler
Summary:
Changelog: [internal]

This diff moves all calls to RuntimeExecutor to RuntimeScheduler. The calls are still immediately dispatched. Timing of events will not change.

The goal of this diff is to prepare infrastructure for Concurrent Mode.

Reviewed By: JoshuaGross

Differential Revision: D27938536

fbshipit-source-id: 750b0e21e0ecbd7aa5a14885ebc70aae82203bd4
2021-05-13 08:03:29 -07:00
Joshua Gross 3c7809ed0d Call onDropViewInstance on all Views when stopSurface is called
Summary:
Call `onDropViewInstance` on all Views when stopSurface is called.

We used to do this but stopped doing it ~6 months ago. This did not cause any prod issues but is not correct.

This allows product code to do cleanup upon view deletion.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28388929

fbshipit-source-id: a8f06d4b1b12a11a907667e0a837c653db035941
2021-05-12 14:05:56 -07:00
Joshua Gross 52b45a44b4 Event should infer UIManagerType by presence of SurfaceId
Summary:
See comments inline for motivation. It's not safe to use viewtag of an Event to infer whether or not the view is in a Fabric or non-Fabric RootView.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D28365566

fbshipit-source-id: 187ddcc5d5a43a31a71232fdb2f1f5b334bec8c2
2021-05-12 09:14:14 -07:00
David Vacca aafdf9fee1 Refactor access of ReactFeatureFlags from C++
Summary:
This diff is a follow up of D28360679 (https://github.com/facebook/react-native/commit/e3367354cc93f17b830ed8dc601dff5e87748dd1), here we refactor the access of ReactFeatureFlags from C++ to use methods instead of fields

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28362066

fbshipit-source-id: caed5e7fddeb6c0d9846fb037152befa8f1ed5c2
2021-05-11 14:10:47 -07:00
David Vacca 22ddab2025 Ensure ReactFeatureFlag fields are not deleted by Redex
Summary:
Since we are now using ReactFeatureFlag from C++, we need to ensure redex doesn't strip its fields.

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28360678

fbshipit-source-id: 74604e2d008a056c161d8b6ab8f5b30807087d9e
2021-05-11 12:57:38 -07:00
David Vacca e3367354cc Use ReactFeatureFlags to enable mapBuffer in C++
Summary:
This diff refactors the way we are populating the 'MapBufferSerializationEnabled' context cointainer key to use ReactFeatureFlags instead of MobileConfig.

This is necessary to make sure we always use a consistent value between C++ and Java. e.g. to prevent SEV like S230730 set different values in java and c++ code

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28360679

fbshipit-source-id: baef9d53f84de25c5671483dcd995674bfa61984
2021-05-11 12:57:38 -07:00
David Vacca 13169f0987 Delete ReactFeatureFlags.useViewManagerDelegatesForCommands
Summary:
This diff deletes ReactFeatureFlags.useViewManagerDelegatesForCommands, this has been enabled in prod for 9+ months

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28265338

fbshipit-source-id: 2f07cb83d6ef9191f9ebea52e230490ef98d6e2d
2021-05-10 15:57:43 -07:00
Joshua Gross 0ef5beee85 Support ScrollAway in ReactScrollView
Summary:
Support ScrollAway in ReactScrollView for Fabric/non-Fabric.

Changelog: [Android][Added] Support for ScrollAway native nav bars added to ReactScrollView

Reviewed By: mdvacca

Differential Revision: D28308855

fbshipit-source-id: 9a922159ef50fb7c8e9c484a4b97ca57ab248496
2021-05-10 12:16:10 -07:00
David Vacca bf8037cad0 Remove ReactFeatureFlags.useViewManagerDelegates
Summary:
This diff deleted the ReactFeatureFlags.useViewManagerDelegates, this has been enabled for 9+ months

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28265339

fbshipit-source-id: f5c97e77ca4fc72d2e2b8f891e800e362177d67a
2021-05-07 20:40:38 -07:00
David Vacca 570c6f1f29 Quick refactor of string tags used in UIManagerHelper
Summary:
this is a quick refactor of the string tags used in UIManagerHelper

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28243264

fbshipit-source-id: c32c9908d40e6184d7e940b14c9782799db3f891
2021-05-06 12:13:40 -07:00
David Vacca b0e8c1eac0 Refactor UIManagerHelper.getUIManager to return null when there's no UIManager registered
Summary:
This diff refactors the UIManagerHelper.getUIManager method to return null when there's no UIManager registered for the uiManagerType received as a parameter.

This is necessary to workaround: https://github.com/facebook/react-native/issues/31245

changelog: [changed] UIManagerHelper.getUIManager now returns null when there's no UIManager registered for the uiManagerType received as a parameter

Reviewed By: fkgozali

Differential Revision: D28242592

fbshipit-source-id: c3a4979bcf6e547d0f0060737e41bbf19860a984
2021-05-06 12:13:40 -07:00
David Vacca 7fe6bc1150 Create MC to verify impact of eager initialization of fabric classes
Summary:
This diff creates a MC to verify impact of eager initialization of fabric classes, the purpose is to remove this code, but before doing that I would like to verify what's the impact.

changelog: [internal] internal

Reviewed By: sammy-SC

Differential Revision: D28223943

fbshipit-source-id: 6f7c4701fb730fe1c0629ec13ead592ff619373f
2021-05-05 13:06:56 -07:00
David Vacca c6b8625664 Eager load all fabric classes before initializing Fabric
Summary:
Eager load all fabric classes before initializing Fabric

changelog: [internal] internal

Reviewed By: fkgozali

Differential Revision: D28207855

fbshipit-source-id: 205f8220fd75178aa0f33cbf734ea74859a4f973
2021-05-05 09:12:41 -07:00
David Vacca 3555bf20a8 Update javadoc in fabric to remove reference to UIManagerModule
Summary:
Update javadoc in fabric to remove reference to UIManagerModule

changelog: [internal] internal

Reviewed By: fkgozali

Differential Revision: D28207024

fbshipit-source-id: 1658c01b10c2433565d6df1a6ed989d83a3c4a8d
2021-05-05 09:12:41 -07:00
David Vacca d583cb8def Dispatch switch events using the proper UIModule
Summary:
This diff ensures that the dispatch of switch events is performed using the proper UIModule

changelog: [internal] internal

Reviewed By: JoshuaGross

Differential Revision: D28204930

fbshipit-source-id: 625b536ab3106efa7dbf583589dfe268b880a6a0
2021-05-05 09:12:41 -07:00
fabriziobertoglio1987 333b46c4b0 Fix Image does not announce "disabled" (#31252)
Summary:
This issue fixes https://github.com/facebook/react-native/issues/30935 screenreader does not announce Image disabled accessibilityState.

As stated in AOSP View.java, the framework will handle routine focus movement, views indicate their willingness to take focus through the `isFocusable` method https://bit.ly/3dCnyHb

```
* <p>The framework will handle routine focus movement in response to user input. This includes
* changing the focus as views are removed or hidden, or as new views become available. Views
* indicate their willingness to take focus through the {link #isFocusable} method. To change
* whether a view can take focus, call {link #setFocusable(boolean)}.
```

The property is updated through its shadow node `ReactImageManager` method `setAccessible` https://bit.ly/3dDuK5L

```java
 * <p>Instances of this class receive property updates from JS via @{link UIManagerModule}.
 * Subclasses may use {link #updateShadowNode} to persist some of the updated fields in the node
 * instance that corresponds to a particular view type.
```

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Fixed] - adding setAccessible to ReactImageManager to allow screenreader announce Image accessibilityState of "disabled"

Pull Request resolved: https://github.com/facebook/react-native/pull/31252

Test Plan:
**<details><summary>CLICK TO OPEN TESTS RESULTS</summary>**
<p>

Enable audio to hear the screenreader

TEST SCENARIO
- The user moves the screenreader focus to an image and the screenreader reads the Image accessibilityLabel "plain network image"

RESULT
- The screenreader announces the accessibilityState disabled after reading the Image accessibilityLabel "plain network image"

```javascript
<Image
  accessible={true}
  accessibilityLabel="plain network image"
  accessibilityState={{disabled: true}}
  source={fullImage}
  style={styles.base}
/>
```

<video src="https://user-images.githubusercontent.com/24992535/112670432-2f366d00-8e61-11eb-843f-4b56f4a06a91.mp4" width="700" />

</p>
</details>

Reviewed By: kacieb

Differential Revision: D28194597

Pulled By: lunaleaps

fbshipit-source-id: 5f89ce5c714405506261885ac6fea2c15c2e1f23
2021-05-04 14:29:58 -07:00
Kacie Bawiec da899c0cc4 Add support for Toggle Button accessibilityRole
Summary:
Changelog:
[General][Added] Add support for "togglebutton" accessibilityRole

# Context
The role for ToggleButton, which is needed on Android to implement toggle buttons correctly, is not currently supported.

# What does this diff do?
Adds support for accessibilityRole `"togglebutton"`.

On Android, this maps to class `"Android.widget.ToggleButton"`.

iOS does not have an equivalent trait for togglebutton, so I set it to be the same as setting `accessibilityRole="button"` for iOS.

# Caveats - checked vs selected
It seems to me like this role currently requires that you set `accessibilityState={{checked: true/false}}`. The behavior is strange when setting `selected` state, I think because on Android ToggleButtons are meant to use `checked` to indicate toggled on/off.

This is tricky because typically on iOS if you have a toggle button, you would use `selected` instead of `checked`, so RN users are likely to mess this up.

Possible solutions:
1. document that you should use `checked` state on Android for toggle buttons (and maybe throw a warning if someone passes in `selected`).
2. have RN ignore it if someone passes in accessibilityState `selected`, if this role is used.
3. Have RN convert passed in `selected` state to `checked` on the Android side.

Reviewed By: nadiia

Differential Revision: D27976046

fbshipit-source-id: 4ce202449cf2371f4bf83c4db2d53120369ee7b0
2021-05-03 11:50:02 -07:00