Upgrade to WebKitGTK 2.36.3 #169

Kudo · 2022-07-10T17:17:27Z

Why

upgrade to latest WebKitGTK 2.36.3 (svn r294992). mostly from community requests and hopefully to fix the crashes on samsung android 12 devices.

How

upgrade NDK to 23.2.8568313. we need newer versions because the newer webkit needs std::filesystem that doesn't support on NDK r21. also, because the NDK r23 libc++_shared.so has std::filesystem symbols that older version doesn't have. to prevent developers use pickFirst to pick wrong libc++_shared.so, i turned to use link static c++ runtime instead.
[BREAKING CHANGE] remove non Intl variant, because newer JSC doesn't have a build flag to turn it off.
[BREAKING CHANGE] Bump minimum supported Android SDK to 21
remove outdated patches for older jsc or ndk
remove some platform specific cflags, let ndk to do it.

size changes:

-	r250230 Intl	r294992 Intl
armeabi-v7a	14 MB	17 MB
arm64-v8a	18 MB	19 MB
x86	17 MB	20 MB
x86_64	19 MB	21 MB

Test Plan

ci passed.
launch test on arm64 devices

- Keeps only Intl variant - Upgrade NDK to r23 - Set minimal Android SDK to 21 - Remove unused patch - Update CI

wkozyra95 · 2022-07-12T08:28:02Z

.github/workflows/build_and_test.yml

+          "cmake;3.18.1" \
+          "ndk;23.2.8568313"
+        # move out builtin icu headers from ndk and prevent icu build errors
+        mv "${ANDROID_HOME}/ndk/23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/unicode" "${ANDROID_HOME}/ndk/23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/unicode2"


why is this necessary, can you clarify what was the error? Does it mean that building jsc will not work locally? And if it works locally, then it should be possible to make it work on ci without this patch.

from NDK r22, the icu headers are builtin inside NDK. when building icu, the includes such as #include "unicode/utypes.h" will access to NDK headers first and lead to build errors.

i also need to move out the unicode/ folder from NDK when building locally.

wkozyra95 · 2022-07-12T08:30:28Z

scripts/compile/jsc.sh

@@ -19,7 +19,7 @@ $PLATFORM_CFLAGS \
 CMAKE_LD_FLAGS=" \
 -latomic \
 -lm \
-lc++_shared \
+-static-libstdc++ \


why static?

upgrade NDK to 23.2.8568313. we need newer versions because the newer webkit needs std::filesystem that doesn't support on NDK r21. also, because the NDK r23 libc++_shared.so has std::filesystem symbols that older version doesn't have. to prevent developers use pickFirst to pick wrong libc++_shared.so, i turned to use link static c++ runtime instead.

left it on pr description. feel free to let me know if that's not clear.

This does not sound like a safe change. e.g. if we have jsc with statically linked stl and e.g. expo-gl or react-native build with shared stl then we are including stl twice and potentially different versions of it

https://developer.android.com/ndk/guides/cpp-support#sr

In this situation, the STL, including and global data and static constructors, will be present in both libraries. The runtime behavior of this application is undefined, and in practice crashes are very common. Other possible issues include:

Memory allocated in one library, and freed in the other, causing memory leakage or heap corruption.
Exceptions raised in libfoo.so going uncaught in libbar.so, causing your app to crash.
Buffering of std::cout not working properly.

I just wanted to mention my concern on that, I have only a minimal idea of how all this works, so if you think it's a safe change I'm fine with it

interesting! i would say thankfully the libjsc.so is really independency. the exceptions, memory allocations are not across other libraries. the JSCRuntime.cpp implementation in react-native only relies on C interfaces, so it might be safer in this case.

otherwise, TBH i don't have a better solution toward the inconsistent libc++_shared.so issues.

aweffr · 2022-08-20T10:29:25Z

I have test jsc-android@294992.0.0 on react native 0.69.4 but failed to start

the error log is:

2022-08-20 18:17:31.624 30482-30482/com.dongdongapp D/SoLoader: libjscexecutor.so not found on /data/data/com.dongdongapp/lib-main
2022-08-20 18:17:31.624 30482-30482/com.dongdongapp D/SoLoader: libjscexecutor.so found on /data/app/com.dongdongapp-Mweej1BEjjpm6zwMqcyWbQ==/lib/arm64
2022-08-20 18:17:31.624 30482-30482/com.dongdongapp D/SoLoader: Not resolving dependencies for libjscexecutor.so
2022-08-20 18:17:31.630 30482-30482/com.dongdongapp W/System.err: java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.dongdongapp-Mweej1BEjjpm6zwMqcyWbQ==/lib/arm64/libjscexecutor.so"...
2022-08-20 18:17:31.630 30482-30482/com.dongdongapp W/System.err:     at java.lang.Runtime.load0(Runtime.java:938)
2022-08-20 18:17:31.630 30482-30482/com.dongdongapp W/System.err:     at java.lang.System.load(System.java:1632)

if i use 250230 as default, it works, the log is:

