Skip to content

Commit

Permalink
Implement ReadableNative*::mapException without dynamic_cast
Browse files Browse the repository at this point in the history
Summary:
Changing fbjni's mapException interface to use `std::exception_ptr`, this enables us to use `std::rethrow_exception` and `catch` statements to check for specific exception types, rather than using dynamic_casts and having an explicit dependency on RTTI.

While generally, we should always be enabling RTTI and exception support simultaneously, we have a number of targets where we don't, or where avoiding RTTI provides significant binary size gains.

Changelog: [Internal]

Reviewed By: mhorowitz

Differential Revision: D34379950

fbshipit-source-id: 446d105f9fb50ada3526f242f98e163215abd0ab
  • Loading branch information
javache authored and facebook-github-bot committed Feb 25, 2022
1 parent 267d36d commit 42719e3
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 2 deletions.
10 changes: 10 additions & 0 deletions ReactAndroid/src/main/jni/react/jni/ReadableNativeArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,23 @@ __attribute__((visibility("default")))
ReadableNativeArray::ReadableNativeArray(folly::dynamic array)
: HybridBase(std::move(array)) {}

// TODO T112842309: Remove after fbjni upgraded in OSS
void ReadableNativeArray::mapException(const std::exception &ex) {
if (dynamic_cast<const folly::TypeError *>(&ex) != nullptr) {
throwNewJavaException(
exceptions::gUnexpectedNativeTypeExceptionClass, ex.what());
}
}

void ReadableNativeArray::mapException(std::exception_ptr ex) {
try {
std::rethrow_exception(ex);
} catch (const folly::TypeError &err) {
throwNewJavaException(
exceptions::gUnexpectedNativeTypeExceptionClass, err.what());
}
}

local_ref<JArrayClass<jobject>> ReadableNativeArray::importArray() {
jint size = array_.size();
auto jarray = JArrayClass<jobject>::newArray(size);
Expand Down
4 changes: 3 additions & 1 deletion ReactAndroid/src/main/jni/react/jni/ReadableNativeArray.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ class ReadableNativeArray
"Lcom/facebook/react/bridge/ReadableNativeArray;";

static void mapException(const std::exception &ex);
static void mapException(std::exception_ptr ex);
static void registerNatives();

jni::local_ref<jni::JArrayClass<jobject>> importArray();
jni::local_ref<jni::JArrayClass<jobject>> importTypeArray();
static void registerNatives();
};

} // namespace react
Expand Down
10 changes: 10 additions & 0 deletions ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,23 @@ using namespace facebook::jni;
namespace facebook {
namespace react {

// TODO T112842309: Remove after fbjni upgraded in OSS
void ReadableNativeMap::mapException(const std::exception &ex) {
if (dynamic_cast<const folly::TypeError *>(&ex) != nullptr) {
throwNewJavaException(
exceptions::gUnexpectedNativeTypeExceptionClass, ex.what());
}
}

void ReadableNativeMap::mapException(std::exception_ptr ex) {
try {
std::rethrow_exception(ex);
} catch (const folly::TypeError &err) {
throwNewJavaException(
exceptions::gUnexpectedNativeTypeExceptionClass, err.what());
}
}

void addDynamicToJArray(
local_ref<JArrayClass<jobject>> jarray,
jint index,
Expand Down
2 changes: 1 addition & 1 deletion ReactAndroid/src/main/jni/react/jni/ReadableNativeMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct ReadableNativeMap : jni::HybridClass<ReadableNativeMap, NativeMap> {
static jni::local_ref<jhybridobject> createWithContents(folly::dynamic &&map);

static void mapException(const std::exception &ex);

static void mapException(std::exception_ptr ex);
static void registerNatives();

using HybridBase::HybridBase;
Expand Down

0 comments on commit 42719e3

Please sign in to comment.