Skip to content

Commit

Permalink
Fix Binding JNI type (#41657)
Browse files Browse the repository at this point in the history
Summary:
New implementation:
This PR adds cast from interface Binding to BindingImpl class.

Previous implementation:
The changes made in this PR make the `mBinding` field of `FabricUIManager` visible for JNI.

Without these changes, calling the method `JFabricUIManager::getBinding()` would result in an error.

<img width="400" alt="Screenshot 2023-11-27 at 13 55 44" src="https://github.com/facebook/react-native/assets/36106620/04418291-8ce8-4bae-b16c-29a5c9f2ee52">

In the `react-native-reanimated` library, we utilize `JFabricUIManager::getBinding()`, and we have noticed this issue since version 0.73. This isn't perfect solution, but I'm not certain which change in RN or FBJNI is the source of the problem. If there are any alternative solutions worth considering, I am open to discussing them.

Usage of `getBinding()` in Reanimated:
https://github.com/software-mansion/react-native-reanimated/blob/main/android/src/main/cpp/NativeProxy.cpp#L57

## Changelog:

[ANDROID] [FIXED] - Fix type for unrecognisable field mBinding

Pull Request resolved: #41657

Test Plan:
Just call `JFabricUIManager::getBinding` method (https://github.com/facebook/react-native/blob/v0.73.0-rc.5/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JFabricUIManager.cpp#L14)

or run app with repro:
https://github.com/piaskowyk/missing-mBinding-repro
after the app lunch you will receive error from above screenshot.

Co-author: tomekzaw

Reviewed By: NickGerleman

Differential Revision: D51661873

Pulled By: javache

fbshipit-source-id: 1891c36bf25c503ebc9b0501211df03be6f74115
  • Loading branch information
piaskowyk authored and facebook-github-bot committed Nov 30, 2023
1 parent 05ed007 commit 31d8a93
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ class ReactNativeConfig;
class Scheduler;
class SurfaceHandlerBinding;

class Binding : public jni::HybridClass<Binding>,
struct JBinding : public jni::JavaClass<JBinding> {
constexpr static auto kJavaDescriptor = "Lcom/facebook/react/fabric/Binding;";
};

class Binding : public jni::HybridClass<Binding, JBinding>,
public SchedulerDelegate,
public LayoutAnimationStatusDelegate {
public:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ namespace facebook::react {

Binding* JFabricUIManager::getBinding() {
static const auto bindingField =
javaClassStatic()->getField<Binding::javaobject>("mBinding");
javaClassStatic()->getField<JBinding::javaobject>("mBinding");

return getFieldValue(bindingField)->cthis();
return jni::static_ref_cast<Binding::javaobject>(getFieldValue(bindingField))
->cthis();
}
} // namespace facebook::react

0 comments on commit 31d8a93

Please sign in to comment.