2022-08-20 18:23:26.739 31472-31472/com.dongdongapp D/SoLoader: libjscexecutor.so not found on /data/data/com.dongdongapp/lib-main
2022-08-20 18:23:26.739 31472-31472/com.dongdongapp D/SoLoader: libjscexecutor.so found on /data/app/com.dongdongapp-efMygP2ImvdqkCDQPP5tdQ==/lib/arm64
2022-08-20 18:23:26.739 31472-31472/com.dongdongapp D/SoLoader: Not resolving dependencies for libjscexecutor.so
2022-08-20 18:23:26.776 31472-31472/com.dongdongapp D/JavaScriptCore.Version: 250230.2.1
2022-08-20 18:23:26.889 31472-31472/com.dongdongapp D/NetworkSecurityConfig: No Network Security Config specified, using platform default

Kudo · 2022-08-21T15:34:30Z

hi @aweffr, do you mind to create a github repository of minimal reproducible example for me? i tested jsc-android@294992.0.0 + react-native@0.69.4 on android 12 emulator and it works for me.

khoanguyen-yang · 2022-11-30T09:12:32Z

Hi @Kudo,

Is there any way that I can use non Intl for this JSC version? Enabling Intl significantly increases the app bundle size though

Kudo · 2022-12-06T08:31:34Z

@khoanguyen-yang unfortunately no. as the pr described the new jsc has no flag to turn off Intl now.

cortinico · 2022-12-06T10:18:52Z

@khoanguyen-yang unfortunately no. as the pr described the new jsc has no flag to turn off Intl now.

@Kudo do we need to update the RN template and remove the non-intl flavor there?

Kudo · 2022-12-06T11:30:49Z

@cortinico as facebook/react-native#35504 (comment) mentioned, we don't have to update RN template in the mean time.

# Why fixes #178 # How - manually pick necessary changes for ndk r23 from #169. it's ndk r23c which is newer than react-native 0.71's r23b. i cannot build jsc on top on r23b successfully. hopefully r23b <-> r23c is good for abi safety. - fix build errors for webkitgtk 2.26.1 + ndk r23. - unlike #169, this pr is still using shared c++ runtime. - update test project to 0.71 and build from source code because we need a patch to link libunwind in libjscexecutor.so. # Test Plan ci passed

Hesowcharov · 2024-01-10T08:21:30Z

I have test jsc-android@294992.0.0 on react native 0.69.4 but failed to start

the error log is:

2022-08-20 18:17:31.624 30482-30482/com.dongdongapp D/SoLoader: libjscexecutor.so not found on /data/data/com.dongdongapp/lib-main
2022-08-20 18:17:31.624 30482-30482/com.dongdongapp D/SoLoader: libjscexecutor.so found on /data/app/com.dongdongapp-Mweej1BEjjpm6zwMqcyWbQ==/lib/arm64
2022-08-20 18:17:31.624 30482-30482/com.dongdongapp D/SoLoader: Not resolving dependencies for libjscexecutor.so
2022-08-20 18:17:31.630 30482-30482/com.dongdongapp W/System.err: java.lang.UnsatisfiedLinkError: dlopen failed: cannot locate symbol "_Unwind_Resume" referenced by "/data/app/com.dongdongapp-Mweej1BEjjpm6zwMqcyWbQ==/lib/arm64/libjscexecutor.so"...
2022-08-20 18:17:31.630 30482-30482/com.dongdongapp W/System.err:     at java.lang.Runtime.load0(Runtime.java:938)
2022-08-20 18:17:31.630 30482-30482/com.dongdongapp W/System.err:     at java.lang.System.load(System.java:1632)

if i use 250230 as default, it works, the log is:

2022-08-20 18:23:26.739 31472-31472/com.dongdongapp D/SoLoader: libjscexecutor.so not found on /data/data/com.dongdongapp/lib-main
2022-08-20 18:23:26.739 31472-31472/com.dongdongapp D/SoLoader: libjscexecutor.so found on /data/app/com.dongdongapp-efMygP2ImvdqkCDQPP5tdQ==/lib/arm64
2022-08-20 18:23:26.739 31472-31472/com.dongdongapp D/SoLoader: Not resolving dependencies for libjscexecutor.so
2022-08-20 18:23:26.776 31472-31472/com.dongdongapp D/JavaScriptCore.Version: 250230.2.1
2022-08-20 18:23:26.889 31472-31472/com.dongdongapp D/NetworkSecurityConfig: No Network Security Config specified, using platform default

The same issue. Did you find the solution ?

Kudo · 2024-01-10T10:12:37Z

@Hesowcharov jsc-android@294992.0.0 is not compatible with older react-native since android ndk is incompatible.

Hesowcharov · 2024-01-10T12:44:35Z

@Kudo Thank you for your reply! So, after rn 0.69 (i used 0.69.12) it's recommended to use the default jsc (from facebook) ? I'm just wondering that it should be working since the example app (in this repo) works with rn 0.69 🤔

Hesowcharov · 2024-01-11T19:38:54Z

