Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add TODOs for missing ReactActivityDelegate methods #43294

Closed
wants to merge 2 commits into from
Closed

Add TODOs for missing ReactActivityDelegate methods #43294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
05d856f
[skip ci] Allow moving SyncCallback (#43268)
javache Mar 4, 2024
a56877c
Add TODOs for missing ReactActivityDelegate methods (#43294)
javache Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions ...ges/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactActivityDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
}

public boolean onKeyDown(int keyCode, KeyEvent event) {
if (!ReactFeatureFlags.enableBridgelessArchitecture) {
if (ReactFeatureFlags.enableBridgelessArchitecture) {
// TODO T156475655: support onKeyDown
} else {
if (getReactNativeHost().hasInstance()
&& getReactNativeHost().getUseDeveloperSupport()
&& keyCode == KeyEvent.KEYCODE_MEDIA_FAST_FORWARD) {
Expand All @@ -160,7 +162,9 @@ public boolean onKeyUp(int keyCode, KeyEvent event) {
}

public boolean onKeyLongPress(int keyCode, KeyEvent event) {
if (!ReactFeatureFlags.enableBridgelessArchitecture) {
if (ReactFeatureFlags.enableBridgelessArchitecture) {
// TODO T156475655: support onKeyLongPress
} else {
if (getReactNativeHost().hasInstance()
&& getReactNativeHost().getUseDeveloperSupport()
&& keyCode == KeyEvent.KEYCODE_MEDIA_FAST_FORWARD) {
Expand All @@ -176,7 +180,9 @@ public boolean onBackPressed() {
}

public boolean onNewIntent(Intent intent) {
if (!ReactFeatureFlags.enableBridgelessArchitecture) {
if (ReactFeatureFlags.enableBridgelessArchitecture) {
// TODO T156475655: support onNewIntent
} else {
if (getReactNativeHost().hasInstance()) {
getReactNativeHost().getReactInstanceManager().onNewIntent(intent);
return true;
Expand All @@ -186,15 +192,19 @@ public boolean onNewIntent(Intent intent) {
}

public void onWindowFocusChanged(boolean hasFocus) {
if (!ReactFeatureFlags.enableBridgelessArchitecture) {
if (ReactFeatureFlags.enableBridgelessArchitecture) {
// TODO T156475655: support onWindowFocusChanged
} else {
if (getReactNativeHost().hasInstance()) {
getReactNativeHost().getReactInstanceManager().onWindowFocusChange(hasFocus);
}
}
}

public void onConfigurationChanged(Configuration newConfig) {
if (!ReactFeatureFlags.enableBridgelessArchitecture) {
if (ReactFeatureFlags.enableBridgelessArchitecture) {
// TODO T156475655: support onConfigurationChanged
} else {
if (getReactNativeHost().hasInstance()) {
getReactInstanceManager().onConfigurationChanged(getContext(), newConfig);
}
Expand Down
1 change: 0 additions & 1 deletion packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ public void onActivityResult(
int requestCode, int resultCode, Intent data, boolean shouldForwardToReactInstance) {
if (ReactFeatureFlags.enableBridgelessArchitecture) {
// TODO T156475655: Implement onActivityResult for Bridgeless
return;
} else {
if (getReactNativeHost().hasInstance() && shouldForwardToReactInstance) {
getReactNativeHost()
Expand Down
28 changes: 16 additions & 12 deletions packages/react-native/ReactCommon/react/bridging/Function.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,6 @@ class AsyncCallback {
callWithFunction(priority, std::move(callImpl));
}

/// Invoke the function write-away as if it was a synchronous function
/// without any synchronization or delegating to JS context.
/// @note Caller is responsible for calling this from within JS context.
void unsafeCallSync(Args... args) const noexcept {
if (callback_) {
(*callback_)(std::forward<Args>(args)...);
}
}

private:
friend Bridging<AsyncCallback>;

Expand Down Expand Up @@ -110,6 +101,9 @@ class AsyncCallback {
}
};

// You must ensure that when invoking this you're located on the JS thread, or
// have exclusive control of the JS VM context. If you cannot ensure this, use
// AsyncCallback instead.
template <typename R, typename... Args>
class SyncCallback<R(Args...)> {
public:
Expand All @@ -122,9 +116,19 @@ class SyncCallback<R(Args...)> {
rt,
std::move(jsInvoker))) {}

// Disallow moving to prevent function from get called on another thread.
SyncCallback(SyncCallback&&) = delete;
SyncCallback& operator=(SyncCallback&&) = delete;
// Disallow copying, as we can no longer safely destroy the callback
// from the destructor if there's multiple copies
SyncCallback(const SyncCallback&) = delete;
SyncCallback& operator=(const SyncCallback&) = delete;

// Allow move
SyncCallback(SyncCallback&& other) noexcept
: wrapper_(std::move(other.wrapper_)) {}

SyncCallback& operator=(SyncCallback&& other) {
wrapper_ = std::move(other.wrapper_);
return *this;
}

~SyncCallback() {
if (auto wrapper = wrapper_.lock()) {
Expand Down
Loading