for someone who will try to migrate to RN 0.69-0.70, move straight to 0.71.x (my version is 0.71.15) => there jsc-android@294992.0.0 works!

  JavaScriptCore.Version  myapp                D  294992.0.0
  SoLoader                myapp                D  libjscexecutor.so found on /data/app/myapp-blah-blah/base.apk!/lib/arm64-v8a

Upgrade to WebKit GTK 2.36.3

81ff9a8

- Keeps only Intl variant - Upgrade NDK to r23 - Set minimal Android SDK to 21 - Remove unused patch - Update CI

Kudo force-pushed the webkit-gtk-2.36.3 branch from c21c5b2 to 81ff9a8 Compare July 11, 2022 06:34

Kudo added 2 commits July 11, 2022 16:33

remove unused cppruntime

0acc0fe

ignore dist.unstripped from git

7dd772f

Kudo changed the title ~~WebKitGTK 2.36.3~~ Upgrade to WebKitGTK 2.36.3 Jul 11, 2022

Kudo marked this pull request as ready for review July 11, 2022 12:23

Kudo requested a review from wkozyra95 July 11, 2022 15:18

wkozyra95 reviewed Jul 12, 2022

View reviewed changes

wkozyra95 approved these changes Jul 12, 2022

View reviewed changes

Kudo merged commit 6b1ec06 into main Jul 12, 2022

Kudo deleted the webkit-gtk-2.36.3 branch July 12, 2022 17:01

Kudo mentioned this pull request Jul 13, 2022

Upgrade JSC past 260345 #159

Open

khoanguyen-yang mentioned this pull request Nov 30, 2022

0.67.5 - Android BigInt being NaN facebook/react-native#35504

Closed

kraenhansen mentioned this pull request Feb 3, 2023

Current jsc-android prebuilt mismatch NDK version with template app facebook/react-native#36052

Closed

Kudo added a commit that referenced this pull request Feb 3, 2023

pick ndkr23 changes from #169

ee44305

Kudo mentioned this pull request Feb 3, 2023

feat: build with ndk r23c on top of webkitgtk 2.26.1 #179

Merged

Kudo added a commit that referenced this pull request Feb 4, 2023

pick ndkr23 changes from #169

dc3c05a

Kudo added a commit that referenced this pull request Feb 4, 2023

pick ndkr23 changes from #169

c4110fa

Kudo added a commit that referenced this pull request Feb 4, 2023

pick ndkr23 changes from #169

dd1a9d2

Kudo added a commit that referenced this pull request Feb 4, 2023

pick ndkr23 changes from #169

376fefb

Kudo added a commit that referenced this pull request Feb 4, 2023

pick ndkr23 changes from #169

a432296

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to WebKitGTK 2.36.3 #169

Upgrade to WebKitGTK 2.36.3 #169

Kudo commented Jul 10, 2022 •

edited

Loading

wkozyra95 Jul 12, 2022

Kudo Jul 12, 2022

wkozyra95 Jul 12, 2022

Kudo Jul 12, 2022

wkozyra95 Jul 12, 2022

Kudo Jul 12, 2022

aweffr commented Aug 20, 2022 •

edited

Loading

Kudo commented Aug 21, 2022

khoanguyen-yang commented Nov 30, 2022 •

edited

Loading

Kudo commented Dec 6, 2022

cortinico commented Dec 6, 2022 •

edited

Loading

Kudo commented Dec 6, 2022

Hesowcharov commented Jan 10, 2024

Kudo commented Jan 10, 2024

Hesowcharov commented Jan 10, 2024

Hesowcharov commented Jan 11, 2024

Upgrade to WebKitGTK 2.36.3 #169

Upgrade to WebKitGTK 2.36.3 #169

Conversation

Kudo commented Jul 10, 2022 • edited Loading

Why

How

Test Plan

wkozyra95 Jul 12, 2022

Choose a reason for hiding this comment

Kudo Jul 12, 2022

Choose a reason for hiding this comment

wkozyra95 Jul 12, 2022

Choose a reason for hiding this comment

Kudo Jul 12, 2022

Choose a reason for hiding this comment

wkozyra95 Jul 12, 2022

Choose a reason for hiding this comment

Kudo Jul 12, 2022

Choose a reason for hiding this comment

aweffr commented Aug 20, 2022 • edited Loading

Kudo commented Aug 21, 2022

khoanguyen-yang commented Nov 30, 2022 • edited Loading

Kudo commented Dec 6, 2022

cortinico commented Dec 6, 2022 • edited Loading

Kudo commented Dec 6, 2022

Hesowcharov commented Jan 10, 2024

Kudo commented Jan 10, 2024

Hesowcharov commented Jan 10, 2024

Hesowcharov commented Jan 11, 2024

Kudo commented Jul 10, 2022 •

edited

Loading

aweffr commented Aug 20, 2022 •

edited

Loading

khoanguyen-yang commented Nov 30, 2022 •

edited

Loading

cortinico commented Dec 6, 2022 •

edited

Loading